Bug 1943011 - If 'no-default-alpn' is not found, add 'http/1.1' to the list of records, a=RyanVM

Original Revision: https://phabricator.services.mozilla.com/D235170

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

netwerk/dns/HTTPSSVC.cpp

@@ -214,13 +214,25 @@ class AlpnComparator {
 
 nsTArray<std::tuple<nsCString, SupportedAlpnRank>> SVCB::GetAllAlpn() const {
   nsTArray<std::tuple<nsCString, SupportedAlpnRank>> alpnList;
+  bool shouldAddDefaultAlpn = true;
   for (const auto& value : mSvcFieldValue) {
     if (value.mValue.is<SvcParamAlpn>()) {
       for (const auto& alpn : value.mValue.as<SvcParamAlpn>().mValue) {
-        alpnList.AppendElement(std::make_tuple(alpn, IsAlpnSupported(alpn)));
+        auto tuple = std::make_tuple(alpn, IsAlpnSupported(alpn));
+        alpnList.AppendElement(tuple);
+        if (std::get<1>(tuple) == SupportedAlpnRank::HTTP_1_1) {
+          shouldAddDefaultAlpn = false;
+        }
       }
+    } else if (value.mValue.is<SvcParamKeyNoDefaultAlpn>()) {
+      // Found "no-default-alpn", so don't bother to add it.
+      shouldAddDefaultAlpn = false;
     }
   }
+  if (shouldAddDefaultAlpn) {
+    alpnList.AppendElement(
+        std::make_tuple("http/1.1"_ns, SupportedAlpnRank::HTTP_1_1));
+  }
   alpnList.Sort(AlpnComparator());
   return alpnList;
 }
@@ -330,8 +342,10 @@ static bool CheckAlpnIsUsable(SupportedAlpnRank aAlpnType, bool aNoHttp2,
   return true;
 }
 
-static nsTArray<SVCBWrapper> FlattenRecords(const nsTArray<SVCB>& aRecords) {
+static nsTArray<SVCBWrapper> FlattenRecords(const nsTArray<SVCB>& aRecords,
+                                            uint32_t& aH3RecordCount) {
   nsTArray<SVCBWrapper> result;
+  aH3RecordCount = 0;
   for (const auto& record : aRecords) {
     nsTArray<std::tuple<nsCString, SupportedAlpnRank>> alpnList =
         record.GetAllAlpn();
@@ -341,6 +355,9 @@ static nsTArray<SVCBWrapper> FlattenRecords(const nsTArray<SVCB>& aRecords) {
       for (const auto& alpn : alpnList) {
         SVCBWrapper wrapper(record);
         wrapper.mAlpn = Some(alpn);
+        if (IsHttp3(std::get<1>(alpn))) {
+          aH3RecordCount++;
+        }
         result.AppendElement(wrapper);
       }
     }
@@ -359,8 +376,8 @@ DNSHTTPSSVCRecordBase::GetServiceModeRecordInternal(
   aRecordsAllExcluded = false;
   nsCOMPtr<nsIDNSService> dns = do_GetService(NS_DNSSERVICE_CONTRACTID);
   uint32_t h3ExcludedCount = 0;
-
-  nsTArray<SVCBWrapper> records = FlattenRecords(aRecords);
+  uint32_t h3RecordCount = 0;
+  nsTArray<SVCBWrapper> records = FlattenRecords(aRecords, h3RecordCount);
   for (const auto& record : records) {
     if (record.mRecord.mSvcFieldPriority == 0) {
       // In ServiceMode, the SvcPriority should never be 0.
@@ -421,7 +438,7 @@ DNSHTTPSSVCRecordBase::GetServiceModeRecordInternal(
 
     // If all records are in http3 excluded list, try again without checking the
     // excluded list. This is better than returning nothing.
-    if (h3ExcludedCount == records.Length() && aCheckHttp3ExcludedList) {
+    if (h3ExcludedCount == h3RecordCount && aCheckHttp3ExcludedList) {
       return GetServiceModeRecordInternal(aNoHttp2, aNoHttp3, aRecords,
                                           aRecordsAllExcluded, false);
     }
@@ -454,7 +471,8 @@ void DNSHTTPSSVCRecordBase::GetAllRecordsWithEchConfigInternal(
   }
 
   uint32_t h3ExcludedCount = 0;
-  nsTArray<SVCBWrapper> records = FlattenRecords(aRecords);
+  uint32_t h3RecordCount = 0;
+  nsTArray<SVCBWrapper> records = FlattenRecords(aRecords, h3RecordCount);
   for (const auto& record : records) {
     if (record.mRecord.mSvcFieldPriority == 0) {
       // This should not happen, since GetAllRecordsWithEchConfigInternal()
@@ -493,7 +511,7 @@ void DNSHTTPSSVCRecordBase::GetAllRecordsWithEchConfigInternal(
 
   // If all records are in http3 excluded list, try again without checking the
   // excluded list. This is better than returning nothing.
-  if (h3ExcludedCount == records.Length() && aCheckHttp3ExcludedList) {
+  if (h3ExcludedCount == h3RecordCount && aCheckHttp3ExcludedList) {
     GetAllRecordsWithEchConfigInternal(
         aNoHttp2, aNoHttp3, aRecords, aAllRecordsHaveEchConfig,
         aAllRecordsInH3ExcludedList, aResult, false);

netwerk/test/unit/test_http3_fast_fallback.js

@@ -211,6 +211,7 @@ add_task(async function testFastfallback() {
           name: "test.fastfallback1.com",
           values: [
             { key: "alpn", value: "h3-29" },
+            { key: "no-default-alpn" },
             { key: "port", value: h3Port },
             { key: "echconfig", value: "456..." },
           ],
@@ -300,6 +301,7 @@ add_task(async function testFastfallback1() {
           name: "test.fastfallback1.org",
           values: [
             { key: "alpn", value: "h3-29" },
+            { key: "no-default-alpn" },
             { key: "port", value: h3Port },
             { key: "echconfig", value: "456..." },
           ],

netwerk/test/unit/test_https_rr_ech_prefs.js

@@ -74,7 +74,10 @@ add_task(async function testEchConfigEnabled() {
         data: {
           priority: 1,
           name: "test.bar_1.com",
-          values: [{ key: "alpn", value: ["h3-29"] }],
+          values: [
+            { key: "alpn", value: ["h3-29"] },
+            { key: "no-default-alpn" },
+          ],
         },
       },
       {
@@ -113,7 +116,11 @@ add_task(async function testEchConfigEnabled() {
     expectedName: "test.bar_1.com",
     expectedAlpn: "h3-29",
   });
-  checkResult(inRecord, true, true);
+  checkResult(inRecord, true, true, {
+    expectedPriority: 2,
+    expectedName: "test.bar_2.com",
+    expectedAlpn: "http/1.1",
+  });
 
   Services.prefs.setBoolPref("network.dns.echconfig.enabled", true);
   Services.dns.clearCache(true);
@@ -133,11 +140,15 @@ add_task(async function testEchConfigEnabled() {
     expectedAlpn: "h2",
   });
   checkResult(inRecord, true, false, {
-    expectedPriority: 1,
-    expectedName: "test.bar_1.com",
-    expectedAlpn: "h3-29",
+    expectedPriority: 2,
+    expectedName: "test.bar_2.com",
+    expectedAlpn: "http/1.1",
+  });
+  checkResult(inRecord, true, true, {
+    expectedPriority: 2,
+    expectedName: "test.bar_2.com",
+    expectedAlpn: "http/1.1",
   });
-  checkResult(inRecord, true, true);
 
   await trrServer.stop();
   trrServer = null;
@@ -175,6 +186,7 @@ add_task(async function testTwoRecordsHaveEchConfig() {
           name: "test.foo_h3.com",
           values: [
             { key: "alpn", value: ["h3"] },
+            { key: "no-default-alpn" },
             { key: "echconfig", value: "456..." },
           ],
         },
@@ -211,11 +223,15 @@ add_task(async function testTwoRecordsHaveEchConfig() {
     expectedAlpn: "h2",
   });
   checkResult(inRecord, true, false, {
-    expectedPriority: 1,
-    expectedName: "test.foo_h3.com",
-    expectedAlpn: "h3",
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
+  checkResult(inRecord, true, true, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
   });
-  checkResult(inRecord, true, true);
 
   Services.prefs.setBoolPref("network.dns.http3_echconfig.enabled", true);
   Services.dns.clearCache(true);
@@ -238,7 +254,11 @@ add_task(async function testTwoRecordsHaveEchConfig() {
     expectedName: "test.foo_h3.com",
     expectedAlpn: "h3",
   });
-  checkResult(inRecord, true, true);
+  checkResult(inRecord, true, true, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
 
   await trrServer.stop();
   trrServer = null;
@@ -274,6 +294,7 @@ add_task(async function testTwoRecordsHaveEchConfig1() {
           name: "test.foo_h3.com",
           values: [
             { key: "alpn", value: ["h3", "h2"] },
+            { key: "no-default-alpn" },
             { key: "echconfig", value: "456..." },
           ],
         },
@@ -380,6 +401,7 @@ add_task(async function testOneRecordsHasEchConfig() {
           name: "test.foo_h3.com",
           values: [
             { key: "alpn", value: ["h3"] },
+            { key: "no-default-alpn" },
             { key: "echconfig", value: "456..." },
           ],
         },
@@ -417,7 +439,11 @@ add_task(async function testOneRecordsHasEchConfig() {
     expectedName: "test.foo_h3.com",
     expectedAlpn: "h3",
   });
-  checkResult(inRecord, true, true);
+  checkResult(inRecord, true, true, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
 
   Services.prefs.setBoolPref("network.dns.http3_echconfig.enabled", true);
   Services.dns.clearCache(true);
@@ -440,7 +466,11 @@ add_task(async function testOneRecordsHasEchConfig() {
     expectedName: "test.foo_h3.com",
     expectedAlpn: "h3",
   });
-  checkResult(inRecord, true, true);
+  checkResult(inRecord, true, true, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
 
   await trrServer.stop();
   trrServer = null;
@@ -475,6 +505,7 @@ add_task(async function testHttp3AndHttp2Pref() {
           name: "test.foo_h3.com",
           values: [
             { key: "alpn", value: ["h3", "h2"] },
+            { key: "no-default-alpn" },
             { key: "echconfig", value: "456..." },
           ],
         },
@@ -510,11 +541,23 @@ add_task(async function testHttp3AndHttp2Pref() {
     expectedName: "test.foo_h3.com",
     expectedAlpn: "h2",
   });
-  checkResult(inRecord, true, false);
-  checkResult(inRecord, true, true);
+  checkResult(inRecord, true, false, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
+  checkResult(inRecord, true, true, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
 
   Services.prefs.setBoolPref("network.http.http2.enabled", false);
-  checkResult(inRecord, false, false);
+  checkResult(inRecord, false, false, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
 
   Services.prefs.setBoolPref("network.http.http3.enable", true);
   checkResult(inRecord, false, false, {
@@ -522,13 +565,21 @@ add_task(async function testHttp3AndHttp2Pref() {
     expectedName: "test.foo_h3.com",
     expectedAlpn: "h3",
   });
-  checkResult(inRecord, false, true);
+  checkResult(inRecord, false, true, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
   checkResult(inRecord, true, false, {
     expectedPriority: 1,
     expectedName: "test.foo_h3.com",
     expectedAlpn: "h3",
   });
-  checkResult(inRecord, true, true);
+  checkResult(inRecord, true, true, {
+    expectedPriority: 2,
+    expectedName: "test.foo_h2.com",
+    expectedAlpn: "http/1.1",
+  });
 
   await trrServer.stop();
   trrServer = null;

netwerk/test/unit/test_trr_https_fallback.js

@@ -206,20 +206,6 @@ add_task(async function testFallbackToTheOrigin() {
           ],
         },
       },
-      {
-        name: "test.foo.com",
-        ttl: 55,
-        type: "HTTPS",
-        flush: false,
-        data: {
-          priority: 3,
-          name: "test.foo3.com",
-          values: [
-            { key: "alpn", value: ["h2", "h3-26"] },
-            { key: "echconfig", value: "456..." },
-          ],
-        },
-      },
       {
         name: "test.foo.com",
         ttl: 55,
@@ -799,6 +785,7 @@ add_task(async function testHttp3ExcludedList() {
           name: "www.h3_fail.org",
           values: [
             { key: "alpn", value: "h3-29" },
+            { key: "no-default-alpn" },
             { key: "port", value: h3Port },
           ],
         },
@@ -901,6 +888,7 @@ add_task(async function testAllRecordsInHttp3ExcludedList() {
           name: "www.h3_fail1.org",
           values: [
             { key: "alpn", value: "h3-29" },
+            { key: "no-default-alpn" },
             { key: "port", value: h3Port },
             { key: "echconfig", value: "456..." },
           ],