833947 - System Proxy Settings fails for PAC URL without trailing /

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Geoff Morrison]

User Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20130116073211

Steps to reproduce:

Updated from FF17.0.1 to FF18. FF was configured to "Use system proxy settings". The system proxy is a PAC file at "http://<url>". After the update, the "checking for new versions of plugins" dialog opened but made no progress. Clicking Cancel opened my Firefox windows from my previous session, but they could not connect to sites on either the intranet or internet (the page shows no errors, but stays in a "loading" state forever).

This problem continues to exist in FF 18.0.1.


Actual results:

Configuring the PAC file manually in FF18 ("Automatic proxy configuration URL:" set to "http://<url>") restored connectivity, both internal and external. After looking at bug 828202 and bug 815783, I noticed that Firefox added a trailing "/" to the PAC URL as I closed the proxy configuration window.

I changed Firefox back to "Use system proxy settings", which again caused a loss of connectivity (loading pages takes forever). I then added a trailing "/" to my system PAC URL, which restored connectivity.


Expected results:

The PAC URL normalization that applies to URLs entered directly into the FF proxy configuration screen should also apply to URLs loaded from the system proxy settings.

Comment 1

[Patrick McManus [:mcmanus]]

Geoff, thanks for the report and testing 18.0.1

please provide the urls you used (both the working and not working). Thanks.

If you don't want to include them in the bug email them to me at [e-mail]

Comment 2

[Patrick McManus [:mcmanus]]

from email we can say the non working pac url is a system proxy url of the form

http://blah.domain.com  [no /]

and a request for "/" at that site returns a 302 pointing to a more traditional pac url with a query param like
http://blah.domain.com/foo.pac?queryinfo

Comment 3

[Patrick McManus [:mcmanus]]

crud.. this is more confusion related to not having threadsafe nsiuri.

isPacURI needs to be able to identify the spec of non-normalized uris for use in checking the uri from system proxy settings (looking for changes), and also for use in checking whether or not a network load is for the pac uri (so it can go direct) which uses the normalized version.. the latter is where this is falling down - http://blah.domain.com is normalized to http://blah.domain.com/

Comment 4

[Patrick McManus [:mcmanus]]

Attachment: patch 0

this whole thing can probably be rethought and replaced with a transaction flag somewhere.. but that's not a suitable backport so let's just fix it up at the moment.

Comment 5

[Patrick McManus [:mcmanus]]

Comment on attachment 705915 [details] [diff] [review]
patch 0

biesi r? timeout resched

Comment 6

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7836f6575a77

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7836f6575a77

Comment 9

[Jason Duell]

Comment on attachment 705915 [details] [diff] [review]
patch 0

[Approval Request Comment]
Bug caused by (feature/regressing bug #): I'm not sure (Patrick, do you know?), but it's a Firefox 17->18 regression.
User impact if declined: some proxy users can't connect to internet until they figure out they need to add a / to their proxy URL.  So small population but bad for them.
Testing completed (on m-c, etc.): has baked for a while.
Risk to taking this patch (and alternatives if risky):  low.
String or UUID changes made by this patch: none

Comment 10

[Jason Duell]

Comment on attachment 705915 [details] [diff] [review]
patch 0

This is already in aurora.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Since this already shipped in 18/19 we'd rather let this wait and ride the trains (let others workaround by fixing their PACs) rather than take any risk in an uplift.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 705915 [details] [diff] [review]
patch 0

oops - wrong flag, correcting to minus.

Comment 13

[Michael Schmitt]

If I'm reading this correctly, in THIS case:
 - If PAC URL is typed into the Firefox dialog box, it normalizes it and saves it. That is, it is corrected at the time it is entered.
 - This bug case is dealing with the situation where Firefox is to use a system proxy setting, which isn't normallized.
 
FYI, the situation I had (reported in bug: 843088) is a third situation: the PAC URL was poked into Firefox via the Proxy Selector add-on.  The PAC is not normalized in the Proxy Selector settings, and that's how it gets saved that way in Firefox.
 
So, the fix needs to normalize the PAC URL at the time it loads it, regardless of how it acquires the URL.  That is, it should not depend on FF modifying it at the time it is entered into the options dialog.