Last Comment Bug 569648 - First async XHR without other network activity has null securityInfo for the channel when using auto-detect proxy
: First async XHR without other network activity has null securityInfo for the ...
Status: VERIFIED FIXED
:
Product: Toolkit Graveyard
Classification: Graveyard
Component: Security (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla1.9.3a5
Assigned To: Robert Strong [:rstrong] (use needinfo to contact me)
:
Mentors:
Depends on:
Blocks: 471889
  Show dependency treegraph
 
Reported: 2010-06-02 11:08 PDT by Robert Strong [:rstrong] (use needinfo to contact me)
Modified: 2016-06-29 13:03 PDT (History)
2 users (show)
robert.strong.bugs: in‑testsuite-
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
xpcshell test - uses an extrenal server since xpcshell doesn't support ssl (4.20 KB, text/plain)
2010-06-02 15:43 PDT, Robert Strong [:rstrong] (use needinfo to contact me)
no flags Details
patch rev1 (887 bytes, patch)
2010-06-02 20:58 PDT, Robert Strong [:rstrong] (use needinfo to contact me)
dveditz: review+
christian: approval1.9.2.7-
christian: approval1.9.2.9+
Details | Diff | Splinter Review

Description Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-02 11:08:30 PDT
Spinoff of bug 471889.

Might also affect sync.

On Windows in Firefox with a new profile:
Set Options -> Advanced -> Network -> Settings button -> Auto-detect proxy setting for network.
note: bug 471889 comment 21 states this can also happen with a PAC file.
Set Options -> General -> When Firefox starts to "Show a blank page" so no pages are opened.
Restart Firefox.
Select Help -> Check for Updates.

I'll try to come up with a test that exhibits this.
Comment 1 Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-02 14:48:47 PDT
I wasn't able to come up with a test since we need to use mochitest, the http server requires the use of a proxy, and to see the bug requires changing the proxy setting. :(
Comment 2 Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-02 15:43:28 PDT
Created attachment 448877 [details]
xpcshell test - uses an extrenal server since xpcshell doesn't support ssl

The first request fails and the second request succeeds even though they are the exact same request.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2010-06-02 20:15:38 PDT
Well, the first issue I see in that test is that the BadCertHandler fails out the attempt to replace the original load with the one for the correct proxy.  As a result, the whole load is aborted, and of course the aborted channel didn't have any security info yet...

None of that seems like a DOM problem to me, so far.  I'm just not sure where to put BadCertHandler bugs.

Does "Check for Updates" use BadCertHandler, though?
Comment 4 Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-02 20:34:02 PDT
It uses a slightly different implementation of BadCertHandler.
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/shared/CertUtils.jsm

Sorry, I filed in DOM due to other XHR bugs getting filed in DOM. If this needs to be handled by the BadCertHandler then it should go under toolkit -> security or toolkit -> app update would be fine since it is one of the consumers.

Before moving though...
In this instance there is no proxy and the client is configured to Auto-detect a proxy. I haven't verified this but I have read reports where this works fine when there is a proxy. Seems like this should be handled outside of BadCertHandler.
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2010-06-02 20:40:44 PDT
> Sorry, I filed in DOM due to other XHR bugs getting filed in DOM.

Not a problem; it's not like you knew the issue was BadCertHandler.

> In this instance there is no proxy and the client is configured to Auto-detect
> a proxy.

Right; the way necko handles that is by starting out with the assumption that there is a proxy and then doing a fake redirect to a no-proxy channel once it discovers there isn't one.  That's the redirect that BadCertHandler aborts.
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2010-06-02 20:41:11 PDT
Just as a note, that fake redirect is marked as such (has the "internal redirect" flag set).
Comment 7 Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-02 20:42:28 PDT
Thank you for the explanation! It is much appreciated
Comment 8 Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-02 20:58:42 PDT
Created attachment 448938 [details] [diff] [review]
patch rev1

Dan, there isn't a way to add a test that I can think of (see previous comments) for this regretfully.
Comment 9 Daniel Veditz [:dveditz] 2010-06-04 17:37:24 PDT
Comment on attachment 448938 [details] [diff] [review]
patch rev1

r=dveditz
Comment 10 Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-05 00:40:45 PDT
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/54b1ef730167
Comment 11 Henrik Skupin (:whimboo) 2010-06-06 16:52:05 PDT
Verified fixed with builds on OS X and Windows like Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100606 Minefield/3.7a5pre.

A Litmus test will be added on bug 471889.
Comment 12 Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-07 11:05:58 PDT
Comment on attachment 448938 [details] [diff] [review]
patch rev1

Drivers, this adds a check to the bad cert handler used by app update, blocklist, and extension manager so we don't try to validate the cert when the redirect is internal (e.g. when we try to detect if there is a proxy). There is no test due to our tests requiring a proxy and this bug happens when we try to detect a proxy when there isn't one... catch 22.
Comment 13 christian 2010-06-24 21:52:21 PDT
Comment on attachment 448938 [details] [diff] [review]
patch rev1

We will not be taking this for 3.6.6. Moving approval request forward.

If you disagree, send me an email.
Comment 14 Robert Strong [:rstrong] (use needinfo to contact me) 2010-06-24 21:54:18 PDT
Comment on attachment 448938 [details] [diff] [review]
patch rev1

I'm going to reset the request so I get the approval granted email if it is approved
Comment 15 christian 2010-08-11 20:46:32 PDT
Comment on attachment 448938 [details] [diff] [review]
patch rev1

a=LegNeato for 1.9.2.9. This needs to land by tomorrow to make it into 3.6.9
Comment 16 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-11 21:14:54 PDT
Pushed to mozilla-1.9.2
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a14d33d80d4e

Note You need to log in before you can comment on or make changes to this bug.