diff --git a/netwerk/dns/TRRService.cpp b/netwerk/dns/TRRService.cpp index 5fe3155345de..5e8027b45d0e 100644 --- a/netwerk/dns/TRRService.cpp +++ b/netwerk/dns/TRRService.cpp @@ -125,7 +125,7 @@ void TRRService::GetPrefBranch(nsIPrefBranch** result) { nsresult TRRService::ReadPrefs(const char* name) { MOZ_ASSERT(NS_IsMainThread(), "wrong thread"); if (!name || !strcmp(name, TRR_PREF("mode"))) { - // 0 - off, 1 - reserved, 2 - TRR first, 3 - TRR only, 4 - shadow, + // 0 - off, 1 - reserved, 2 - TRR first, 3 - TRR only, 4 - reserved, // 5 - explicit off uint32_t tmp; if (NS_SUCCEEDED(Preferences::GetUint(TRR_PREF("mode"), &tmp))) { @@ -135,6 +135,9 @@ nsresult TRRService::ReadPrefs(const char* name) { if (tmp == MODE_RESERVED1) { tmp = MODE_TRROFF; } + if (tmp == MODE_RESERVED4) { + tmp = MODE_TRROFF; + } mMode = tmp; } } diff --git a/netwerk/dns/nsHostResolver.cpp b/netwerk/dns/nsHostResolver.cpp index 4197c5ebf6fe..9992e69e11ea 100644 --- a/netwerk/dns/nsHostResolver.cpp +++ b/netwerk/dns/nsHostResolver.cpp @@ -442,44 +442,7 @@ void AddrHostRecord::ResolveComplete() { } } - if (mTRRUsed && mNativeUsed && mNativeSuccess && - mTRRSuccess) { // race or shadow! - static const TimeDuration k50ms = TimeDuration::FromMilliseconds(50); - static const TimeDuration k100ms = TimeDuration::FromMilliseconds(100); - if (mTrrDuration <= mNativeDuration) { - if ((mNativeDuration - mTrrDuration) > k100ms) { - AccumulateCategorical(Telemetry::LABELS_DNS_TRR_RACE2::TRRFasterBy100); - } else if ((mNativeDuration - mTrrDuration) > k50ms) { - AccumulateCategorical(Telemetry::LABELS_DNS_TRR_RACE2::TRRFasterBy50); - } else { - AccumulateCategorical(Telemetry::LABELS_DNS_TRR_RACE2::TRRFaster); - } - LOG(("nsHostRecord::Complete %s Dns Race: TRR\n", host.get())); - } else { - if ((mTrrDuration - mNativeDuration) > k100ms) { - AccumulateCategorical( - Telemetry::LABELS_DNS_TRR_RACE2::NativeFasterBy100); - } else if ((mTrrDuration - mNativeDuration) > k50ms) { - AccumulateCategorical( - Telemetry::LABELS_DNS_TRR_RACE2::NativeFasterBy50); - } else { - AccumulateCategorical(Telemetry::LABELS_DNS_TRR_RACE2::NativeFaster); - } - LOG(("nsHostRecord::Complete %s Dns Race: NATIVE\n", host.get())); - } - } - - if (mTRRUsed && mNativeUsed && ((mResolverMode == MODE_SHADOW))) { - // both were used, accumulate comparative success - AccumulateCategorical( - mNativeSuccess && mTRRSuccess - ? Telemetry::LABELS_DNS_TRR_COMPARE::BothWorked - : ((mNativeSuccess - ? Telemetry::LABELS_DNS_TRR_COMPARE::NativeWorked - : (mTRRSuccess - ? Telemetry::LABELS_DNS_TRR_COMPARE::TRRWorked - : Telemetry::LABELS_DNS_TRR_COMPARE::BothFailed)))); - } else if (mResolverMode == MODE_TRRFIRST) { + if (mResolverMode == MODE_TRRFIRST) { if (flags & nsIDNSService::RESOLVE_DISABLE_TRR) { // TRR is disabled on request, which is a next-level back-off method. Telemetry::Accumulate(Telemetry::DNS_TRR_DISABLED, mNativeSuccess); @@ -510,12 +473,12 @@ void AddrHostRecord::ResolveComplete() { case MODE_TRRONLY: AccumulateCategorical(Telemetry::LABELS_DNS_LOOKUP_ALGORITHM::trrOnly); break; - case MODE_SHADOW: - AccumulateCategorical(Telemetry::LABELS_DNS_LOOKUP_ALGORITHM::trrShadow); - break; case MODE_RESERVED1: MOZ_DIAGNOSTIC_ASSERT(false, "MODE_RESERVED1 should not be used"); break; + case MODE_RESERVED4: + MOZ_DIAGNOSTIC_ASSERT(false, "MODE_RESERVED4 should not be used"); + break; } if (mTRRUsed && !mTRRSuccess && mNativeSuccess && gTRRService) { @@ -1448,8 +1411,7 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec) { rv = TrrLookup(rec); } - if (TRR_DISABLED(mode) || (mode == MODE_SHADOW) || - ((mode == MODE_TRRFIRST) && NS_FAILED(rv)) || + if (TRR_DISABLED(mode) || ((mode == MODE_TRRFIRST) && NS_FAILED(rv)) || (mode == MODE_TRRONLY && skipTRR)) { if (!rec->IsAddrRecord()) { return rv; @@ -1742,8 +1704,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup( if (addrRec->mTRRSuccess == 1) { // Store the duration on first succesful TRR response. We // don't know that there will be a second response nor can we - // tell which of two has useful data, especially in - // MODE_SHADOW where the actual results are discarded. + // tell which of two has useful data. addrRec->mTrrDuration = TimeStamp::Now() - addrRec->mTrrStart; } } @@ -1759,7 +1720,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup( addrRec->mFirstTRR.swap(newRRSet); // autoPtr.swap() MOZ_ASSERT(addrRec->mFirstTRR && !newRRSet); - if (addrRec->mDidCallbacks || addrRec->mResolverMode == MODE_SHADOW) { + if (addrRec->mDidCallbacks) { return LOOKUP_OK; } @@ -1828,8 +1789,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup( // update record fields. We might have a addrRec->addr_info already if a // previous lookup result expired and we're reresolving it or we get // a late second TRR response. - // note that we don't update the addr_info if this is trr shadow results - if (!mShutdown && !(trrResult && addrRec->mResolverMode == MODE_SHADOW)) { + if (!mShutdown) { MutexAutoLock lock(addrRec->addr_info_lock); RefPtr old_addr_info; if (different_rrset(addrRec->addr_info, newRRSet)) { @@ -1849,14 +1809,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup( bool doCallbacks = true; - if (trrResult && (addrRec->mResolverMode == MODE_SHADOW) && - !addrRec->mDidCallbacks) { - // don't report result based only on suppressed TRR info - doCallbacks = false; - LOG(( - "nsHostResolver Suppressing TRR %s because it is first shadow result\n", - addrRec->host.get())); - } else if (trrResult && addrRec->mDidCallbacks) { + if (trrResult && addrRec->mDidCallbacks) { // already callback'ed on the first TRR response LOG(("nsHostResolver Suppressing callback for second TRR response for %s\n", addrRec->host.get())); diff --git a/netwerk/dns/nsHostResolver.h b/netwerk/dns/nsHostResolver.h index 26ddf23b0ac2..c5944bbcd4d0 100644 --- a/netwerk/dns/nsHostResolver.h +++ b/netwerk/dns/nsHostResolver.h @@ -35,7 +35,7 @@ enum ResolverMode { MODE_RESERVED1, // 1 - Reserved value. Used to be parallel resolve. MODE_TRRFIRST, // 2 - fallback to native on TRR failure MODE_TRRONLY, // 3 - don't even fallback - MODE_SHADOW, // 4 - race for stats, but always use native result + MODE_RESERVED4, // 4 - Reserved value. Used to be race TRR with native. MODE_TRROFF // 5 - identical to MODE_NATIVEONLY but explicitly selected }; } // namespace net diff --git a/netwerk/test/unit/test_trr.js b/netwerk/test/unit/test_trr.js index f5956e6b9868..2cd966581c8e 100644 --- a/netwerk/test/unit/test_trr.js +++ b/netwerk/test/unit/test_trr.js @@ -28,7 +28,7 @@ add_task(function setup() { // make all native resolve calls "secretly" resolve localhost instead Services.prefs.setBoolPref("network.dns.native-is-localhost", true); - // 0 - off, 1 - race, 2 TRR first, 3 TRR only, 4 shadow + // 0 - off, 1 - reserved, 2 - TRR first, 3 - TRR only, 4 - reserved Services.prefs.setIntPref("network.trr.mode", 2); // TRR first Services.prefs.setBoolPref("network.trr.wait-for-portal", false); // don't confirm that TRR is working, just go! @@ -299,21 +299,29 @@ add_task(async function test13() { await new DNSListener("test13.example.com", "127.0.0.1"); }); -// TRR-shadow gets a 404 back from DOH +// Test that MODE_RESERVED4 (previously MODE_SHADOW) is treated as TRR off. add_task(async function test14() { dns.clearCache(true); - Services.prefs.setIntPref("network.trr.mode", 4); // TRR-shadow + Services.prefs.setIntPref("network.trr.mode", 4); // MODE_RESERVED4. Interpreted as TRR off. Services.prefs.setCharPref("network.trr.uri", `https://foo.example.com:${h2Port}/404`); await new DNSListener("test14.example.com", "127.0.0.1"); + + Services.prefs.setIntPref("network.trr.mode", 4); // MODE_RESERVED4. Interpreted as TRR off. + Services.prefs.setCharPref("network.trr.uri", `https://foo.example.com:${h2Port}/doh?responseIP=2.2.2.2`); + await new DNSListener("bar.example.com", "127.0.0.1"); }); -// TRR-shadow and timed out TRR +// Test that MODE_RESERVED4 (previously MODE_SHADOW) is treated as TRR off. add_task(async function test15() { dns.clearCache(true); - Services.prefs.setIntPref("network.trr.mode", 4); // TRR-shadow + Services.prefs.setIntPref("network.trr.mode", 4); // MODE_RESERVED4. Interpreted as TRR off. Services.prefs.setCharPref("network.trr.uri", `https://foo.example.com:${h2Port}/dns-750ms`); Services.prefs.setIntPref("network.trr.request-timeout", 10); await new DNSListener("test15.example.com", "127.0.0.1"); + + Services.prefs.setIntPref("network.trr.mode", 4); // MODE_RESERVED4. Interpreted as TRR off. + Services.prefs.setCharPref("network.trr.uri", `https://foo.example.com:${h2Port}/doh?responseIP=2.2.2.2`); + await new DNSListener("bar.example.com", "127.0.0.1"); }); // TRR-first and timed out TRR @@ -363,12 +371,16 @@ add_task(async function test20() { await new DNSListener("test20.example.com", "127.0.0.1"); }); -// TRR-shadow and a CNAME loop +// Test that MODE_RESERVED4 (previously MODE_SHADOW) is treated as TRR off. add_task(async function test21() { dns.clearCache(true); - Services.prefs.setIntPref("network.trr.mode", 4); // shadow + Services.prefs.setIntPref("network.trr.mode", 4); // MODE_RESERVED4. Interpreted as TRR off. Services.prefs.setCharPref("network.trr.uri", `https://foo.example.com:${h2Port}/doh?responseIP=none&cnameloop=true`); await new DNSListener("test21.example.com", "127.0.0.1"); + + Services.prefs.setIntPref("network.trr.mode", 4); // MODE_RESERVED4. Interpreted as TRR off. + Services.prefs.setCharPref("network.trr.uri", `https://foo.example.com:${h2Port}/doh?responseIP=2.2.2.2`); + await new DNSListener("bar.example.com", "127.0.0.1"); }); // verify that basic A record name mismatch gets rejected.