From 35d48f8da7151fb9ff1fbe6ee3ea2d48c68dae91 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 15 Sep 2025 19:24:51 +0200 Subject: [PATCH] http,https: handle IPv6 with proxies This simplifies the proxy configuration handling code, adds tests to make sure the proxy support works with IPv6 and throws correct errors for invalid proxy IPs. Drive-by: remove useless properties from ProxyConfig --- lib/https.js | 10 ++- lib/internal/http.js | 32 ++++--- .../test-http-proxy-request-invalid-proxy.mjs | 36 ++++++++ .../test-http-proxy-request-ipv6.mjs | 51 +++++++++++ .../test-https-proxy-request-ipv6.mjs | 90 +++++++++++++++++++ test/common/proxy-server.js | 10 ++- 6 files changed, 208 insertions(+), 21 deletions(-) create mode 100644 test/client-proxy/test-http-proxy-request-invalid-proxy.mjs create mode 100644 test/client-proxy/test-http-proxy-request-ipv6.mjs create mode 100644 test/client-proxy/test-https-proxy-request-ipv6.mjs diff --git a/lib/https.js b/lib/https.js index fd9ae36e7b4d6f..799a8f197c7347 100644 --- a/lib/https.js +++ b/lib/https.js @@ -68,7 +68,7 @@ let debug = require('internal/util/debuglog').debuglog('https', (fn) => { const net = require('net'); const { URL, urlToHttpOptions, isURL } = require('internal/url'); const { validateObject } = require('internal/validators'); -const { isIP, isIPv6 } = require('internal/net'); +const { isIP } = require('internal/net'); const assert = require('internal/assert'); const { getOptionValue } = require('internal/options'); @@ -171,7 +171,11 @@ function getTunnelConfigForProxiedHttps(agent, reqOptions) { } const { auth, href } = agent[kProxyConfig]; // The request is a HTTPS request, assemble the payload for establishing the tunnel. - const requestHost = isIPv6(reqOptions.host) ? `[${reqOptions.host}]` : reqOptions.host; + const ipType = isIP(reqOptions.host); + // The request target must put IPv6 address in square brackets. + // Here reqOptions is already processed by urlToHttpOptions so we'll add them back if necessary. + // See https://www.rfc-editor.org/rfc/rfc3986#section-3.2.2 + const requestHost = ipType === 6 ? `[${reqOptions.host}]` : reqOptions.host; const requestPort = reqOptions.port || agent.defaultPort; const endpoint = `${requestHost}:${requestPort}`; // The ClientRequest constructor should already have validated the host and the port. @@ -198,7 +202,7 @@ function getTunnelConfigForProxiedHttps(agent, reqOptions) { proxyTunnelPayload: payload, requestOptions: { // Options used for the request sent after the tunnel is established. __proto__: null, - servername: reqOptions.servername || (isIP(reqOptions.host) ? undefined : reqOptions.host), + servername: reqOptions.servername || ipType ? undefined : reqOptions.host, ...reqOptions, }, }; diff --git a/lib/internal/http.js b/lib/internal/http.js index aa3ec354dabf4a..9bf929f7f3360f 100644 --- a/lib/internal/http.js +++ b/lib/internal/http.js @@ -2,6 +2,7 @@ const { Date, + Number, NumberParseInt, Symbol, decodeURIComponent, @@ -99,24 +100,23 @@ function ipToInt(ip) { * Represents the proxy configuration for an agent. The built-in http and https agent * implementation have one of this when they are configured to use a proxy. * @property {string} href - Full URL of the proxy server. - * @property {string} host - Full host including port, e.g. 'localhost:8080'. - * @property {string} hostname - Hostname without brackets for IPv6 addresses. - * @property {number} port - Port number of the proxy server. - * @property {string} protocol - Protocol of the proxy server, e.g. 'http:' or 'https:'. + * @property {string} protocol - Proxy protocol used to talk to the proxy server. * @property {string|undefined} auth - proxy-authorization header value, if username or password is provided. * @property {Array} bypassList - List of hosts to bypass the proxy. * @property {object} proxyConnectionOptions - Options for connecting to the proxy server. */ class ProxyConfig { constructor(proxyUrl, keepAlive, noProxyList) { - const { host, hostname, port, protocol, username, password } = new URL(proxyUrl); - this.href = proxyUrl; // Full URL of the proxy server. - this.host = host; // Full host including port, e.g. 'localhost:8080'. - // Trim off the brackets from IPv6 addresses. As it's parsed from a valid URL, an opening - // "[" Must already have a matching "]" at the end. - this.hostname = hostname[0] === '[' ? hostname.slice(1, -1) : hostname; - this.port = port ? NumberParseInt(port, 10) : (protocol === 'https:' ? 443 : 80); - this.protocol = protocol; // Protocol of the proxy server, e.g. 'http:' or 'https:'. + let parsedURL; + try { + parsedURL = new URL(proxyUrl); + } catch { + throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${proxyUrl}`); + } + const { hostname, port, protocol, username, password } = parsedURL; + + this.href = proxyUrl; + this.protocol = protocol; if (username || password) { // If username or password is provided, prepare the proxy-authorization header. @@ -128,9 +128,13 @@ class ProxyConfig { } else { this.bypassList = []; // No bypass list provided. } + this.proxyConnectionOptions = { - host: this.hostname, - port: this.port, + // The host name comes from parsed URL so if it starts with '[' it must be an IPv6 address + // ending with ']'. Remove the brackets for net.connect(). + host: hostname[0] === '[' ? hostname.slice(1, -1) : hostname, + // The port comes from parsed URL so it is either '' or a valid number string. + port: port ? Number(port) : (protocol === 'https:' ? 443 : 80), }; } diff --git a/test/client-proxy/test-http-proxy-request-invalid-proxy.mjs b/test/client-proxy/test-http-proxy-request-invalid-proxy.mjs new file mode 100644 index 00000000000000..9197ebf57c6b01 --- /dev/null +++ b/test/client-proxy/test-http-proxy-request-invalid-proxy.mjs @@ -0,0 +1,36 @@ +// This tests that constructing agents with invalid proxy URLs throws ERR_PROXY_INVALID_CONFIG. +import '../common/index.mjs'; +import assert from 'node:assert'; +import http from 'node:http'; + +const testCases = [ + { + name: 'invalid IPv4 address', + proxyUrl: 'http://256.256.256.256:8080', + }, + { + name: 'invalid IPv6 address', + proxyUrl: 'http://::1:8080', + }, + { + name: 'missing host', + proxyUrl: 'http://:8080', + }, + { + name: 'non-numeric port', + proxyUrl: 'http://proxy.example.com:port', + }, +]; + +for (const testCase of testCases) { + assert.throws(() => { + new http.Agent({ + proxyEnv: { + HTTP_PROXY: testCase.proxyUrl, + }, + }); + }, { + code: 'ERR_PROXY_INVALID_CONFIG', + message: `Invalid proxy URL: ${testCase.proxyUrl}`, + }); +} diff --git a/test/client-proxy/test-http-proxy-request-ipv6.mjs b/test/client-proxy/test-http-proxy-request-ipv6.mjs new file mode 100644 index 00000000000000..af27a647873ddd --- /dev/null +++ b/test/client-proxy/test-http-proxy-request-ipv6.mjs @@ -0,0 +1,51 @@ +// This tests making HTTP requests through an HTTP proxy using IPv6 addresses. + +import * as common from '../common/index.mjs'; +import assert from 'node:assert'; +import http from 'node:http'; +import { once } from 'events'; +import { createProxyServer, runProxiedRequest } from '../common/proxy-server.js'; + +if (!common.hasIPv6) { + common.skip('missing IPv6 support'); +} + +// Start a server to process the final request. +const server = http.createServer(common.mustCall((req, res) => { + res.end('Hello world'); +})); +server.on('error', common.mustNotCall((err) => { console.error('Server error', err); })); +server.listen(0); +await once(server, 'listening'); + +// Start a minimal proxy server. +const { proxy, logs } = createProxyServer(); +proxy.listen(0); +await once(proxy, 'listening'); + +{ + const serverHost = `[::1]:${server.address().port}`; + const requestUrl = `http://${serverHost}/test`; + const expectedLogs = [{ + method: 'GET', + url: requestUrl, + headers: { + 'connection': 'keep-alive', + 'proxy-connection': 'keep-alive', + 'host': serverHost, + }, + }]; + + const { code, signal, stdout } = await runProxiedRequest({ + NODE_USE_ENV_PROXY: 1, + REQUEST_URL: requestUrl, + HTTP_PROXY: `http://[::1]:${proxy.address().port}`, + }); + assert.deepStrictEqual(logs, expectedLogs); + assert.match(stdout, /Hello world/); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +} + +proxy.close(); +server.close(); diff --git a/test/client-proxy/test-https-proxy-request-ipv6.mjs b/test/client-proxy/test-https-proxy-request-ipv6.mjs new file mode 100644 index 00000000000000..c2baecf8358c5a --- /dev/null +++ b/test/client-proxy/test-https-proxy-request-ipv6.mjs @@ -0,0 +1,90 @@ +// This tests making HTTPS requests through an HTTP proxy using IPv6 addresses. + +import * as common from '../common/index.mjs'; +import fixtures from '../common/fixtures.js'; +import assert from 'node:assert'; +import { once } from 'events'; +import { createProxyServer, runProxiedRequest } from '../common/proxy-server.js'; + +if (!common.hasIPv6) { + common.skip('missing IPv6 support'); +} + +if (!common.hasCrypto) { + common.skip('missing crypto'); +} + +// https must be dynamically imported so that builds without crypto support +// can skip it. +const { default: https } = await import('node:https'); + +// Start a server to process the final request. +const server = https.createServer({ + cert: fixtures.readKey('agent8-cert.pem'), + key: fixtures.readKey('agent8-key.pem'), +}, common.mustCall((req, res) => { + res.end('Hello world'); +}, 2)); +server.on('error', common.mustNotCall((err) => { console.error('Server error', err); })); +server.listen(0); +await once(server, 'listening'); + +// Start a minimal proxy server. +const { proxy, logs } = createProxyServer(); +proxy.listen(0); +await once(proxy, 'listening'); + +{ + const serverHost = `localhost:${server.address().port}`; + const requestUrl = `https://${serverHost}/test`; + const expectedLogs = [{ + method: 'CONNECT', + url: serverHost, + headers: { + 'proxy-connection': 'keep-alive', + 'host': serverHost, + }, + }]; + + const { code, signal, stdout } = await runProxiedRequest({ + NODE_USE_ENV_PROXY: 1, + REQUEST_URL: requestUrl, + HTTPS_PROXY: `http://[::1]:${proxy.address().port}`, + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), + }); + assert.deepStrictEqual(logs, expectedLogs); + assert.match(stdout, /Hello world/); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +} + +// Test with IPv6 address in the request URL. +{ + logs.splice(0, logs.length); // Clear the logs. + const serverHost = `[::1]:${server.address().port}`; + const requestUrl = `https://${serverHost}/test`; + const expectedLogs = [{ + method: 'CONNECT', + url: serverHost, + headers: { + 'proxy-connection': 'keep-alive', + 'host': serverHost, + }, + }]; + + const { code, signal, stdout } = await runProxiedRequest({ + NODE_USE_ENV_PROXY: 1, + REQUEST_URL: requestUrl, + HTTPS_PROXY: `http://[::1]:${proxy.address().port}`, + // Disable certificate verification for this request, for we don't have + // a certificate for [::1]. + NODE_TLS_REJECT_UNAUTHORIZED: '0', + }); + assert.deepStrictEqual(logs, expectedLogs); + assert.match(stdout, /Hello world/); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +} + +proxy.close(); +server.close(); diff --git a/test/common/proxy-server.js b/test/common/proxy-server.js index 29a4073e13467a..44dee0060cdc2a 100644 --- a/test/common/proxy-server.js +++ b/test/common/proxy-server.js @@ -32,12 +32,12 @@ exports.createProxyServer = function(options = {}) { } proxy.on('request', (req, res) => { logRequest(logs, req); - const [hostname, port] = req.headers.host.split(':'); + const { hostname, port } = new URL(`http://${req.headers.host}`); const targetPort = port || 80; const url = new URL(req.url); const options = { - hostname: hostname, + hostname: hostname.startsWith('[') ? hostname.slice(1, -1) : hostname, port: targetPort, path: url.pathname + url.search, // Convert back to relative URL. method: req.method, @@ -72,13 +72,15 @@ exports.createProxyServer = function(options = {}) { proxy.on('connect', (req, res, head) => { logRequest(logs, req); - const [hostname, port] = req.url.split(':'); + const { hostname, port } = new URL(`https://${req.url}`); res.on('error', (err) => { logs.push({ error: err, source: 'proxy response' }); }); - const proxyReq = net.connect(port, hostname, () => { + const normalizedHostname = hostname.startsWith('[') && hostname.endsWith(']') ? + hostname.slice(1, -1) : hostname; + const proxyReq = net.connect(port, normalizedHostname, () => { res.write( 'HTTP/1.1 200 Connection Established\r\n' + 'Proxy-agent: Node.js-Proxy\r\n' +