8000 http,https: handle IPv6 with proxies · nodejs/node@6695067 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6695067

Browse files
joyeecheungtargos
joyeecheung
authored and
targos
committed
http,https: handle IPv6 with proxies
This simplifies the proxy configuration handling co 8000 de, 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 PR-URL: #59894 Refs: #57872 Reviewed-By: Aditi Singh <aditisingh1400@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent b970c0b commit 6695067

File tree

6 files changed

+208
-21
lines changed

6 files changed

+208
-21
lines changed

‎lib/https.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ let debug = require('internal/util/debuglog').debuglog('https', (fn) => {
6868
const net = require('net');
6969
const { URL, urlToHttpOptions, isURL } = require('internal/url');
7070
const { validateObject } = require('internal/validators');
71-
const { isIP, isIPv6 } = require('internal/net');
71+
const { isIP } = require('internal/net');
7272
const assert = require('internal/assert');
7373
const { getOptionValue } = require('internal/options');
7474

@@ -171,7 +171,11 @@ function getTunnelConfigForProxiedHttps(agent, reqOptions) {
171171
}
172172
const { auth, href } = agent[kProxyConfig];
173173
// The request is a HTTPS request, assemble the payload for establishing the tunnel.
174-
const requestHost = isIPv6(reqOptions.host) ? `[${reqOptions.host}]` : reqOptions.host;
174+
const ipType = isIP(reqOptions.host);
175+
// The request target must put IPv6 address in square brackets.
176+
// Here reqOptions is already processed by urlToHttpOptions so we'll add them back if necessary.
177+
// See https://www.rfc-editor.org/rfc/rfc3986#section-3.2.2
178+
const requestHost = ipType === 6 ? `[${reqOptions.host}]` : reqOptions.host;
175179
const requestPort = reqOptions.port || agent.defaultPort;
176180
const endpoint = `${requestHost}:${requestPort}`;
177181
// The ClientRequest constructor should already have validated the host and the port.
@@ -198,7 +202,7 @@ function getTunnelConfigForProxiedHttps(agent, reqOptions) {
198202
proxyTunnelPayload: payload,
199203
requestOptions: { // Options used for the request sent after the tunnel is established.
200204
__proto__: null,
201-
servername: reqOptions.servername || (isIP(reqOptions.host) ? undefined : reqOptions.host),
205+
servername: reqOptions.servername || ipType ? undefined : reqOptions.host,
202206
...reqOptions,
203207
},
204208
};

‎lib/internal/http.js‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
Date,
5+
Number,
56
NumberParseInt,
67
Symbol,
78
decodeURIComponent,
@@ -99,24 +100,23 @@ function ipToInt(ip) {
99100
* Represents the proxy configuration for an agent. The built-in http and https agent
100101
* implementation have one of this when they are configured to use a proxy.
101102
* @property {string} href - Full URL of the proxy server.
102-
* @property {string} host - Full host including port, e.g. 'localhost:8080'.
103-
* @property {string} hostname - Hostname without brackets for IPv6 addresses.
104-
* @property {number} port - Port number of the proxy server.
105-
* @property {string} protocol - Protocol of the proxy server, e.g. 'http:' or 'https:'.
103+
* @property {string} protocol - Proxy protocol used to talk to the proxy server.
106104
* @property {string|undefined} auth - proxy-authorization header value, if username or password is provided.
107105
* @property {Array<string>} bypassList - List of hosts to bypass the proxy.
108106
* @property {object} proxyConnectionOptions - Options for connecting to the proxy server.
109107
*/
110108
class ProxyConfig {
111109
constructor(proxyUrl, keepAlive, noProxyList) {
112-
const { host, hostname, port, protocol, username, password } = new URL(proxyUrl);
113-
this.href = proxyUrl; // Full URL of the proxy server.
114-
this.host = host; // Full host including port, e.g. 'localhost:8080'.
115-
// Trim off the brackets from IPv6 addresses. As it's parsed from a valid URL, an opening
116-
// "[" Must already have a matching "]" at the end.
117-
this.hostname = hostname[0] === '[' ? hostname.slice(1, -1) : hostname;
118-
this.port = port ? NumberParseInt(port, 10) : (protocol === 'https:' ? 443 : 80);
119-
this.protocol = protocol; // Protocol of the proxy server, e.g. 'http:' or 'https:'.
110+
let parsedURL;
111+
try {
112+
parsedURL = new URL(proxyUrl);
113+
} catch {
114+
throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${proxyUrl}`);
115+
}
116+
const { hostname, port, protocol, username, password } = parsedURL;
117+
118+
this.href = proxyUrl;
119+
this.protocol = protocol;
120120

121121
if (username || password) {
122122
// If username or password is provided, prepare the proxy-authorization header.
@@ -128,9 +128,13 @@ class ProxyConfig {
128128
} else {
129129
this.bypassList = []; // No bypass list provided.
130130
}
131+
131132
this.proxyConnectionOptions = {
132-
host: this.hostname,
133-
port: this.port,
133+
// The host name comes from parsed URL so if it starts with '[' it must be an IPv6 address
134+
// ending with ']'. Remove the brackets for net.connect().
135+
host: hostname[0] === '[' ? hostname.slice(1, -1) : hostname,
136+
// The port comes from parsed URL so it is either '' or a valid number string.
137+
port: port ? Number(port) : (protocol === 'https:' ? 443 : 80),
134138
};
135139
}
136140

‎test/client-proxy/test-http-proxy-request-invalid-proxy.mjs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// This tests that constructing agents with invalid proxy URLs throws ERR_PROXY_INVALID_CONFIG.
2+
import '../common/index.mjs';
3+
import assert from 'node:assert';
4+
import http from 'node:http';
5+
6+
const testCases = [
7+
{
8+
name: 'invalid IPv4 address',
9+
proxyUrl: 'http://256.256.256.256:8080',
10+
},
11+
{
12+
name: 'invalid IPv6 address',
13+
proxyUrl: 'http://::1:8080',
14+
},
15+
{
16+
name: 'missing host',
17+
proxyUrl: 'http://:8080',
18+
},
19+
{
20+
name: 'non-numeric port',
21+
proxyUrl: 'http://proxy.example.com:port',
22+
},
23+
];
24+
25+
for (const testCase of testCases) {
26+
assert.throws(() => {
27+
new http.Agent({
28+
proxyEnv: {
29+
HTTP_PROXY: testCase.proxyUrl,
30+
},
31+
});
32+
}, {
33+
code: 'ERR_PROXY_INVALID_CONFIG',
34+
message: `Invalid proxy URL: ${testCase.proxyUrl}`,
35+
});
36+
}

‎test/client-proxy/test-http-proxy-request-ipv6.mjs‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// This tests making HTTP requests through an HTTP proxy using IPv6 addresses.
2+
3+
import * as common from '../common/index.mjs';
4+
import assert from 'node:assert';
5+
import http from 'node:http';
6+
import { once } from 'events';
7+
import { createProxyServer, runProxiedRequest } from '../common/proxy-server.js';
8+
9+
if (!common.hasIPv6) {
10+
common.skip('missing IPv6 support');
11+
}
12+
13+
// Start a server to process the final request.
14+
const server = http.createServer(common.mustCall((req, res) => {
15+
res.end('Hello world');
16+
}));
17+
server.on('error', common.mustNotCall((err) => { console.error('Server error', err); }));
18+
server.listen(0);
19+
await once(server, 'listening');
20+
21+
// Start a minimal proxy server.
22+
const { proxy, logs } = createProxyServer();
23+
proxy.listen(0);
24+
await once(proxy, 'listening');
25+
26+
{
27+
const serverHost = `[::1]:${server.address().port}`;
28+
const requestUrl = `http://${serverHost}/test`;
29+
const expectedLogs = [{
30+
method: 'GET',
31+
url: requestUrl,
32+
headers: {
33+
'connection': 'keep-alive',
34+
'proxy-connection': 'keep-alive',
35+
'host': serverHost,
36+
},
37+
}];
38+
39+
const { code, signal, stdout } = await runProxiedRequest({
40+
NODE_USE_ENV_PROXY: 1,
41+
REQUEST_URL: requestUrl,
42+
HTTP_PROXY: `http://[::1]:${proxy.address().port}`,
43+
});
44+
assert.deepStrictEqual(logs, expectedLogs);
45+
assert.match(stdout, /Hello world/);
46+
assert.strictEqual(code, 0);
47+
assert.strictEqual(signal, null);
48+
}
49+
50+
proxy.close();
51+
server.close();

‎test/client-proxy/test-https-proxy-request-ipv6.mjs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// This tests making HTTPS requests through an HTTP proxy using IPv6 addresses.
2+
3+
import * as common from '../common/index.mjs';
4+
import fixtures from '../common/fixtures.js';
5+
import assert from 'node:assert';
6+
import { once } from 'events';
7+
import { createProxyServer, runProxiedRequest } from '../common/proxy-server.js';
8+
9+
if (!common.hasIPv6) {
10+
common.skip('missing IPv6 support');
11+
}
12+
13+
if (!common.hasCrypto) {
14+
common.skip('missing crypto');
15+
}
16+
17+
// https must be dynamically imported so that builds without crypto support
18+
// can skip it.
19+
const { default: https } = await import('node:https');
20+
21+
// Start a server to process the final request.
22+
const server = https.createServer({
23+
cert: fixtures.readKey('agent8-cert.pem'),
24+
key: fixtures.readKey('agent8-key.pem'),
25+
}, common.mustCall((req, res) => {
26+
res.end('Hello world');
27+
}, 2));
28+
server.on('error', common.mustNotCall((err) => { console.error('Server error', err); }));
29+
server.listen(0);
30+
await once(server, 'listening');
31+
32+
// Start a minimal proxy server.
33+
const { proxy, logs } = createProxyServer();
34+
proxy.listen(0);
35+
await once(proxy, 'listening');
36+
37+
{
38+
const serverHost = `localhost:${server.address().port}`;
39+
const requestUrl = `https://${serverHost}/test`;
40+
const expectedLogs = [{
41+
method: 'CONNECT',
42+
url: serverHost,
43+
headers: {
44+
'proxy-connection': 'keep-alive',
45+
'host': serverHost,
46+
},
47+
}];
48+
49+
const { code, signal, stdout } = await runProxiedRequest({
50+
NODE_USE_ENV_PROXY: 1,
51+
REQUEST_URL: requestUrl,
52+
HTTPS_PROXY: `http://[::1]:${proxy.address().port}`,
53+
NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'),
54+
});
55+
assert.deepStrictEqual(logs, expectedLogs);
56+
assert.match(stdout, /Hello world/);
57+
assert.strictEqual(code, 0);
58+
assert.strictEqual(signal, null);
59+
}
60+
61+
// Test with IPv6 address in the request URL.
62+
{
63+
logs.splice(0, logs.length); // Clear the logs.
64+
const serverHost = `[::1]:${server.address().port}`;
65+
const requestUrl = `https://${serverHost}/test`;
66+
const expectedLogs = [{
67+
method: 'CONNECT',
68+
url: serverHost,
69+
headers: {
70+
'proxy-connection': 'keep-alive',
71+
'host': serverHost,
72+
},
73+
}];
74+
75+
const { code, signal, stdout } = await runProxiedRequest({
76+
NODE_USE_ENV_PROXY: 1,
77+
REQUEST_URL: requestUrl,
78+
HTTPS_PROXY: `http://[::1]:${proxy.address().port}`,
79+
// Disable certificate verification for this request, for we don't have
80+
// a certificate for [::1].
81+
NODE_TLS_REJECT_UNAUTHORIZED: '0',
82+
});
83+
assert.deepStrictEqual(logs, expectedLogs);
84+
assert.match(stdout, /Hello world/);
85+
assert.strictEqual(code, 0);
86+
assert.strictEqual(signal, null);
87+
}
88+
89+
proxy.close();
90+
server.close();

‎test/common/proxy-server.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ exports.createProxyServer = function(options = {}) {
3232
}
3333
proxy.on('request', (req, res) => {
3434
logRequest(logs, req);
35-
const [hostname, port] = req.headers.host.split(':');
35+
const { hostname, port } = new URL(`http://${req.headers.host}`);
3636
const targetPort = port || 80;
3737

3838
const url = new URL(req.url);
3939
const options = {
40-
hostname: hostname,
40+
hostname: hostname.startsWith('[') ? hostname.slice(1, -1) : hostname,
4141
port: targetPort,
4242
path: url.pathname + url.search, // Convert back to relative URL.
4343
method: req.method,
@@ -72,13 +72,15 @@ exports.createProxyServer = function(options = {}) {
7272
proxy.on('connect', (req, res, head) => {
7373
logRequest(logs, req);
7474

75-
const [hostname, port] = req.url.split(':');
75+
const { hostname, port } = new URL(`https://${req.url}`);
7676

7777
res.on('error', (err) => {
7878
logs.push({ error: err, source: 'client response for connect' });
7979
});
8080

81-
const proxyReq = net.connect(port, hostname, () => {
81+
const normalizedHostname = hostname.startsWith('[') && hostname.endsWith(']') ?
82+
hostname.slice(1, -1) : hostname;
83+
const proxyReq = net.connect(port, normalizedHostname, () => {
8284
res.write(
8385
'HTTP/1.1 200 Connection Established\r\n' +
8486
'Proxy-agent: Node.js-Proxy\r\n' +

0 commit comments

Comments
 (0)
0