Bug 1558495 - Make sure we retry the TRR connection if it fails r=agr…

…over

This patch adds:
* tests that we restart the TRR connection if it gets abnormally shut down
* a way to terminate the TRR connection when attempting to resolve closeme.com
* makes sure that resolving excluded domains with the DISABLE_TRR flag does
  not fail. Before this we would return an error code without checking the
  excluded domains first.

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

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

netwerk/dns/nsHostResolver.cpp

@@ -1396,13 +1396,6 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec) {
     addrRec->mTrrAAAAUsed = AddrHostRecord::INIT;
   }
 
-  if (rec->flags & RES_DISABLE_TRR) {
-    if (mode == MODE_TRRONLY) {
-      return rv;
-    }
-    mode = MODE_NATIVEONLY;
-  }
-
   // For domains that are excluded from TRR we fallback to NativeLookup.
   // This happens even in MODE_TRRONLY.
   // By default localhost and local are excluded (so we cover *.local hosts)
@@ -1412,6 +1405,13 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec) {
     skipTRR = gTRRService->IsExcludedFromTRR(rec->host);
   }
 
+  if (rec->flags & RES_DISABLE_TRR) {
+    if (mode == MODE_TRRONLY && !skipTRR) {
+      return rv;
+    }
+    mode = MODE_NATIVEONLY;
+  }
+
   if (!TRR_DISABLED(mode) && !skipTRR) {
     rv = TrrLookup(rec);
   }

netwerk/test/unit/test_trr.js

@@ -124,6 +124,9 @@ add_task(async function test1b() {
   Services.prefs.setCharPref("network.dns.localDomains", "foo.example.com");
 
   await new DNSListener("bar.example.com", "3.3.3.3");
+
+  Services.prefs.setCharPref("network.trr.bootstrapAddress", "127.0.0.1");
+  Services.prefs.clearUserPref("network.dns.localDomains");
 });
 
 // verify that the name was put in cache - it works with bad DNS URI
@@ -535,3 +538,90 @@ add_task(async function count_cookies() {
   Assert.equal(cm.countCookiesFromHost("example.com"), 0);
   Assert.equal(cm.countCookiesFromHost("foo.example.com."), 0);
 });
+
+add_task(async function test_connection_closed() {
+  dns.clearCache(true);
+  Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only
+  Services.prefs.setCharPref("network.trr.excluded-domains", "");
+  Services.prefs.setCharPref("network.trr.uri", `https://foo.example.com:${h2Port}/doh?responseIP=2.2.2.2`);
+  // bootstrap
+  Services.prefs.clearUserPref("network.dns.localDomains");
+  Services.prefs.setCharPref("network.trr.bootstrapAddress", "127.0.0.1");
+
+  await new DNSListener("bar.example.com", "2.2.2.2");
+
+  // makes the TRR connection shut down.
+  let [, , inStatus] = await new DNSListener("closeme.com", undefined, false);
+  Assert.ok(!Components.isSuccessCode(inStatus), `${inStatus} should be an error code`);
+  await new DNSListener("bar2.example.com", "2.2.2.2");
+});
+
+add_task(async function test_connection_closed_no_bootstrap() {
+  dns.clearCache(true);
+  Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only
+  Services.prefs.setCharPref("network.trr.excluded-domains", "localhost,local");
+  Services.prefs.setCharPref("network.trr.uri", `https://foo.example.com:${h2Port}/doh?responseIP=3.3.3.3`);
+  Services.prefs.setCharPref("network.dns.localDomains", "foo.example.com");
+  Services.prefs.clearUserPref("network.trr.bootstrapAddress");
+
+  await new DNSListener("bar.example.com", "3.3.3.3");
+
+  // makes the TRR connection shut down.
+  let [, , inStatus] = await new DNSListener("closeme.com", undefined, false);
+  Assert.ok(!Components.isSuccessCode(inStatus), `${inStatus} should be an error code`);
+  await new DNSListener("bar2.example.com", "3.3.3.3");
+});
+
+add_task(async function test_connection_closed_no_bootstrap_localhost() {
+  dns.clearCache(true);
+  Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only
+  Services.prefs.setCharPref("network.trr.excluded-domains", "localhost");
+  Services.prefs.setCharPref("network.trr.uri", `https://localhost:${h2Port}/doh?responseIP=3.3.3.3`);
+  Services.prefs.clearUserPref("network.dns.localDomains");
+  Services.prefs.clearUserPref("network.trr.bootstrapAddress");
+
+  await new DNSListener("bar.example.com", "3.3.3.3");
+
+  // makes the TRR connection shut down.
+  let [, , inStatus] = await new DNSListener("closeme.com", undefined, false);
+  Assert.ok(!Components.isSuccessCode(inStatus), `${inStatus} should be an error code`);
+  await new DNSListener("bar2.example.com", "3.3.3.3");
+});
+
+add_task(async function test_connection_closed_no_bootstrap_no_excluded() {
+  // This test exists to document what happens when we're in TRR only mode
+  // and we don't set a bootstrap address. We use DNS to resolve the
+  // initial URI, but if the connection fails, we don't fallback to DNS
+  dns.clearCache(true);
+  Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only
+  Services.prefs.setCharPref("network.trr.excluded-domains", "");
+  Services.prefs.setCharPref("network.trr.uri", `https://localhost:${h2Port}/doh?responseIP=3.3.3.3`);
+  Services.prefs.clearUserPref("network.dns.localDomains");
+  Services.prefs.clearUserPref("network.trr.bootstrapAddress");
+
+  await new DNSListener("bar.example.com", "3.3.3.3");
+
+  // makes the TRR connection shut down.
+  let [, , inStatus] = await new DNSListener("closeme.com", undefined, false);
+  Assert.ok(!Components.isSuccessCode(inStatus), `${inStatus} should be an error code`);
+  [, , inStatus] = await new DNSListener("bar2.example.com", undefined, false);
+  Assert.ok(!Components.isSuccessCode(inStatus), `${inStatus} should be an error code`);
+});
+
+add_task(async function test_connection_closed_trr_first() {
+  // This test exists to document what happens when we're in TRR only mode
+  // and we don't set a bootstrap address. We use DNS to resolve the
+  // initial URI, but if the connection fails, we don't fallback to DNS
+  dns.clearCache(true);
+  Services.prefs.setIntPref("network.trr.mode", 2); // TRR-first
+  Services.prefs.setCharPref("network.trr.uri", `https://localhost:${h2Port}/doh?responseIP=9.9.9.9`);
+  Services.prefs.setCharPref("network.dns.localDomains", "closeme.com");
+  Services.prefs.clearUserPref("network.trr.bootstrapAddress");
+
+  await new DNSListener("bar.example.com", "9.9.9.9");
+
+  // makes the TRR connection shut down. Should fallback to DNS
+  await new DNSListener("closeme.com", "127.0.0.1");
+  // TRR should be back up again
+  await new DNSListener("bar2.example.com", "9.9.9.9");
+});

testing/xpcshell/moz-http2/moz-http2.js

@@ -611,6 +611,13 @@ function handleRequest(req, res) {
     function emitResponse(response, requestPayload) {
       let packet = dnsPacket.decode(requestPayload);
 
+      // This shuts down the connection so we can test if the client reconnects
+      if (packet.questions.length > 0 &&
+        packet.questions[0].name == "closeme.com") {
+        response.stream.connection.close('INTERNAL_ERROR', response.stream.id);
+        return;
+      }
+
       function responseType() {
         if (packet.questions.length > 0 &&
           packet.questions[0].name == "confirm.example.com" &&