shaneMangudi / bcrypt-nodejs

Native implementation of bcrypt for NodeJS
Other
574 stars 69 forks source link

Corrected hashing of UTF-8 strings. #4

Closed NicolasPelletier closed 11 years ago

NicolasPelletier commented 11 years ago

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.

shaneMangudi commented 11 years ago

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 ?

NicolasPelletier commented 11 years ago

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.