1354245 - Firefox ignore the network.captive-portal-service.enabled preference, polls anyway

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Curt Self]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170404004003

Steps to reproduce:

No active steps are required to reproduce other than setting this preference in about:config
network.captive-portal-service.enabled = false


Actual results:

Firefox continues to poll the URL listed under captivedetect.canonicalURL regardless of the enabled preference being set to true or false.


Expected results:

Expected results are that the preference is not ignored and the polling does not occur.

Comment 1

[YF (Yang)]

WFM in 54.0a2 20170406004017 on Win10.

Comment 2

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

Hi Curt,

I am unable to reproduce your findings. Could you assist us with the process?
Go to about:networking, into the Logging section, and set the Log modules to 

timestamp,sync,nsHttp:5,nsHostResolver:5,CaptivePortalService:5

Then click start logging and reproduce your bug, turning the pref off, and checking if it still polls the canonicalURL
How are you watching the requests? Wireshark?

Comment 3

[Curt Self]

Attachment: log.txt-main.8380

(In reply to Valentin Gosu [:valentin] from comment #2)
> Hi Curt,
> 
> I am unable to reproduce your findings. Could you assist us with the process?
> Go to about:networking, into the Logging section, and set the Log modules to 
> 
> timestamp,sync,nsHttp:5,nsHostResolver:5,CaptivePortalService:5
> 
> Then click start logging and reproduce your bug, turning the pref off, and
> checking if it still polls the canonicalURL
> How are you watching the requests? Wireshark?

I use Burp Suite as a part of my development process (Web Applications Dev), that is where I see the request.
I enabled network logging and went to lunch. When I got back I saw that one request was logged by burp: http://detectportal.firefox.com/success.txt
I am attaching the log file that was created.

Comment 4

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

Hi Curt, I have found the issue.
We disable the captive portal service when the network goes down, and enable it again when it comes back up. Turns out we didn't check the pref before enabling it there.
Thanks a lot for reporting this bug!

Comment 5

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

Attachment: Bug 1354245 - Don't call CaptivePortalService::Start() in SetConnectivityInternal if the captive portal service is disabled via pref

Review commit: https://reviewboard.mozilla.org/r/129468/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129468/

Comment 6

[Patrick McManus [:mcmanus]]

Comment on attachment 8857511 [details]
Bug 1354245 - Don't call CaptivePortalService::Start() in SetConnectivityInternal if the captive portal service is disabled via pref

https://reviewboard.mozilla.org/r/129468/#review132024

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5616b65b694
Don't call CaptivePortalService::Start() in SetConnectivityInternal if the captive portal service is disabled via pref r=mcmanus

Comment 8

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

Comment on attachment 8857511 [details]
Bug 1354245 - Don't call CaptivePortalService::Start() in SetConnectivityInternal if the captive portal service is disabled via pref

Approval Request Comment
[Feature/Bug causing the regression]: bug 1320087
[User impact if declined]: browser will check captive portal and make connections against user preference.
[Is this code covered by automated tests?]: no.
[Has the fix been verified in Nightly?]: just landed.
[Needs manual test from QE? If yes, steps to reproduce]: 

Yes.
Set network.captive-portal-service.enabled to false.
Disconnect from network (It is important that only loopback interface is up)
Reconnect to network.
Check that no requests to http://detectportal.firefox.com/success.txt are made for network.captive-portal-service.maxInterval seconds.
Since maxInterval can be quite high, I advise setting the minInterval and maxInterval prefs to low values, such as 10000 (10 seconds) and 20000 (20 seconds).

[List of other uplifts needed for the feature/fix]: none.
[Is the change risky?]: no.
[Why is the change risky/not risky?]: It is a very small and easy fix.
[String changes made/needed]: none.

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

This issue has been around since 52, do you feel it is worth the very last minute uplift to 53? We are very close to release and I'm going to say wontfix for 53 for now. Let me know if you disagree strongly.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f5616b65b694

Comment 11

[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 12

[ovidiu boca[:Ovidiu]]

Hi Valentin,

Can you please tell me how to I perform the "Check that no requests to http://detectportal.firefox.com/success.txt are made for network.captive-portal-service.maxInterval seconds"?

Comment 13

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

(In reply to ovidiu boca[:Ovidiu] from comment #12)
> Hi Valentin,
> 
> Can you please tell me how to I perform the "Check that no requests to
> http://detectportal.firefox.com/success.txt are made for
> network.captive-portal-service.maxInterval seconds"?

I suggest using Wireshark. You should see a HTTP request before you turn the pref to off, afterwards you shouldn't see any other requests to the same IP.

Comment 14

[ovidiu boca[:Ovidiu]]

I tested this on Mac OS X 10.12 with FF Nightly 55.0a1(2017-04-13), please see the steps that I performed to verify this:

    1.Disconnect from any network
    2.First I set the "network.captive-portal-service.maxInterval" value=10000
    3.Start the WireShark
    4.I set the network.captive-portal-service.enabled to false
    5.Reconnect to network (Wi-Fi)

I don't see any HTTP request to http://detectportal.firefox.com/success.txt, if I set the network.captive-portal-service.enabled to true I can see the HTTP request to http://detectportal.firefox.com/success.txt.  

Please tell me if you consider this steps to be correct in order to mark this bug as verified. Thank you

Comment 15

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

Yes, Those steps are correct. Thanks!

Comment 16

[Julien Cristau [:jcristau]]

Comment on attachment 8857511 [details]
Bug 1354245 - Don't call CaptivePortalService::Start() in SetConnectivityInternal if the captive portal service is disabled via pref

honor captive portal pref in connectivity check, aurora54+.  Too late for 53.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[Bogdan Maris, Desktop Test Engineering]

I reproduced the initial issue using old Aurora from 2017-04-06 on Windows 10 with the help of Microsoft Message Analyzer and Wireshark. 
With the help of instructions from comment 14, I could verify that the HTTP request does not appear when setting pref 'network.captive-portal-service.enabled' to 'false' using Firefox 54 beta 10 across platforms (Windows 10 64bit, macOS 10.12.4 and Ubuntu 16.04 64bit).

Comment 20

[Julien Cristau [:jcristau]]

This bug came up on the enterprise mailing list.  Is this something we'd want to fix in esr52?

Comment 21

[cawoodm]

I would say this is not fixed. I'm using FFv55 (win10 64) and it's got network.captive-portal-service.minInterval = 60000 and it's pinging http://detectportal.firefox.com/success.txt every second.

Comment 22

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

If you have issues with this feature, please open another bug.

Comment 23

[Arno]

Hi,

Still traffic (unwanted) to detectportal.firefox.com

Settings:
network.captive-portal-service.enabled set to false
captivedetect.canonicalURL set to ""

OS: Win7 32bit
FF: 61.0.1
Language OS/FF: Dutch (is this relevant for non UI bugs?)

Can I reopen this bug or create a new one?

Comment 24

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

(In reply to Arno from comment #23)
> Can I reopen this bug or create a new one?

Please create a new one. Thanks!