From d0ca971a2b2d0103aeb451ee42b6566469c2521d Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Thu, 21 Aug 2025 07:34:49 +0000 Subject: [PATCH] Fix HTTPS Agent IPv6 family option not being passed to DNS lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The family option in https.Agent (e.g., {family: 6} for IPv6) was not being passed to the DNS lookup function, causing DNS resolution to return both IPv4 and IPv6 addresses regardless of the family setting. This meant that IPv6-only connections would fall back to IPv4. Changes: - Add default DNS lookup function when not provided in ClientRequest - Pass agent's family option to DNS lookup when available - Ensure DNS lookup respects family filtering for proper address selection This allows users to force IPv6-only connections using: ```js const agent = new https.Agent({ family: 6 }); https.request({ hostname: 'example.com', agent }, callback); ``` Fixes issue where IPv6 family setting was ignored. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/js/node/_http_client.ts | 13 +- .../http/https-agent-family-simple.test.ts | 69 +++++++++ test/js/node/http/https-agent-family.test.ts | 132 ++++++++++++++++++ .../issue/ipv6-https-agent-family.test.ts | 73 ++++++++++ 4 files changed, 286 insertions(+), 1 deletion(-) create mode 100644 test/js/node/http/https-agent-family-simple.test.ts create mode 100644 test/js/node/http/https-agent-family.test.ts create mode 100644 test/regression/issue/ipv6-https-agent-family.test.ts diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index d6299c2b80..93d0d83db4 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -490,7 +490,12 @@ function ClientRequest(input, options, cb) { } try { - options.lookup(host, { all: true }, (err, results) => { + const lookupOptions = { all: true }; + // Pass family option from agent if available + if (this[kAgent]?.options?.family) { + lookupOptions.family = this[kAgent].options.family; + } + options.lookup(host, lookupOptions, (err, results) => { if (err) { if (!!$debug) globalReportError(err); process.nextTick((self, err) => self.emit("error", err), this, err); @@ -635,6 +640,12 @@ function ClientRequest(input, options, cb) { options = ObjectAssign(input || {}, options); } + // Set default DNS lookup if not provided + if (!options.lookup) { + const dns = require("node:dns"); + options.lookup = dns.lookup; + } + this[kTls] = null; this[kAbortController] = null; diff --git a/test/js/node/http/https-agent-family-simple.test.ts b/test/js/node/http/https-agent-family-simple.test.ts new file mode 100644 index 0000000000..7b6b3f0105 --- /dev/null +++ b/test/js/node/http/https-agent-family-simple.test.ts @@ -0,0 +1,69 @@ +import { test, expect, describe } from "bun:test"; +import https from "node:https"; + +describe("https.Agent family option (no network)", () => { + test("Agent should store family option correctly", () => { + const agent6 = new https.Agent({ family: 6 }); + expect(agent6.options.family).toBe(6); + + const agent4 = new https.Agent({ family: 4 }); + expect(agent4.options.family).toBe(4); + + const agentDefault = new https.Agent(); + expect(agentDefault.options.family).toBeUndefined(); + }); + + test("Agent.getName should include family in connection name", () => { + const agent6 = new https.Agent({ family: 6 }); + const agent4 = new https.Agent({ family: 4 }); + + const name6 = agent6.getName({ host: 'example.com', port: 443, family: 6 }); + const name4 = agent4.getName({ host: 'example.com', port: 443, family: 4 }); + const nameDefault = agent6.getName({ host: 'example.com', port: 443 }); + + expect(name6).toContain(':6'); + expect(name4).toContain(':4'); + // Without family parameter, should not include family in name + expect(nameDefault).not.toMatch(/:6$/); + }); + + test("DNS lookup function gets set by default in ClientRequest", () => { + // This test verifies that our fix to set options.lookup works + let lookupWasCalled = false; + let capturedLookupOptions: any = null; + + const mockLookup = (hostname: string, options: any, callback: any) => { + lookupWasCalled = true; + capturedLookupOptions = { ...options }; + // Call callback with error to avoid actual connection + callback(new Error("Test DNS error")); + }; + + const agent = new https.Agent({ family: 6 }); + + const req = https.request({ + hostname: 'test.example', + path: '/', + agent: agent, + lookup: mockLookup + }, (res) => {}); + + req.on('error', (err) => { + // Ignore the test DNS error + }); + + req.end(); + + // Give it a moment for the lookup to be called + return new Promise(resolve => { + setTimeout(() => { + expect(lookupWasCalled).toBe(true); + expect(capturedLookupOptions).not.toBeNull(); + expect(capturedLookupOptions.family).toBe(6); + expect(capturedLookupOptions.all).toBe(true); + req.destroy(); + resolve(undefined); + }, 50); + }); + }); +}); \ No newline at end of file diff --git a/test/js/node/http/https-agent-family.test.ts b/test/js/node/http/https-agent-family.test.ts new file mode 100644 index 0000000000..6ff954ae43 --- /dev/null +++ b/test/js/node/http/https-agent-family.test.ts @@ -0,0 +1,132 @@ +import { test, expect, describe } from "bun:test"; +import https from "node:https"; +import dns from "node:dns"; + +describe("https.Agent family option", () => { + test("should pass family option to DNS lookup", async () => { + // Mock DNS lookup to verify family option is passed through + let capturedOptions: any = null; + const originalLookup = dns.lookup; + + // Mock lookup function to capture options + const mockLookup = (hostname: string, options: any, callback: any) => { + capturedOptions = { ...options }; + // Call the real lookup to get actual results + originalLookup(hostname, options, callback); + }; + + try { + // Test with family: 6 + const agent6 = new https.Agent({ family: 6 }); + expect(agent6.options.family).toBe(6); + + // Create request that should use DNS lookup + const req = https.request({ + hostname: 'example.com', // Use a hostname that requires DNS lookup + path: '/', + agent: agent6, + lookup: mockLookup + }, (res) => { + // Don't need to handle response for this test + }); + + // End the request to trigger DNS lookup + req.end(); + + // Wait for DNS lookup to be called + await new Promise(resolve => setTimeout(resolve, 100)); + + // Verify family option was passed to DNS lookup + expect(capturedOptions).not.toBeNull(); + expect(capturedOptions.family).toBe(6); + expect(capturedOptions.all).toBe(true); + + req.destroy(); + } finally { + // Restore original lookup + dns.lookup = originalLookup; + } + }); + + test("should pass family: 4 option to DNS lookup", async () => { + let capturedOptions: any = null; + const originalLookup = dns.lookup; + + const mockLookup = (hostname: string, options: any, callback: any) => { + capturedOptions = { ...options }; + originalLookup(hostname, options, callback); + }; + + try { + const agent4 = new https.Agent({ family: 4 }); + expect(agent4.options.family).toBe(4); + + const req = https.request({ + hostname: 'example.com', + path: '/', + agent: agent4, + lookup: mockLookup + }, (res) => {}); + + req.end(); + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(capturedOptions).not.toBeNull(); + expect(capturedOptions.family).toBe(4); + expect(capturedOptions.all).toBe(true); + + req.destroy(); + } finally { + dns.lookup = originalLookup; + } + }); + + test("should not pass family option when not specified", async () => { + let capturedOptions: any = null; + const originalLookup = dns.lookup; + + const mockLookup = (hostname: string, options: any, callback: any) => { + capturedOptions = { ...options }; + originalLookup(hostname, options, callback); + }; + + try { + const agent = new https.Agent(); // No family specified + expect(agent.options.family).toBeUndefined(); + + const req = https.request({ + hostname: 'example.com', + path: '/', + agent: agent, + lookup: mockLookup + }, (res) => {}); + + req.end(); + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(capturedOptions).not.toBeNull(); + expect(capturedOptions.family).toBeUndefined(); + expect(capturedOptions.all).toBe(true); + + req.destroy(); + } finally { + dns.lookup = originalLookup; + } + }); + + test("should work with different hosts and preserve agent family setting", () => { + const agent6 = new https.Agent({ family: 6 }); + const agent4 = new https.Agent({ family: 4 }); + + // Test that agent maintains its family setting + expect(agent6.options.family).toBe(6); + expect(agent4.options.family).toBe(4); + + // Test that agent name includes family for connection pooling + const name6 = agent6.getName({ host: 'example.com', port: 443, family: 6 }); + const name4 = agent4.getName({ host: 'example.com', port: 443, family: 4 }); + + expect(name6).toContain(':6'); + expect(name4).toContain(':4'); + }); +}); \ No newline at end of file diff --git a/test/regression/issue/ipv6-https-agent-family.test.ts b/test/regression/issue/ipv6-https-agent-family.test.ts new file mode 100644 index 0000000000..64d3ca49a9 --- /dev/null +++ b/test/regression/issue/ipv6-https-agent-family.test.ts @@ -0,0 +1,73 @@ +import { test, expect, describe } from "bun:test"; +import https from "node:https"; + +describe("IPv6 HTTPS Agent family regression test", () => { + test("HTTPS agent should pass family option to DNS lookup", () => { + // This is a regression test for the issue where IPv6 family setting + // in https.Agent was being ignored during DNS resolution + + let dnsLookupOptions: any = null; + + // Mock lookup to capture the options passed to DNS + const mockLookup = (hostname: string, options: any, callback: any) => { + dnsLookupOptions = { ...options }; + // Return mock IPv6 addresses + callback(null, [ + { address: "2001:db8::1", family: 6 }, + { address: "2001:db8::2", family: 6 } + ]); + }; + + const httpsAgent = new https.Agent({ family: 6 }); + + // Create request similar to the user's code + const req = https.request({ + hostname: 'test.example.com', + path: '/ip', + agent: httpsAgent, + lookup: mockLookup + }, (res) => {}); + + req.on('error', (err) => { + // Expected since we're using mock addresses + }); + + req.end(); + + return new Promise(resolve => { + setTimeout(() => { + // Verify that DNS lookup was called with family: 6 + expect(dnsLookupOptions).not.toBeNull(); + expect(dnsLookupOptions.family).toBe(6); + expect(dnsLookupOptions.all).toBe(true); + req.destroy(); + resolve(undefined); + }, 100); + }); + }); + + test("family option should be ignored when using IP address directly", () => { + // When hostname is already an IP, family option should be ignored + // but agent should still store the option correctly + + const agent = new https.Agent({ family: 6 }); + expect(agent.options.family).toBe(6); + + // Using an IP address directly - this should skip DNS lookup entirely + // The family option should still be stored in agent but not used for connection + const req = https.request({ + hostname: '127.0.0.1', // IPv4 address + path: '/', + port: 8443, + agent: agent + }, (res) => {}); + + req.on('error', (err) => { + // Expected connection error since no server is listening + }); + + // This should not throw or crash + req.end(); + req.destroy(); + }); +}); \ No newline at end of file