First async XHR without other network activity has null securityInfo for the channel when using auto-detect proxy

VERIFIED FIXED in mozilla1.9.3a5

Status

Toolkit Graveyard
Security
VERIFIED FIXED
7 years ago
11 months ago

People

(Reporter: rstrong, Assigned: rstrong)

Tracking

unspecified
mozilla1.9.3a5
Bug Flags:
in-testsuite -

Details

Attachments

(2 attachments)

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.
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. :(
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.
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?
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.
> 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.
Component: DOM → Security
Product: Core → Toolkit
QA Contact: general → toolkit
Just as a note, that fake redirect is marked as such (has the "internal redirect" flag set).
Thank you for the explanation! It is much appreciated
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.
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Attachment #448938 - Flags: review?(dveditz)
Comment on attachment 448938 [details] [diff] [review]
patch rev1

r=dveditz
Attachment #448938 - Flags: review?(dveditz) → review+
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/54b1ef730167
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
OS: Windows 7 → All
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.3a5
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.
Status: RESOLVED → VERIFIED
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.
Attachment #448938 - Flags: approval1.9.2.5?

Comment 13

7 years ago
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.
Attachment #448938 - Flags: approval1.9.2.7?
Attachment #448938 - Flags: approval1.9.2.6-
Attachment #448938 - Flags: approval1.9.2.5?
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
Attachment #448938 - Flags: approval1.9.2.7?
Attachment #448938 - Flags: approval1.9.2.7?

Comment 15

7 years ago
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
Attachment #448938 - Flags: approval1.9.2.9? → approval1.9.2.9+
Pushed to mozilla-1.9.2
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a14d33d80d4e
status1.9.2: --- → .9-fixed
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.