Bug 1597137 - Don't load TRR entries from the cache if they are part of the excluded list. r=dragana

This patch fixes two issues where we may mistakenly load a TRR record even though the host is part of the excluded-domains list:
1. If a.com is part of the excluded domains, but b.com is not, then when we first resolve b.com using TRR, the server may also push the record for a.com; Previously we didn't check if the pushed record is also excluded, which could lead us to load it from the TRR cache.
2. If b.com is resolved using TRR, but later b.com is added to the excluded-domains list, we may mistakenly load b.com from the TRR cache, even though we should use platform DNS for it.

Differential Revision: https://phabricator.services.mozilla.com/D53354

--HG--
extra : moz-landing-system : lando

netwerk/dns/TRR.cpp

@@ -443,6 +443,10 @@ nsresult TRR::ReceivePush(nsIHttpChannel* pushed, nsHostRecord* pushedRec) {
     return NS_ERROR_UNEXPECTED;
   }
 
+  if (gTRRService->IsExcludedFromTRR(mHost)) {
+    return NS_ERROR_FAILURE;
+  }
+
   RefPtr<nsHostRecord> hostRecord;
   nsresult rv;
   rv = mHostResolver->GetHostRecord(

netwerk/dns/nsHostResolver.cpp

@@ -866,6 +866,10 @@ nsresult nsHostResolver::ResolveHost(const nsACString& aHost, uint16_t type,
       nsAutoCString originSuffix;
       aOriginAttributes.CreateSuffix(originSuffix);
 
+      if (gTRRService && gTRRService->IsExcludedFromTRR(host)) {
+        flags |= RES_DISABLE_TRR;
+      }
+
       nsHostKey key(host, type, flags, af,
                     (aOriginAttributes.mPrivateBrowsingId > 0), originSuffix);
       RefPtr<nsHostRecord>& entry = mRecordDB.GetOrInsert(key);

netwerk/test/unit/test_trr.js

@@ -117,7 +117,10 @@ class DNSListener {
   }
 
   onLookupComplete(inRequest, inRecord, inStatus) {
-    Assert.ok(inRequest == this.request);
+    Assert.ok(
+      inRequest == this.request,
+      "Checking that this is the correct callback"
+    );
 
     // If we don't expect success here, just resolve and the caller will
     // decide what to do with the results.
@@ -126,9 +129,13 @@ class DNSListener {
       return;
     }
 
-    Assert.equal(inStatus, Cr.NS_OK);
+    Assert.equal(inStatus, Cr.NS_OK, "Checking status");
     let answer = inRecord.getNextAddrAsString();
-    Assert.equal(answer, this.expectedAnswer);
+    Assert.equal(
+      answer,
+      this.expectedAnswer,
+      `Checking result for ${this.name}`
+    );
     this.resolve([inRequest, inRecord, inStatus]);
   }
 
@@ -1099,6 +1106,7 @@ add_task(async function test_dnsSuffix() {
     );
     await new DNSListener("example.org", "1.2.3.4");
     await new DNSListener("push.example.org", "2018::2018");
+    await new DNSListener("test.com", "1.2.3.4");
 
     let networkLinkService = {
       dnsSuffixList: ["example.org"],
@@ -1109,6 +1117,7 @@ add_task(async function test_dnsSuffix() {
       "network:link-status-changed",
       "changed"
     );
+    await new DNSListener("test.com", "1.2.3.4");
     await new DNSListener("example.org", "127.0.0.1");
     // Also test that we don't use the pushed entry.
     await new DNSListener("push.example.org", "127.0.0.1");