Corrected hashing of UTF-8 strings. #4
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was an issue with hashing string formed of multi-byte characters. Only one of the byte of the character was taken into account when building the hash. This fix solves the situation. Unfortunately, this makes any non-latin passwords previously hashed by the code obsolete and totally impossible to verify. Any password containing UTF-8 multi-byte characters hashed with the previous version of the code cannot be verified after this change is applied. I recommend using this patch only if you are starting up a new project and keep using the old code version if you don't have the capability in your project of having both version in parallel until all your user's passwords are rolled-over to the new hashing algorithm. I added a unit test to demonstrate that the new algorithm works as expected with multi-byte characters.
|
First of all, sorry for the massive delay. I have a doubt on how to proceed (This is pretty much my first npm module). Should I simply merge this in and bump the version and add a warning in the readme about how the passwords already encoded with version 0.0.2 won't work anymore with 0.0.3 ? |
shaneMangudi
added a commit
that referenced
this pull request
Feb 25, 2013
Corrected hashing of UTF-8 strings.
|
Thanks. No worries about the delay, this is the beauty of Open Source that I could work with the fix in my code until you got around to merging this. I came here to remind you that any old passwords will not be verifiable with this new code, but I see you already put the warning in the readme. So, thanks for this and keep up the good work. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was an issue with hashing string formed of multi-byte characters.
Only one of the byte of the character was taken into account when
building the hash. This fix solves the situation.
Unfortunately, this makes any non-latin passwords previously hashed by
the code obsolete and totally impossible to verify. Any password containing
UTF-8 multi-byte characters hashed with the previous version of the code
cannot be verified after this change is applied.
I recommend using this patch only if you are starting up a new project and to
keep using the old code version if you don't have the capability in your
project of having both versions in parallel until all your user's passwords
are rolled-over to the new hashing algorithm.
I added a unit test to demonstrate that the new algorithm works as
expected with multi-byte characters.