Skip to content

Commit

Permalink
Bug 1610741 - Allow setting invalid URL in TRRService and show it in …
Browse files Browse the repository at this point in the history
…settings r=Gijs,fluent-reviewers,settings-reviewers,flod

- Patch changes TRRService::MaybeSetPrivateURI so that it doesn't bail
  if the URL is invalid. That could lead us into a state where the
  network.trr.uri pref has been set to some invalid URL, but we continue
  using the previous domain (or even the default TRR provider) instead.
- Updates the DoH settings UI to not set the network.trr.uri pref to "",
  as that will cause the TRRService to use the value of
  the network.trr.default_provider_uri pref instead.

Differential Revision: https://phabricator.services.mozilla.com/D174111
  • Loading branch information
valenting committed Apr 5, 2023
1 parent ea2e7d9 commit 8f92cec
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 16 deletions.
24 changes: 20 additions & 4 deletions browser/components/preferences/privacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -563,10 +563,24 @@ var gPrivacyPane = {
this.updateDoHResolverList(mode);

let customInput = document.getElementById(`${mode}InputField`);

function updateURIPref() {
if (customInput.value == "") {
// Setting the pref to empty string will make it have the default
// pref value which makes us fallback to using the default TRR
// resolver in network.trr.default_provider_uri.
// If the input is empty we set it to "(space)" which is essentially
// the same.
Services.prefs.setStringPref("network.trr.uri", " ");
} else {
Services.prefs.setStringPref("network.trr.uri", customInput.value);
}
}

menu.addEventListener("command", () => {
if (menu.value == "custom") {
customInput.hidden = false;
Services.prefs.setStringPref("network.trr.uri", customInput.value);
updateURIPref();
} else {
customInput.hidden = true;
if (
Expand All @@ -589,16 +603,18 @@ var gPrivacyPane = {

// Change the URL when you press ENTER in the input field it or loses focus
customInput.addEventListener("change", () => {
Services.prefs.setStringPref("network.trr.uri", customInput.value);
updateURIPref();
});
},

updateDoHStatus() {
async updateDoHStatus() {
let trrURI = Services.dns.currentTrrURI;
let hostname = "";
try {
hostname = new URL(trrURI).hostname;
} catch (e) {}
} catch (e) {
hostname = await document.l10n.formatValue("preferences-doh-bad-url");
}

let steering = document.getElementById("dohSteeringStatus");
steering.hidden = true;
Expand Down
3 changes: 3 additions & 0 deletions browser/locales/en-US/browser/preferences/preferences.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,9 @@ preferences-doh-status = Status: { $status }
# Variables:
# $name (string) - The name of the DNS over HTTPS resolver. If a custom resolver is used, the name will be the domain of the URL.
preferences-doh-resolver = Provider: { $name }
# This is displayed instead of $name in preferences-doh-resolver
# when the DoH URL is not a valid URL
preferences-doh-bad-url = Invalid URL
preferences-doh-steering-status = Using local provider
preferences-doh-status-active = Active
Expand Down
15 changes: 4 additions & 11 deletions netwerk/dns/TRRService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,20 +294,13 @@ bool TRRService::MaybeSetPrivateURI(const nsACString& aURI) {
clearCache = true;
}

nsAutoCString host;

nsCOMPtr<nsIURI> url;
nsresult rv = NS_MutateURI(NS_STANDARDURLMUTATOR_CONTRACTID)
.Apply(&nsIStandardURLMutator::Init,
nsIStandardURL::URLTYPE_STANDARD, 443, newURI,
nullptr, nullptr, nullptr)
.Finalize(url);
if (NS_FAILED(rv)) {
LOG(("TRRService::MaybeSetPrivateURI failed to create URI!\n"));
return false;
if (NS_SUCCEEDED(NS_NewURI(getter_AddRefs(url), newURI))) {
url->GetHost(host);
}

nsAutoCString host;
url->GetHost(host);

SetProviderDomain(host);

mPrivateURI = newURI;
Expand Down
4 changes: 3 additions & 1 deletion netwerk/dns/nsDNSService2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1499,14 +1499,16 @@ nsDNSService::GetCurrentTrrConfirmationState(uint32_t* aConfirmationState) {

NS_IMETHODIMP
nsDNSService::GetTrrDomain(nsACString& aTRRDomain) {
aTRRDomain.Truncate();
nsAutoCString url;
if (mTrrService) {
mTrrService->GetURI(url);
}
nsCOMPtr<nsIURI> uri;
nsresult rv = NS_NewURI(getter_AddRefs(uri), url);
if (NS_FAILED(rv)) {
return rv;
// An empty TRR domain in case of invalid URL.
return NS_OK;
}
return uri->GetHost(aTRRDomain);
}
Expand Down

0 comments on commit 8f92cec

Please sign in to comment.