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

Parameters encoded to base 64 should be decoded as UTF-8, not ASCII. #1496

Merged
merged 1 commit into from Mar 21, 2015

Conversation

albanm
Copy link
Contributor

@albanm albanm commented Mar 20, 2015

This fixes a bug that I have currently. Some users were created with special characters in password but could not authenticate through request.auth method afterward.

@froatsnook
Copy link
Contributor

Is UTF-8 the right encoding to use, though? Or should it be ISO-8859-1? Apparently there's no consistency among the spec, browser implementations, and server implementations: https://code.google.com/p/chromium/issues/detail?id=25790

I would say we should use utf-8 as you suggested, but also accept a Buffer for user and/or pass for users dealing with servers that expect ISO-8859-1 instead of utf-8.

@simov
Copy link
Member

simov commented Mar 20, 2015

Thanks @albanm ! Also the remarks about ISO-8859-1 are correct, but I think we should be good with just utf8 for now 👍

simov added a commit that referenced this pull request Mar 21, 2015
Parameters encoded to base 64 should be decoded as UTF-8, not ASCII.
@simov simov merged commit cd375d1 into request:master Mar 21, 2015
@albanm
Copy link
Contributor Author

albanm commented Mar 22, 2015

Thanks !

Is it possible to release a small patch version containing this fix ?

@simov
Copy link
Member

simov commented Mar 22, 2015

The next request release is scheduled for the upcoming week, I can't tell you exact day and time, but we're rounding up the last features/bugfixes and then we're good to go.

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.

None yet

3 participants