search

sstrigler / JSJaC

JavaScript Jabber Client Library
Other
295 stars 86 forks source link

Fix for #56 + crypt.js update #57

Closed valeriansaliou closed 11 years ago

rraptorr commented 11 years ago

Unfortunately this patch breaks DIGEST-MD5 completely. Updated jshash removes functions binb2b64 and str2binb which are used inside _doSASLAuthDigestMd5S1(). Fixing it should be quite simple though.

valeriansaliou commented 11 years ago

@rraptorr fixed, sorry.

valeriansaliou commented 11 years ago

@gavllew fixed also

rraptorr commented 11 years ago

Any particular reason you changed b64pad from "=" to ""?

valeriansaliou commented 11 years ago

b64pad = "" in both v2.2 md5 & sha1 libs from http://pajhome.org.uk/crypt/md5/scripts.html

I had to upgrade from v2.1 to v2.2 to fix the UTF-8 chars coding issue which prevented usernames with accents to login using JSJaC.

rraptorr commented 11 years ago

I am aware that it is from jshash and I am aware of the issue. But why have you changed it from "=" to ""?

valeriansaliou commented 11 years ago

I did not change it, it has been changed by the author of jshash itself. This var belongs to both md5.js and sha1.js, not JSJaC itself.

rraptorr commented 11 years ago

And have you verified that now all the 'b64' functions, used in several places in JSJaC code, return the same values as previously? Anyway, b64pad is a configuration variable for jshash that was explicitly set by JSJaC author to "=". You have changed it to "". It was not changed by jshash author, you can check it easily by looking at original jshash source.

valeriansaliou commented 11 years ago

You're right, value has been changed for JSJaC to comply with RFC. I'll make changes, then, and review b64 functions too.

valeriansaliou commented 11 years ago

'b64' are not used anywhere else than in JSJaCConnection.js, which have been updated to use newer jshash functions.

b64pad var has been set back to "=".

rraptorr commented 11 years ago

It is used in JSJaCHttpPollingConnection.js. However, I believe that setting b64pad to "=" should be enough.

valeriansaliou commented 11 years ago

Yeah it's used in JSJaCHttpPollingConnection.js too.

I tested the old one and the new one, using the value '123' and both old and new b64_sha1() return me "QL0AFWMIX8NRZTKeof9cXsvbvu8=" so that's fine. The only difference is that now it's considering UTF-8 chars, so it's working fine with accents (returning the correct hash) in comparison to v2.1 which returned invalid hashes when the passed string contained accents.

rraptorr commented 11 years ago

As said, I believe that setting b64pad back to "=" should be enough:)

valeriansaliou commented 11 years ago

Great :)

sstrigler commented 11 years ago

Thanks so much guys! <3 Great job!