Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isBase64URL support #1277

Merged

Conversation

mum-never-proud
Copy link
Contributor

@mum-never-proud mum-never-proud commented Apr 1, 2020

Resolves #1212

@profnandaa
Copy link
Member

@mum-never-proud -- BTW, what's the difference between isBase64 and isBase64URL? #1212

@mum-never-proud
Copy link
Contributor Author

mum-never-proud commented Apr 1, 2020

generally, base64 encoded strings are not URL safe as they contain the following characters

+, / that may alter the meaning of the data when passed raw base64Url encoding just replaces those occurrences with -, _ respectively

it also contains = generally its safe

shouldn't it be renamed as isBase64URLSafe?

@mum-never-proud
Copy link
Contributor Author

@profnandaa pong :P

@profnandaa
Copy link
Member

How about if we extended isBase64 with a { urlsafe: true } option? Other than having this as a separate validator 🤔

@mum-never-proud
Copy link
Contributor Author

mum-never-proud commented Apr 7, 2020

yep that works as well, I was just thinking about autocomplete in editors still, I guess your idea is better

thoughts?

cc: @profnandaa

@profnandaa
Copy link
Member

What do you mean by autocomplete?

@mum-never-proud
Copy link
Contributor Author

Autocomplete in editor, editor suggestions while typing the code

@mum-never-proud
Copy link
Contributor Author

@profnandaa pong

@profnandaa
Copy link
Member

@mum-never-proud -- sorry for the late reply. I think auto-complete in this case shouldn't be a big driver for our design...

@mum-never-proud
Copy link
Contributor Author

@profnandaa since we don't have any other options can we add it as a param like

isBase64(str, urlSafe)

@profnandaa
Copy link
Member

how about isBase64(str, { urlSafe: true }), to just follow the convention of (str, configObject).

@mum-never-proud
Copy link
Contributor Author

@profnandaa done

@profnandaa
Copy link
Member

Can resolve the merge conflicts?

@parasg1999 parasg1999 mentioned this pull request May 26, 2020
@profnandaa
Copy link
Member

@mum-never-proud -- ping! Looks like your work with help with the JWT bug #964

@mum-never-proud
Copy link
Contributor Author

@profnandaa ah sorry i missed the conversation, will resolve the conflict in sometime

@mum-never-proud
Copy link
Contributor Author

@profnandaa can you review?

and also let me know is there a easy way to check if my changes are into min files

@profnandaa
Copy link
Member

The min file is usually rebuilt before publishing, so no worries.

@profnandaa profnandaa merged commit 027d092 into validatorjs:master May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

isBase64Url()
2 participants