Closed
Bug 573115
Opened 14 years ago
Closed 14 years ago
Logging in fails for an account created with non-ascii last-character password
Categories
(Cloud Services Graveyard :: Server: Sync, defect)
Cloud Services Graveyard
Server: Sync
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Mardak, Assigned: telliott)
References
Details
Attachments
(1 file)
558 bytes,
patch
|
telliott
:
review+
|
Details | Diff | Splinter Review |
Here's the body of the POST to create the account that successfully gets created: 2010-06-18 13:13:15 Net.Resource TRACE PUT Body: {"password":"testingé","email":"edtest3@agadak.net","captcha-challenge":"02A_dq3UyEAzpRCZzzGIqUK_RfDvLKuky ELxWBPknA8RBJN9bvcttyXwMYcsMO2QMPegOJPXB4sC_E8U7zvmwOEuOQt6W86D0o7xlY2dRJVfc3AZqzaMXB29oHcKc6KY1HIYnCbIAyC4C7rKnf5PkmxMMlkTjZqE-wVFezu_cPVsZZwKNM8q9-ot_ll_DUJYrjS0CzSgacemFRoaNZyqu-wtvhGb7G39oYokUykDhikMzjVuzbRtiC_ylQTTmqBBEXxOacZNMb-EHNZENLqd9hqe2sIP8B","captcha-response":"giamo woodcut"} 2010-06-18 13:13:15 Net.Resource DEBUG PUT success 200 https://auth.services.mozilla.com/user/1.0/edtest3 Auth fails for verifying login: 2010-06-18 13:17:03 Net.Resource TRACE HTTP Header Authorization: Basic ZWR0ZXN0Mzp0ZXN0aW5n6Q== 2010-06-18 13:17:03 Net.Resource DEBUG GET fail 401 https://phx-sync129.services.mozilla.com/1.0/edtest3/info/collections atob('ZWR0ZXN0Mzp0ZXN0aW5n6Q==') == edtest3:testingé Perhaps related is that using the reset-password interface, using a password with non-ascii as the last character fails but using the client's change-password interface, things works fine.
Assignee | ||
Comment 1•14 years ago
|
||
Update here. We've definitely found a problem. We have the following function for fixing UTF-8 if(mb_detect_encoding($string, 'UTF-8,ISO-8859-1') == 'UTF-8') return $string; else return utf8_encode($string); On the sync server, if I use the password testingé, mb_detect encoding claims it is utf8 and returns testing\xe9 If I change the line to if(mb_detect_encoding($string . " ", 'UTF-8,ISO-8859-1') == 'UTF-8') it evaluates to false, at which point utf8_encode changes the string to testing\xc3\xa9 (I also note that we're not doing any utf8 detection on the password in the reg server, which we should probably be doing) Les, any thoughts?
Assignee | ||
Comment 2•14 years ago
|
||
More details: [22-Jun-2010 15:57:20] Sync received password: testing� [22-Jun-2010 15:57:20] Sync base string encoding: UTF-8 [22-Jun-2010 15:57:20] Forced utf8 encode: testingé So the first is registering as a utf8 string, but it's not the same string as the one when run through utf8 encode :P
Assignee | ||
Comment 3•14 years ago
|
||
And, if it's not at the end: [22-Jun-2010 16:03:45] Sync received password: testingéf [22-Jun-2010 16:03:45] Sync SSHA: {SSHA-256}5hgqdHMAPBY9oU5QVq3MuPmYP0QwBOrplIcfYu1sLqs/em3FNeYwLA== wtf?
Updated•14 years ago
|
Flags: blocking-fx-sync1.4?
Comment 4•14 years ago
|
||
(In reply to comment #3) > And, if it's not at the end: > > [22-Jun-2010 16:03:45] Sync received password: testingéf > [22-Jun-2010 16:03:45] Sync SSHA: > {SSHA-256}5hgqdHMAPBY9oU5QVq3MuPmYP0QwBOrplIcfYu1sLqs/em3FNeYwLA== > > wtf? Is there some difference in the PHP versions between SJC and PHX that could be causing this?
Comment 5•14 years ago
|
||
(In reply to comment #4) > Is there some difference in the PHP versions between SJC and PHX that could be > causing this? Not sure I understand how that might come into play, but... [root@wp-web01 ~]# php -v PHP 5.2.9 (cli) (built: Feb 1 2010 15:52:33) Copyright (c) 1997-2009 The PHP Group Zend Engine v2.2.0, Copyright (c) 1998-2009 Zend Technologies with eAccelerator v0.9.5.2, Copyright (c) 2004-2006 eAccelerator, by eAccelerator [root@pm-weave01 ~]# php -v PHP 5.2.9 (cli) (built: Jun 23 2009 14:49:15) Copyright (c) 1997-2009 The PHP Group Zend Engine v2.2.0, Copyright (c) 1998-2009 Zend Technologies
Comment 6•14 years ago
|
||
(In reply to comment #1) > On the sync server, if I use the password testingé, mb_detect encoding claims > it is utf8 and returns testing\xe9 Looks like \xe9 is the ISO-8859-1 encoding of "é" > If I change the line to > if(mb_detect_encoding($string . " ", 'UTF-8,ISO-8859-1') == 'UTF-8') > it evaluates to false, at which point utf8_encode changes the string to > testing\xc3\xa9 And \xc3\xa9 is the UTF-8 encoding of "é", so the bugfix here is actually to append a space in the detection call, as above. This is admittedly hacky, but it looks like the PHP function mb_detect_encoding() is itself buggy: http://www.php.net/manual/en/function.mb-detect-encoding.php#81936 > (I also note that we're not doing any utf8 detection on the password in the > reg server, which we should probably be doing) Can you point me to where you see the reg server is missing UTF8 munging on the password? It's there for account creation / modification via get_http_body() which calls fix_utf8_encoding() > Les, any thoughts? Ultimately, I think need to give up on UTF-8 detection, because in reality there's no such thing - there's only UTF-8 educated guessing, and it's fallible (and buggy in the case of PHP). Really, a client should send a header like Content-Type: application/json;charset=iso-8859-1 or Content-Type: application/json;charset=utf-8 to explicitly declare how to interpret the request.
Comment 7•14 years ago
|
||
(In reply to comment #5) > Not sure I understand how that might come into play, but... FWIW: Because point releases of PHP sometime offer unpleasant surprises. (eg. changes in method signatures, "fixed" bugs, and new bugs) But, that doesn't appear to be the case here
Comment 8•14 years ago
|
||
Attaching the patch for sync-server, but this applies to each of {sync-server,reg-server,reg-server-secure}: http://hg.mozilla.org/users/lorchard_mozilla.com/weaveserver-sync-patches/file/46f6e54238c4/bug-573115-last-char-encoding http://hg.mozilla.org/users/lorchard_mozilla.com/weaveserver-registration-patches/file/69036cae1208/bug-573115-last-char-encoding http://hg.mozilla.org/users/lorchard_mozilla.com/weaveserver-registration-admin-patches/file/531c050a347e/bug-573115-last-char-encoding
Attachment #453394 -
Flags: review?
Updated•14 years ago
|
Attachment #453394 -
Flags: review? → review?(telliott)
Comment 9•14 years ago
|
||
Or, look at these, since I just fixed the tab-indents: http://hg.mozilla.org/users/lorchard_mozilla.com/weaveserver-sync-patches/file/tip/bug-573115-last-char-encoding http://hg.mozilla.org/users/lorchard_mozilla.com/weaveserver-registration-patches/file/tip/bug-573115-last-char-encoding http://hg.mozilla.org/users/lorchard_mozilla.com/weaveserver-registration-admin-patches/file/tip/bug-573115-last-char-encoding
Assignee | ||
Comment 10•14 years ago
|
||
Comment on attachment 453394 [details] [diff] [review] Hacky bugfix for buggy mb_detect_encoding I was afraid of this :)
Attachment #453394 -
Flags: review?(telliott) → review+
Assignee | ||
Comment 11•14 years ago
|
||
http://hg.mozilla.org/services/reg-server-secure/rev/a5d54cb6a31d http://hg.mozilla.org/services/reg-server/rev/ef47e083c35b http://hg.mozilla.org/services/sync-server/rev/7d3551d57958
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 12•14 years ago
|
||
https://stage-auth.services.mozilla.com (really dm-weave-stage-reg01 and -adm01) have been updated to tip.
Comment 13•14 years ago
|
||
So, we never pushed this to prod? Can we please push this as soon as possible? And why did we resolve it fixed before it was actually deployed in production?
Comment 14•14 years ago
|
||
Process should be FIXED>push to stage>VERIFIED>push to prod. Never VERIFIED, so never pushed. We did a push to reg and sreg yesterday. There are a LOT of changes on sync since we did a push there, so I'm not so gung-ho to just grab tip. Will look at diffs and figure it out now.
Comment 15•14 years ago
|
||
We don't have a production branch, so I pushed 7d3551d57958 to production. Someone VERIFY for me?
Updated•14 years ago
|
Flags: in-testsuite?
Updated•13 years ago
|
Flags: blocking-fx-sync1.4?
Updated•1 year ago
|
Product: Cloud Services → Cloud Services Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•