Closed Bug 1336069 Opened 3 years ago Closed 3 years ago

network.proxy.autoconfig_url.include_path=true does not work

Categories

(Core :: Networking, defect)

51 Branch
x86
Windows 10
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla54
Tracking Status
firefox-esr45 --- unaffected
firefox51 --- wontfix
firefox52 + verified
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- verified

People

(Reporter: alice0775, Assigned: bagder)

References

Details

(Keywords: regression, Whiteboard: [necko-active])

Attachments

(1 file)

Steps To Reproduce:
1. save proxy.pac file with the following contents

function FindProxyForURL(url, host) { 
alert(url);
return "DIRECT";
}

2. Set network.proxy.autoconfig_url.include_path=true in about:config
3. about:preferences#advanced > Network > Settings...
   turn on "Automatic proxy configuration URL:" 
    file:///C:/path/to/proxy.pac

4. Open Browser Console
5. Load any page (e.g. http://forums.mozillazine.org/viewforum.php?f=38 )
   ---- observe log

Actual Results:
network.proxy.autoconfig_url.include_path=true does not work.
The url is stripped.
PAC-alert: http://forums.mozillazine.org/

Expected Results:
network.proxy.autoconfig_url.include_path=true should work.
The following should be logged.
PAC-alert: http://forums.mozillazine.org/viewforum.php?f=38
Daniel, is this a regression from recent changes?
Assignee: nobody → daniel
Whiteboard: [necko-active]
[Tracking Requested - why for this release]: it is worth to fix the issue for next ESR 52

Regressed by Bug 1255474
Keywords: regression
When toggling this option, you need to restart Firefox for it to "take effect". Did you do that?
Flags: needinfo?(alice0775)
(In reply to Daniel Stenberg [:bagder] from comment #3)
> When toggling this option, you need to restart Firefox for it to "take
> effect". Did you do that?

Yes, I restart Firefox.
Flags: needinfo?(alice0775)
Ugh. I think I spotted the mistake. I'll try a patch and report back soonish.
This tiny change fixes the problem for me. The initialization of the class member in the Init() method was accidentally put after the code path that normally returns early so it remained with its default value (false) all the time!
Attachment #8835900 - Flags: review?(mcmanus)
Comment on attachment 8835900 [details] [diff] [review]
0001-bug-1336069-init-class-member-before-return-r-mcmanu.patch

Review of attachment 8835900 [details] [diff] [review]:
-----------------------------------------------------------------

doh!
Attachment #8835900 - Flags: review?(mcmanus) → review+
Keywords: checkin-needed
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a2f028e6943
init class member before return, r=mcmanus
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8a2f028e6943
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Please nominate for aurora/beta uplift when you get a chance.
Flags: needinfo?(daniel)
Comment on attachment 8835900 [details] [diff] [review]
0001-bug-1336069-init-class-member-before-return-r-mcmanu.patch

Approval Request Comment
[Feature/Bug causing the regression]: 1336069
[User impact if declined]: "network.proxy.autoconfig_url.include_path" won't work as documented
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: the change is minuscule, easy to review and in code not frequently changed
[String changes made/needed]: N/A
Flags: needinfo?(daniel)
Attachment #8835900 - Flags: approval-mozilla-beta?
Attachment #8835900 - Flags: approval-mozilla-aurora?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
Verified as fixed on latest Nightly 54.0a1, Build ID 20170215030342 using Windows 10.
Status: RESOLVED → VERIFIED
Flags: needinfo?(brindusa.tot)
Comment on attachment 8835900 [details] [diff] [review]
0001-bug-1336069-init-class-member-before-return-r-mcmanu.patch

let's take this regression fix in aurora53 and beta52

Should be in 52.0b7
Attachment #8835900 - Flags: approval-mozilla-beta?
Attachment #8835900 - Flags: approval-mozilla-beta+
Attachment #8835900 - Flags: approval-mozilla-aurora?
Attachment #8835900 - Flags: approval-mozilla-aurora+
Let's check this on Beta 52 as well.
Flags: qe-verify+
Reproduced the initial issue using Firefox 51.0.1 on Windows 7 32bit, verified that this is fixed using Firefox 52 beta 8 on Windows 7 x32, Ubuntu 12.04 x32 and macOS 10.12.3.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.