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

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Alice0775 White]

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

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Daniel, is this a regression from recent changes?

Comment 2

[Alice0775 White]

[Tracking Requested - why for this release]: it is worth to fix the issue for next ESR 52

Regressed by Bug 1255474

Comment 3

[Daniel Stenberg [:bagder]]

When toggling this option, you need to restart Firefox for it to "take effect". Did you do that?

Comment 4

[Alice0775 White]

(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.

Comment 5

[Daniel Stenberg [:bagder]]

Ugh. I think I spotted the mistake. I'll try a patch and report back soonish.

Comment 6

[Daniel Stenberg [:bagder]]

Attachment: 0001-bug-1336069-init-class-member-before-return-r-mcmanu.patch

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!

Comment 7

[Patrick McManus [: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!

Comment 8

[Daniel Stenberg [:bagder]]

Green try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=22212a6da202

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a2f028e6943
init class member before return, r=mcmanus

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8a2f028e6943

Comment 11

[Julien Cristau [:jcristau]]

Please nominate for aurora/beta uplift when you get a chance.

Comment 12

[Daniel Stenberg [:bagder]]

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

Comment 13

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 14

[Brindusa Tot, Desktop QA]

Verified as fixed on latest Nightly 54.0a1, Build ID 20170215030342 using Windows 10.

Comment 15

[Julien Cristau [:jcristau]]

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

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f7a276a267cd

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/23b17b9d6a4e

Comment 18

[Andrei Vaida [:avaida]]

Let's check this on Beta 52 as well.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/23b17b9d6a4e

Comment 20

[Bogdan Maris, Desktop QA]

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.