359824 - necko unit test failure: test_authentication.js

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Dave Liebreich [:davel]]

test has been failing since 2006-11-06 08:50

Comment 1

[:Gavin Sharp [email: [e-mail]]]

This is due to the patch for bug 223846, though I'm not sure why.
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaExperimental/1162831860.21562.gz

Comment 2

[Adam Guthrie]

*** Bug 359981 has been marked as a duplicate of this bug. ***

Comment 3

[Adam Guthrie]

The "problem" is in nsMIMEHeaderParamImpl::GetParameterInternal. We pass "Basic realm="secret"" in as the header value, and it expects things to be in the form of "<token>=<token or quoted string>". 

It worked before because we were just calling strstr() on the challenge string and that got us where we needed to be to parse the value.

So the fix is probably to tidy up the challenge string before it's passed into GetParameter, I guess.

Comment 4

[Adam Guthrie]

Attachment: Be sure to pass GetParameter the right thing v1.0

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 245056 [details] [diff] [review]
Be sure to pass GetParameter the right thing v1.0

hm, come to think of it, I'm not sure that you can use nsIMIMEHeaderParam for this header... it separates the parameters with a comma rather than a semicolon

Comment 6

[Adam Guthrie]

Comment on attachment 245056 [details] [diff] [review]
Be sure to pass GetParameter the right thing v1.0

I'm just attempting to fix bustage. If using nsIMIMEHeaderParam isn't going to work, I think we need to back out the patch for bug 223846.

Comment 7

[Jeff Walden [:Waldo]]

Attachment: Back out bug 223846

Even ignoring the issue of where the string passed to ParseRealm begins, the patch for bug 223846 was wrong, and it won't handle cases where realm is just one of several key-value pairs in a comma-separated list (particularly if realm isn't the first item in the list), which per RFC 2617 is how the syntax of WWW-Authenticate is structured:

  challenge   = auth-scheme 1*SP 1#auth-param

It should be possible to fix this by adding another method to nsIMIMEHeaderParam to parse comma-separated key-value pair lists, probably by converting the code in GetParameterInternal to a new private method which takes either ';' or ',' as one of its arguments and which GetParameterInternal and the new method would call, but I'd rather not spend that much time on this at the moment.  Someone else who cares more can make that fix, which should be slightly tedious but not particularly difficult.

Comment 8

[(not reading, please use [e-mail] instead)]

I think we want to back this out for 1.9a1, as this prevents people from logging into sites like http://intranet.mozilla.org, see bug #361544

I backed out bug #223846 (thanks waldo for the patch) and now I can log in.

Comment 9

[(not reading, please use [e-mail] instead)]

I think we should take waldo's back out for 1.9a1

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yes, please back out ASAP -- shouldn't need reviews for the backout (or can r=me for it)

Comment 11

[(not reading, please use [e-mail] instead)]

Comment on attachment 246259 [details] [diff] [review]
Back out bug 223846

r=vlad,sspitzer with a verbal "good idea" from g#.

I've backed out the other fix.

Comment 12

[(not reading, please use [e-mail] instead)]

fixed.

Comment 13

[Jeff Walden [:Waldo]]

For the record, the backout omitted the removal of the nsIMIMEHeaderParam.h #include added in bug 223846 (and included in the backout patch posted here); I just removed the #include on trunk.