1580046 - user-defined network.trr.excluded-domains may get overwritten

Status: RESOLVED FIXED

(Core :: Networking: DNS, defect, P1)

Description

[Nhi Nguyen (:nhi)]

From reading the code, and in preparation for bug 1578571, we should make sure that we don't overwrite user-defined values for network.trr.excluded-domains

eg.: https://searchfox.org/mozilla-central/source/netwerk/dns/TRRService.cpp#302

Comment 1

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

Since we already published the pref on support.mozilla.org I guess we should avoid changing the excluded-domains pref. I think we should add a different pref (builtin-excluded-domains) - that holds localhost, local, and maybe the captive portal domain as well?

Comment 2

[Kershaw Chang [:kershaw]]

(In reply to Nhi Nguyen (:nhi) from comment #0)

From reading the code, and in preparation for bug 1578571, we should make sure that we don't overwrite user-defined values for network.trr.excluded-domains

eg.: https://searchfox.org/mozilla-central/source/netwerk/dns/TRRService.cpp#302

We won't overwrite user-defined values. mExcludedDomains is cleared at here, but we read the preference value from network.trr.excluded-domains again and put the hosts into mExcludedDomains at here.

Valentin, could you take a look at the code regarding mExcludedDomains again? I'm afraid I might get something wrong.
And, do we really need another new pref (builtin-excluded-domains)?

Comment 3

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

I think the title may be worded ambiguously.
The worry is that at some point in the future we will want to update trr.excluded-domains to include other domains (such as the captive portal one maybe). If we change the pref in mozilla-central it will get pushed out to users, and it will overwrite their custom value, which we want to avoid.
What we want to do is to have another pref that we update instead, so we never touch the pref that might be changed by users.

Comment 4

[Nhi Nguyen (:nhi)]

From a UX point of view it is also better to have a different pref for built-in values. Right now we have to explicitly tell users to not delete the values we put there (local, localhost), and only append new values to the list.

Comment 5

[Kershaw Chang [:kershaw]]

Attachment: Bug 1580046 - Introduce new trr pref: builtin-excluded-domains

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43746856fea2
Introduce new trr pref: builtin-excluded-domains r=valentin

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/43746856fea2