commit 12e4d8e30af263d4dad847a17fae817eace6887a
parent 709adbc66d6251cdadf0f3c54d7cd871784a5f8e
Author: Valentin Gosu <valentin.gosu@gmail.com>
Date: Wed, 19 Nov 2025 12:32:54 +0000
Bug 1996813 - Move test_connection_reuse_and_cycling into a separate trr test file r=necko-reviewers,kershaw
Also makes the test use the node TRRServer instead of relying on the global moz-http2 server
Differential Revision: https://phabricator.services.mozilla.com/D273189
Diffstat:
6 files changed, 247 insertions(+), 195 deletions(-)
diff --git a/netwerk/test/unit/head_trr.js b/netwerk/test/unit/head_trr.js
@@ -319,6 +319,25 @@ function trrQueryHandler(req, resp, url) {
let domain = dnsQuery.questions[0].name;
let type = dnsQuery.questions[0].type;
let response = global.dns_query_answers[`${domain}/${type}`] || {};
+ let delay = response.delay || 0;
+ let searchParams = new URL(req1.url, "http://example.com").searchParams;
+ if (searchParams.get("conncycle")) {
+ if (domain.startsWith("newconn")) {
+ // If we haven't seen a req for this newconn name before,
+ // or if we've seen one for the same name on the same port,
+ // synthesize a timeout.
+ if (
+ !global.gDoHNewConnLog[domain] ||
+ global.gDoHNewConnLog[domain] == req1.socket.remotePort
+ ) {
+ delay = 1000;
+ }
+ if (!global.gDoHNewConnLog[domain]) {
+ global.gDoHNewConnLog[domain] = req1.socket.remotePort;
+ }
+ }
+ global.gDoHPortsLog.push([domain, req1.socket.remotePort]);
+ }
if (!global.dns_query_counts[domain]) {
global.dns_query_counts[domain] = {};
@@ -361,7 +380,7 @@ function trrQueryHandler(req, resp, url) {
} catch (e) {}
};
- if (response.delay) {
+ if (delay) {
// This function is handled within the httpserver where setTimeout is
// available.
// eslint-disable-next-line no-undef
@@ -369,7 +388,7 @@ function trrQueryHandler(req, resp, url) {
arg => {
writeResponse(arg[0], arg[1], arg[2]);
},
- response.delay,
+ delay,
[resp1, buf, response]
);
return;
@@ -402,9 +421,13 @@ class TRRServer extends TRRNodeHttp2Server {
// value: a map containing {key: type, value: number of requests}
global.dns_query_counts = {};
+ global.gDoHPortsLog = [];
+ global.gDoHNewConnLog = {};
+
global.dnsPacket = require(\`\${__dirname}/../dns-packet\`);
global.ip = require(\`\${__dirname}/../node_ip\`);
global.http2 = require("http2");
+ global.url = require("url");
})()`);
await this.registerPathHandler("/dns-query", trrQueryHandler);
await this.registerPathHandler("/dnsAnswer", answerHandler);
diff --git a/netwerk/test/unit/test_trr.js b/netwerk/test/unit/test_trr.js
@@ -880,8 +880,6 @@ add_task(async function test_padding() {
);
});
-add_task(test_connection_reuse_and_cycling);
-
// Can't test for socket process since telemetry is captured in different process.
add_task(
{ skip_if: () => mozinfo.socketprocess_networking },
diff --git a/netwerk/test/unit/test_trr_connection_cycle.js b/netwerk/test/unit/test_trr_connection_cycle.js
@@ -0,0 +1,217 @@
+"use strict";
+
+/* import-globals-from trr_common.js */
+/* import-globals-from head_trr.js */
+
+const { TestUtils } = ChromeUtils.importESModule(
+ "resource://testing-common/TestUtils.sys.mjs"
+);
+
+function setLocalModeAndURI(mode, url) {
+ Services.prefs.setCharPref("network.trr.uri", url);
+ Services.prefs.setIntPref("network.trr.mode", mode);
+}
+
+let trrServer;
+add_setup(async function setup() {
+ trr_test_setup();
+ Services.prefs.setBoolPref("network.dns.native-is-localhost", true);
+ Services.dns.clearCache(true);
+ Services.prefs.setIntPref("network.trr.request_timeout_ms", 500);
+ Services.prefs.setIntPref(
+ "network.trr.strict_fallback_request_timeout_ms",
+ 500
+ );
+ Services.prefs.setIntPref("network.trr.request_timeout_mode_trronly_ms", 500);
+
+ trrServer = new TRRServer();
+ registerCleanupFunction(async () => {
+ await trrServer.stop();
+ });
+ await trrServer.start();
+ dump(`port = ${trrServer.port()}\n`);
+ await trrServer.registerDoHAnswers("confirm.example.com", "NS", {
+ answers: [
+ {
+ name: "confirm.example.com",
+ ttl: 55,
+ type: "NS",
+ flush: false,
+ data: "test.com",
+ },
+ ],
+ });
+});
+
+add_task(async function test_connection_reuse_and_cycling() {
+ Services.prefs.setCharPref(
+ "network.trr.confirmationNS",
+ "confirm.example.com"
+ );
+ setLocalModeAndURI(
+ 2,
+ `https://foo.example.com:${trrServer.port()}/dns-query`
+ );
+ Services.prefs.setBoolPref("network.trr.strict_native_fallback", true);
+ Services.prefs.setBoolPref("network.trr.retry_on_recoverable_errors", true);
+
+ await TestUtils.waitForCondition(
+ // 2 => CONFIRM_OK
+ () => Services.dns.currentTrrConfirmationState == 2,
+ `Timed out waiting for confirmation success. Currently ${Services.dns.currentTrrConfirmationState}`,
+ 1,
+ 5000
+ );
+
+ // Setting conncycle=true in the URI. Server will start logging reqs.
+ // We will do a specific sequence of lookups, then fetch the log from
+ // the server and check that it matches what we'd expect.
+ setLocalModeAndURI(
+ 2,
+ `https://foo.example.com:${trrServer.port()}/dns-query?conncycle=true`
+ );
+ await TestUtils.waitForCondition(
+ // 2 => CONFIRM_OK
+ () => Services.dns.currentTrrConfirmationState == 2,
+ `Timed out waiting for confirmation success. Currently ${Services.dns.currentTrrConfirmationState}`,
+ 1,
+ 5000
+ );
+ // Confirmation upon uri-change will have created one req.
+
+ async function registerDomain(domain) {
+ await trrServer.registerDoHAnswers(domain, "A", {
+ answers: [
+ {
+ name: domain,
+ ttl: 55,
+ type: "A",
+ flush: false,
+ data: "9.8.7.6",
+ },
+ ],
+ });
+ }
+
+ for (let i = 1; i <= 6; i++) {
+ await registerDomain(`bar${i}.example.org`);
+ }
+ await registerDomain("newconn.example.org");
+ await registerDomain("newconn2.example.org");
+
+ // Two reqs for each bar1 and bar2 - A + AAAA.
+ await new TRRDNSListener("bar1.example.org", "9.8.7.6");
+ await new TRRDNSListener("bar2.example.org", "9.8.7.6");
+ // Total so far: (1) + 2 + 2 = 5
+
+ // Two reqs that fail, one Confirmation req, two retried reqs that succeed.
+ await new TRRDNSListener("newconn.example.org", "9.8.7.6");
+ await TestUtils.waitForCondition(
+ // 2 => CONFIRM_OK
+ () => Services.dns.currentTrrConfirmationState == 2,
+ `Timed out waiting for confirmation success. Currently ${Services.dns.currentTrrConfirmationState}`,
+ 1,
+ 5000
+ );
+ // Total so far: (5) + 2 + 1 + 2 = 10
+
+ // Two reqs for each bar3 and bar4 .
+ await new TRRDNSListener("bar3.example.org", "9.8.7.6");
+ await new TRRDNSListener("bar4.example.org", "9.8.7.6");
+ // Total so far: (10) + 2 + 2 = 14.
+
+ // Two reqs that fail, one Confirmation req, two retried reqs that succeed.
+ await new TRRDNSListener("newconn2.example.org", "9.8.7.6");
+ await TestUtils.waitForCondition(
+ // 2 => CONFIRM_OK
+ () => Services.dns.currentTrrConfirmationState == 2,
+ `Timed out waiting for confirmation success. Currently ${Services.dns.currentTrrConfirmationState}`,
+ 1,
+ 5000
+ );
+ // Total so far: (14) + 2 + 1 + 2 = 19
+
+ // Two reqs for each bar5 and bar6 .
+ await new TRRDNSListener("bar5.example.org", "9.8.7.6");
+ await new TRRDNSListener("bar6.example.org", "9.8.7.6");
+ // Total so far: (19) + 2 + 2 = 23
+
+ let dohReqPortLog = await trrServer.execute(`global.gDoHPortsLog`);
+ info(JSON.stringify(dohReqPortLog));
+
+ // Since the actual ports seen will vary at runtime, we use placeholders
+ // instead in our expected output definition. For example, if two entries
+ // both have "port1", it means they both should have the same port in the
+ // server's log.
+ // For reqs that fail and trigger a Confirmation + retry, the retried reqs
+ // might not re-use the new connection created for Confirmation due to a
+ // race, so we have an extra alternate expected port for them. This lets
+ // us test that they use *a* new port even if it's not *the* new port.
+ // Subsequent lookups are not affected, they will use the same conn as
+ // the Confirmation req.
+ let expectedLogTemplate = [
+ ["confirm.example.com", "port1"],
+ ["bar1.example.org", "port1"],
+ ["bar1.example.org", "port1"],
+ ["bar2.example.org", "port1"],
+ ["bar2.example.org", "port1"],
+ ["newconn.example.org", "port1"],
+ ["newconn.example.org", "port1"],
+ ["confirm.example.com", "port2"],
+ ["newconn.example.org", "port2"],
+ ["newconn.example.org", "port2"],
+ ["bar3.example.org", "port2"],
+ ["bar3.example.org", "port2"],
+ ["bar4.example.org", "port2"],
+ ["bar4.example.org", "port2"],
+ ["newconn2.example.org", "port2"],
+ ["newconn2.example.org", "port2"],
+ ["confirm.example.com", "port3"],
+ ["newconn2.example.org", "port3"],
+ ["newconn2.example.org", "port3"],
+ ["bar5.example.org", "port3"],
+ ["bar5.example.org", "port3"],
+ ["bar6.example.org", "port3"],
+ ["bar6.example.org", "port3"],
+ ];
+
+ if (expectedLogTemplate.length != dohReqPortLog.length) {
+ // This shouldn't happen, and if it does, we'll fail the assertion
+ // below. But first dump the whole server-side log to help with
+ // debugging should we see a failure. Most likely cause would be
+ // that another consumer of TRR happened to make a request while
+ // the test was running and polluted the log.
+ info(dohReqPortLog);
+ }
+
+ equal(
+ expectedLogTemplate.length,
+ dohReqPortLog.length,
+ "Correct number of req log entries"
+ );
+
+ let seenPorts = new Set();
+ // This is essentially a symbol table - as we iterate through the log
+ // we will assign the actual seen port numbers to the placeholders.
+ let seenPortsByExpectedPort = new Map();
+
+ for (let i = 0; i < expectedLogTemplate.length; i++) {
+ let expectedName = expectedLogTemplate[i][0];
+ let expectedPort = expectedLogTemplate[i][1];
+ let seenName = dohReqPortLog[i][0];
+ let seenPort = dohReqPortLog[i][1];
+ info(`Checking log entry. Name: ${seenName}, Port: ${seenPort}`);
+ equal(expectedName, seenName, "Name matches for entry " + i);
+ if (!seenPortsByExpectedPort.has(expectedPort)) {
+ ok(!seenPorts.has(seenPort), "Port should not have been previously used");
+ seenPorts.add(seenPort);
+ seenPortsByExpectedPort.set(expectedPort, seenPort);
+ } else {
+ equal(
+ seenPort,
+ seenPortsByExpectedPort.get(expectedPort),
+ "Connection was reused as expected"
+ );
+ }
+ }
+});
diff --git a/netwerk/test/unit/trr_common.js b/netwerk/test/unit/trr_common.js
@@ -1079,162 +1079,3 @@ async function test_no_retry_without_doh() {
Services.prefs.clearUserPref("network.trr.allow-rfc1918");
Services.prefs.clearUserPref("network.socket.ip_addr_any.disabled");
}
-
-async function test_connection_reuse_and_cycling() {
- Services.dns.clearCache(true);
- Services.prefs.setIntPref("network.trr.request_timeout_ms", 500);
- Services.prefs.setIntPref(
- "network.trr.strict_fallback_request_timeout_ms",
- 500
- );
- Services.prefs.setIntPref("network.trr.request_timeout_mode_trronly_ms", 500);
-
- setModeAndURI(2, `doh?responseIP=9.8.7.6`);
- Services.prefs.setBoolPref("network.trr.strict_native_fallback", true);
- Services.prefs.setCharPref("network.trr.confirmationNS", "example.com");
- await TestUtils.waitForCondition(
- // 2 => CONFIRM_OK
- () => Services.dns.currentTrrConfirmationState == 2,
- `Timed out waiting for confirmation success. Currently ${Services.dns.currentTrrConfirmationState}`,
- 1,
- 5000
- );
-
- // Setting conncycle=true in the URI. Server will start logging reqs.
- // We will do a specific sequence of lookups, then fetch the log from
- // the server and check that it matches what we'd expect.
- setModeAndURI(2, `doh?responseIP=9.8.7.6&conncycle=true`);
- await TestUtils.waitForCondition(
- // 2 => CONFIRM_OK
- () => Services.dns.currentTrrConfirmationState == 2,
- `Timed out waiting for confirmation success. Currently ${Services.dns.currentTrrConfirmationState}`,
- 1,
- 5000
- );
- // Confirmation upon uri-change will have created one req.
-
- // Two reqs for each bar1 and bar2 - A + AAAA.
- await new TRRDNSListener("bar1.example.org.", "9.8.7.6");
- await new TRRDNSListener("bar2.example.org.", "9.8.7.6");
- // Total so far: (1) + 2 + 2 = 5
-
- // Two reqs that fail, one Confirmation req, two retried reqs that succeed.
- await new TRRDNSListener("newconn.example.org.", "9.8.7.6");
- await TestUtils.waitForCondition(
- // 2 => CONFIRM_OK
- () => Services.dns.currentTrrConfirmationState == 2,
- `Timed out waiting for confirmation success. Currently ${Services.dns.currentTrrConfirmationState}`,
- 1,
- 5000
- );
- // Total so far: (5) + 2 + 1 + 2 = 10
-
- // Two reqs for each bar3 and bar4 .
- await new TRRDNSListener("bar3.example.org.", "9.8.7.6");
- await new TRRDNSListener("bar4.example.org.", "9.8.7.6");
- // Total so far: (10) + 2 + 2 = 14.
-
- // Two reqs that fail, one Confirmation req, two retried reqs that succeed.
- await new TRRDNSListener("newconn2.example.org.", "9.8.7.6");
- await TestUtils.waitForCondition(
- // 2 => CONFIRM_OK
- () => Services.dns.currentTrrConfirmationState == 2,
- `Timed out waiting for confirmation success. Currently ${Services.dns.currentTrrConfirmationState}`,
- 1,
- 5000
- );
- // Total so far: (14) + 2 + 1 + 2 = 19
-
- // Two reqs for each bar5 and bar6 .
- await new TRRDNSListener("bar5.example.org.", "9.8.7.6");
- await new TRRDNSListener("bar6.example.org.", "9.8.7.6");
- // Total so far: (19) + 2 + 2 = 23
-
- let chan = makeChan(
- `https://foo.example.com:${h2Port}/get-doh-req-port-log`,
- Ci.nsIRequest.TRR_DISABLED_MODE
- );
- let dohReqPortLog = await new Promise(resolve =>
- chan.asyncOpen(
- new ChannelListener((stuff, buffer) => {
- resolve(JSON.parse(buffer));
- })
- )
- );
-
- // Since the actual ports seen will vary at runtime, we use placeholders
- // instead in our expected output definition. For example, if two entries
- // both have "port1", it means they both should have the same port in the
- // server's log.
- // For reqs that fail and trigger a Confirmation + retry, the retried reqs
- // might not re-use the new connection created for Confirmation due to a
- // race, so we have an extra alternate expected port for them. This lets
- // us test that they use *a* new port even if it's not *the* new port.
- // Subsequent lookups are not affected, they will use the same conn as
- // the Confirmation req.
- let expectedLogTemplate = [
- ["example.com", "port1"],
- ["bar1.example.org", "port1"],
- ["bar1.example.org", "port1"],
- ["bar2.example.org", "port1"],
- ["bar2.example.org", "port1"],
- ["newconn.example.org", "port1"],
- ["newconn.example.org", "port1"],
- ["example.com", "port2"],
- ["newconn.example.org", "port2"],
- ["newconn.example.org", "port2"],
- ["bar3.example.org", "port2"],
- ["bar3.example.org", "port2"],
- ["bar4.example.org", "port2"],
- ["bar4.example.org", "port2"],
- ["newconn2.example.org", "port2"],
- ["newconn2.example.org", "port2"],
- ["example.com", "port3"],
- ["newconn2.example.org", "port3"],
- ["newconn2.example.org", "port3"],
- ["bar5.example.org", "port3"],
- ["bar5.example.org", "port3"],
- ["bar6.example.org", "port3"],
- ["bar6.example.org", "port3"],
- ];
-
- if (expectedLogTemplate.length != dohReqPortLog.length) {
- // This shouldn't happen, and if it does, we'll fail the assertion
- // below. But first dump the whole server-side log to help with
- // debugging should we see a failure. Most likely cause would be
- // that another consumer of TRR happened to make a request while
- // the test was running and polluted the log.
- info(dohReqPortLog);
- }
-
- equal(
- expectedLogTemplate.length,
- dohReqPortLog.length,
- "Correct number of req log entries"
- );
-
- let seenPorts = new Set();
- // This is essentially a symbol table - as we iterate through the log
- // we will assign the actual seen port numbers to the placeholders.
- let seenPortsByExpectedPort = new Map();
-
- for (let i = 0; i < expectedLogTemplate.length; i++) {
- let expectedName = expectedLogTemplate[i][0];
- let expectedPort = expectedLogTemplate[i][1];
- let seenName = dohReqPortLog[i][0];
- let seenPort = dohReqPortLog[i][1];
- info(`Checking log entry. Name: ${seenName}, Port: ${seenPort}`);
- equal(expectedName, seenName, "Name matches for entry " + i);
- if (!seenPortsByExpectedPort.has(expectedPort)) {
- ok(!seenPorts.has(seenPort), "Port should not have been previously used");
- seenPorts.add(seenPort);
- seenPortsByExpectedPort.set(expectedPort, seenPort);
- } else {
- equal(
- seenPort,
- seenPortsByExpectedPort.get(expectedPort),
- "Connection was reused as expected"
- );
- }
- }
-}
diff --git a/netwerk/test/unit/xpcshell.toml b/netwerk/test/unit/xpcshell.toml
@@ -1420,6 +1420,10 @@ run-sequentially = ["true"] # node server exceptions dont replay well
run-if = ["!socketprocess_networking"] # confirmation state isn't passed cross-process
skip-if = ["appname == 'thunderbird'"] # bug 1760097
+["test_trr_connection_cycle.js"]
+run-sequentially = ["true"] # node server exceptions dont replay well
+run-if = ["!socketprocess_networking"] # confirmation state isn't passed cross-process
+
["test_trr_decoding.js"]
["test_trr_domain.js"]
diff --git a/testing/xpcshell/moz-http2/moz-http2.js b/testing/xpcshell/moz-http2/moz-http2.js
@@ -208,8 +208,6 @@ var didRst = false;
var rstConnection = null;
var illegalheader_conn = null;
-var gDoHPortsLog = [];
-var gDoHNewConnLog = {};
var gDoHRequestCount = 0;
// eslint-disable-next-line complexity
@@ -839,13 +837,6 @@ function handleRequest(req, res) {
emitResponse(res, payload);
});
return;
- } else if (u.pathname == "/get-doh-req-port-log") {
- let rContent = JSON.stringify(gDoHPortsLog);
- res.setHeader("Content-Type", "text/plain");
- res.setHeader("Content-Length", rContent.length);
- res.writeHead(400);
- res.end(rContent);
- return;
} else if (u.pathname == "/reset-doh-request-count") {
gDoHRequestCount = 0;
res.setHeader("Content-Type", "text/plain");
@@ -964,29 +955,7 @@ function handleRequest(req, res) {
// parload is empty when we send redirect response.
if (payload.length) {
let packet = dnsPacket.decode(payload);
- let delay;
- if (u.query.conncycle) {
- let name = packet.questions[0].name;
- if (name.startsWith("newconn")) {
- // If we haven't seen a req for this newconn name before,
- // or if we've seen one for the same name on the same port,
- // synthesize a timeout.
- if (
- !gDoHNewConnLog[name] ||
- gDoHNewConnLog[name] == req.remotePort
- ) {
- delay = 1000;
- }
- if (!gDoHNewConnLog[name]) {
- gDoHNewConnLog[name] = req.remotePort;
- }
- }
- gDoHPortsLog.push([packet.questions[0].name, req.remotePort]);
- } else {
- gDoHPortsLog = [];
- gDoHNewConnLog = {};
- }
- emitResponse(res, payload, packet, delay);
+ emitResponse(res, payload, packet);
}
});
return;
