Closed
Bug 829646
Opened 12 years ago
Closed 12 years ago
Proxy Auto Config - calling myIPAddress() in the top level (outside of FindProxyForURL) fails
Categories
(Core :: Networking: HTTP, defect)
Tracking
()
VERIFIED
FIXED
mozilla21
People
(Reporter: jdrosa, Assigned: mcmanus)
References
Details
(Keywords: regression)
Attachments
(2 files)
15.76 KB,
text/plain
|
Details | |
3.07 KB,
patch
|
Biesinger
:
review+
bajaj
:
approval-mozilla-aurora+
bajaj
:
approval-mozilla-beta+
bajaj
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
User agent: Mozilla/5.0 (Windows NT 6.1; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20130104151925
After automatically being upgraded to Firefox 18 from Firefox 17, all connectivity to internet sites, while being connected to an intranet, was lost. My browsers (I use Firefox 99% of the time, but am forced to use IE on occasion) are all configured to use a Proxy Auto Config (PAC) script to determine how to access internal vs. external websites.
Within Firefox, I had the PAC configured in the "Automatic proxy configuration URL" field. I then changed my Firefox configuration to the "Use system proxy settings," with no change in behavior. I could access intranet sites, but received the "Server not found" response whenever attempting to connect to external sites.
I then manually configured the proxy settings to force the use of the proxy server and I regained access to external sites, but then lost connectivity to internal sites.
I then placed a copy of the PAC on my local hard drive and reconfigured Firefox to use the PAC on my local hard drive. Once again, no external connectivity.
All the while I was able to use IE and Chrome (after downloading it, I hate IE) to access internal and external sites using the non-local PAC URL.
Attached is a copy of the PAC.
Assignee | ||
Comment 1•12 years ago
|
||
The problem is
var index = myIpAddress().lastIndexOf(".") + 1;
var myiparray = myIpAddress().split(".");
when myIPAddress() is executed outside of the context of calling FindProxyForURL() (in this case during pac file load time) it fails.
that's my bug. sorry.
your workaround is just to move those global lines into FindProxyForURL() and life should be good.
Depends on: 769764
Assignee | ||
Comment 2•12 years ago
|
||
one other note - myIPAddress() is actually somewhat dependent on the host being looked up in FindProxyForURL().. in the case you have more than one IP address it will return the one that would be used to directly connect to that host... that's where the conflict came in.
I'll make it deal with the discrepancy, but you'll get a more accurate answer if your pac code scopes myIPAddress inside FindProxyForURL()
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #701212 -
Flags: review?(cbiesinger)
Assignee | ||
Updated•12 years ago
|
status-firefox18:
--- → affected
status-firefox19:
--- → affected
status-firefox20:
--- → affected
status-firefox21:
--- → affected
status-firefox-esr17:
--- → unaffected
Assignee | ||
Updated•12 years ago
|
Component: Preferences → Networking: HTTP
Product: Firefox → Core
Updated•12 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Firefox 18 - Proxy Auto Config Script - Not Working → Proxy Auto Config - calling myIPAddress() in the top level (outside of FindProxyForURL) fails
Updated•12 years ago
|
Blocks: 769764
tracking-firefox18:
--- → ?
tracking-firefox19:
--- → ?
tracking-firefox20:
--- → ?
No longer depends on: 769764
Keywords: regression
Updated•12 years ago
|
Attachment #701212 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Comment 4•12 years ago
|
||
I don't own the PAC script, but I will pass that recommendation on to the people that do.
Thanks for looking into this. I really hate using IE and Chrome just irritates me, so not having FF to use just ruins my day.
(In reply to Patrick McManus [:mcmanus] from comment #2)
> one other note - myIPAddress() is actually somewhat dependent on the host
> being looked up in FindProxyForURL().. in the case you have more than one IP
> address it will return the one that would be used to directly connect to
> that host... that's where the conflict came in.
>
> I'll make it deal with the discrepancy, but you'll get a more accurate
> answer if your pac code scopes myIPAddress inside FindProxyForURL()
Comment 6•12 years ago
|
||
Assignee: nobody → mcmanus
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment 7•12 years ago
|
||
Patrick - please help us understand the likelihood that this is a commonly occurring bug for proxy users on FF18, and the risk of regression if uplifted.
Please also prepare uplifts to FF19/20 asap.
Updated•12 years ago
|
Flags: needinfo?(mcmanus)
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #7)
> Patrick - please help us understand the likelihood that this is a commonly
> occurring bug for proxy users on FF18, and the risk of regression if
> uplifted.
>
A PAC file is an arbitrary piece of javascript and this bug covers one such file.. obviously there an infinite number of possible pac configurations - its really hard to know how common this would be other than to say
* only 1 person has reported the problem
* the issue is with the global context and anecdotally most people just define 1 function (the required FindProxyForURL()) in their pac. But because its an arbitrary script its really hard to say
> Please also prepare uplifts to FF19/20 asap.
its the first thing I'll do after dropping my kid at school this morning. The fix is very eay to reason about in terms of safety.
Flags: needinfo?(mcmanus)
Assignee | ||
Comment 9•12 years ago
|
||
Comment on attachment 701212 [details] [diff] [review]
patch 0
[Approval Request Comment]
Regression caused by (bug #): 769764
User impact if declined: myIpAddress() PAC helper function fails when used in top lovel scope. User can work around it if and only if they control the contents of the PAC file they are using.
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low - small fix.
Attachment #701212 -
Flags: approval-mozilla-release?
Attachment #701212 -
Flags: approval-mozilla-beta?
Attachment #701212 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Updated•12 years ago
|
Attachment #701212 -
Flags: approval-mozilla-beta?
Attachment #701212 -
Flags: approval-mozilla-beta+
Attachment #701212 -
Flags: approval-mozilla-aurora?
Attachment #701212 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 10•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Updated•12 years ago
|
Attachment #701212 -
Flags: approval-mozilla-release? → approval-mozilla-release+
Assignee | ||
Comment 11•12 years ago
|
||
Updated•12 years ago
|
Comment 12•12 years ago
|
||
Checked in 18.0 on Mac OS X using the pac file here, editing the proxy addresses with a different proxy. It didn't work in 18.0. It works with 18.0.1.
Keywords: verifyme
Comment 14•12 years ago
|
||
There is an automated test in the patch that verifies this isssue, test_protocolproxyservice.js, and it passes on all platforms on beta and release channels.
Beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=18881178&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=18877136&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=18878399&full=1&branch=mozilla-beta
Release
https://tbpl.mozilla.org/php/getParsedLog.php?id=18845657&full=1&branch=mozilla-release
https://tbpl.mozilla.org/php/getParsedLog.php?id=18843941&full=1&branch=mozilla-release
https://tbpl.mozilla.org/php/getParsedLog.php?id=18848657&full=1&branch=mozilla-release
Status: RESOLVED → VERIFIED
Comment 15•12 years ago
|
||
This issue is verified on Firefox 19 beta 6 (Build ID: 20130212082553), on the next machines:
1) Ubuntu 12.04 64-bit
2) Windows 7 64-bit
3) Windows 8 32-bit
4) Mac OSX 10.6.8
I've verified it by changing all the proxies from the end of the attached pac file with 81.196.129.33:8080 and it works for me.
You need to log in
before you can comment on or make changes to this bug.
Description
•