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' +