diff --git a/netwerk/dns/HTTPSSVC.cpp b/netwerk/dns/HTTPSSVC.cpp index 0feb0dee0d8d..4260cab820ef 100644 --- a/netwerk/dns/HTTPSSVC.cpp +++ b/netwerk/dns/HTTPSSVC.cpp @@ -359,6 +359,7 @@ static nsTArray FlattenRecords(const nsACString& aHost, if (alpnList.IsEmpty()) { result.AppendElement(SVCBWrapper(record)); } else { + bool h1AlpnAdded = false; if (!hasNoDefaultAlpn) { // Consider two scenarios when "no-default-alpn" is not found: // 1. If echConfig is present in the record: @@ -374,15 +375,32 @@ static nsTArray FlattenRecords(const nsACString& aHost, if (!aHost.Equals(record.mSvcDomainName) || record.mHasEchConfig) { alpnList.AppendElement( std::make_tuple(""_ns, SupportedAlpnRank::HTTP_1_1)); + h1AlpnAdded = true; } } for (const auto& alpn : alpnList) { - SVCBWrapper wrapper(record); - wrapper.mAlpn = Some(alpn); - if (IsHttp3(std::get<1>(alpn))) { + const auto alpnRank = std::get<1>(alpn); + if (IsHttp3(alpnRank)) { aH3RecordCount++; } - result.AppendElement(wrapper); + + // Skip explicit h2 if h1 is already present. + if (alpnRank == SupportedAlpnRank::HTTP_2 && h1AlpnAdded) { + continue; + } + + // For h2, normalize the tuple to use an empty ALPN. If both h1 and h2 + // are available, the ALPN negotiation will automatically select h2 when + // the server supports it, otherwise it will fall back to h1. + // However, if `no-default-alpn` is present, fallback to h1 is not + // possible, so we must explicitly set h2. + auto chosen = + (alpnRank == SupportedAlpnRank::HTTP_2 && !hasNoDefaultAlpn) + ? std::make_tuple(""_ns, alpnRank) + : alpn; + SVCBWrapper wrapper(record); + wrapper.mAlpn = Some(chosen); + result.AppendElement(std::move(wrapper)); } } } diff --git a/netwerk/test/unit/test_http3_fast_fallback.js b/netwerk/test/unit/test_http3_fast_fallback.js index 6498a96bf74b..beef14359602 100644 --- a/netwerk/test/unit/test_http3_fast_fallback.js +++ b/netwerk/test/unit/test_http3_fast_fallback.js @@ -913,3 +913,64 @@ add_task(async function testFastfallbackToTheSameRecord() { await trrServer.stop(); }); + +// Similar to the previous test, but no ech. +add_task(async function testFastfallbackToTheSameRecord1() { + trrServer = new TRRServer(); + await trrServer.start(); + Services.prefs.setBoolPref("network.dns.upgrade_with_https_rr", true); + Services.prefs.setBoolPref("network.dns.use_https_rr_as_altsvc", true); + Services.prefs.setBoolPref("network.dns.echconfig.enabled", true); + Services.prefs.setBoolPref("network.dns.http3_echconfig.enabled", true); + + Services.prefs.setIntPref("network.trr.mode", 3); + Services.prefs.setCharPref( + "network.trr.uri", + `https://foo.example.com:${trrServer.port()}/dns-query` + ); + Services.prefs.setBoolPref("network.http.http3.enable", true); + + Services.prefs.setIntPref( + "network.dns.httpssvc.http3_fast_fallback_timeout", + 1000 + ); + + await trrServer.registerDoHAnswers("test.no_ech.org", "HTTPS", { + answers: [ + { + name: "test.no_ech.org", + ttl: 55, + type: "HTTPS", + flush: false, + data: { + priority: 1, + name: "test.no_ech.org", + values: [ + { key: "alpn", value: ["h3", "h2"] }, + { key: "port", value: h2Port }, + ], + }, + }, + ], + }); + + await trrServer.registerDoHAnswers("test.no_ech.org", "A", { + answers: [ + { + name: "test.no_ech.org", + ttl: 55, + type: "A", + flush: false, + data: "127.0.0.1", + }, + ], + }); + + let chan = makeChan(`https://test.no_ech.org/server-timing`); + let [req] = await channelOpenPromise(chan); + Assert.equal(req.protocolVersion, "h2"); + let internal = req.QueryInterface(Ci.nsIHttpChannelInternal); + Assert.equal(internal.remotePort, h2Port); + + await trrServer.stop(); +}); diff --git a/netwerk/test/unit/test_https_rr_ech_prefs.js b/netwerk/test/unit/test_https_rr_ech_prefs.js index 28874761d24b..de0e27bbc3c6 100644 --- a/netwerk/test/unit/test_https_rr_ech_prefs.js +++ b/netwerk/test/unit/test_https_rr_ech_prefs.js @@ -106,7 +106,7 @@ add_task(async function testEchConfigEnabled() { checkResult(inRecord, false, true, { expectedPriority: 2, expectedName: "test.bar_2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1, @@ -129,12 +129,12 @@ add_task(async function testEchConfigEnabled() { checkResult(inRecord, false, false, { expectedPriority: 2, expectedName: "test.bar_2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, false, true, { expectedPriority: 2, expectedName: "test.bar_2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 2, @@ -212,12 +212,12 @@ add_task(async function testTwoRecordsHaveEchConfig() { checkResult(inRecord, false, false, { expectedPriority: 2, expectedName: "test.foo_h2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, false, true, { expectedPriority: 2, expectedName: "test.foo_h2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 2, @@ -244,7 +244,7 @@ add_task(async function testTwoRecordsHaveEchConfig() { checkResult(inRecord, false, true, { expectedPriority: 2, expectedName: "test.foo_h2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1, @@ -429,7 +429,7 @@ add_task(async function testOneRecordsHasEchConfig() { checkResult(inRecord, false, true, { expectedPriority: 2, expectedName: "test.foo_h2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1, @@ -456,7 +456,7 @@ add_task(async function testOneRecordsHasEchConfig() { checkResult(inRecord, false, true, { expectedPriority: 2, expectedName: "test.foo_h2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1, diff --git a/netwerk/test/unit/test_https_rr_sorted_alpn.js b/netwerk/test/unit/test_https_rr_sorted_alpn.js index cc033b5bee30..21aac5581c5e 100644 --- a/netwerk/test/unit/test_https_rr_sorted_alpn.js +++ b/netwerk/test/unit/test_https_rr_sorted_alpn.js @@ -89,7 +89,7 @@ add_task(async function testSortedAlpnH3() { checkResult(inRecord, false, true, { expectedPriority: 1, expectedName: "test.alpn.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1, @@ -106,12 +106,12 @@ add_task(async function testSortedAlpnH3() { checkResult(inRecord, false, false, { expectedPriority: 1, expectedName: "test.alpn.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, false, true, { expectedPriority: 1, expectedName: "test.alpn.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1, @@ -130,12 +130,12 @@ add_task(async function testSortedAlpnH3() { checkResult(inRecord, false, false, { expectedPriority: 1, expectedName: "test.alpn.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, false, true, { expectedPriority: 1, expectedName: "test.alpn.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1, @@ -158,7 +158,7 @@ add_task(async function testSortedAlpnH3() { checkResult(inRecord, false, true, { expectedPriority: 1, expectedName: "test.alpn.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1, @@ -203,12 +203,12 @@ add_task(async function testSortedAlpnH2() { checkResult(inRecord, false, false, { expectedPriority: 1, expectedName: "test.alpn_2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, false, true, { expectedPriority: 1, expectedName: "test.alpn_2.com", - expectedAlpn: "h2", + expectedAlpn: "", }); checkResult(inRecord, true, false, { expectedPriority: 1,