Skip to content

Commit 822460a

Browse files
committed
fixup! fixup! http,https: add built-in proxy support in http/https.request and Agent
1 parent 26e0f77 commit 822460a

8 files changed

+295
-3
lines changed

lib/internal/errors.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1669,8 +1669,8 @@ E('ERR_PARSE_ARGS_UNKNOWN_OPTION', (option, allowPositionals) => {
16691669
E('ERR_PERFORMANCE_INVALID_TIMESTAMP',
16701670
'%d is not a valid timestamp', TypeError);
16711671
E('ERR_PERFORMANCE_MEASURE_INVALID_OPTIONS', '%s', TypeError);
1672-
E('ERR_PROXY_TUNNEL', '%s', Error);
16731672
E('ERR_PROXY_INVALID_CONFIG', '%s', Error);
1673+
E('ERR_PROXY_TUNNEL', '%s', Error);
16741674
E('ERR_QUIC_APPLICATION_ERROR', 'A QUIC application error occurred. %d [%s]', Error);
16751675
E('ERR_QUIC_CONNECTION_FAILED', 'QUIC connection failed', Error);
16761676
E('ERR_QUIC_ENDPOINT_CLOSED', 'QUIC endpoint closed: %s (%d)', Error);
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// This tests that invalid hostnames or ports with carriage return or newline characters
2+
// in HTTP request options would lead to ERR_INVALID_CHAR.
3+
4+
import * as common from '../common/index.mjs';
5+
import assert from 'node:assert';
6+
import http from 'node:http';
7+
import { once } from 'events';
8+
9+
const server = http.createServer(common.mustNotCall());
10+
server.listen(0);
11+
await once(server, 'listening');
12+
server.on('error', common.mustNotCall());
13+
const port = server.address().port.toString();
14+
15+
const testCases = [
16+
{ hostname: 'local\rost', port: port, path: '/carriage-return-in-hostname' },
17+
{ hostname: 'local\nhost', port: port, path: '/newline-in-hostname' },
18+
{ hostname: 'local\r\nhost', port: port, path: '/crlf-in-hostname' },
19+
{ hostname: 'localhost', port: port.substring(0, 1) + '\r' + port.substring(1), path: '/carriage-return-in-port' },
20+
{ hostname: 'localhost', port: port.substring(0, 1) + '\n' + port.substring(1), path: '/newline-in-port' },
21+
{ hostname: 'localhost', port: port.substring(0, 1) + '\r\n' + port.substring(1), path: '/crlf-in-port' },
22+
];
23+
24+
const proxy = http.createServer(common.mustNotCall());
25+
proxy.listen(0);
26+
await once(proxy, 'listening');
27+
28+
const agent = new http.Agent({
29+
proxyEnv: {
30+
HTTP_PROXY: `http://localhost:${proxy.address().port}`,
31+
},
32+
});
33+
34+
for (const testCase of testCases) {
35+
const options = { ...testCase, agent };
36+
assert.throws(() => {
37+
http.request(options, common.mustNotCall());
38+
}, {
39+
code: 'ERR_INVALID_CHAR',
40+
});
41+
}
42+
43+
server.close();
44+
proxy.close();
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// This tests that invalid hostnames or ports with carriage return or newline characters
2+
// in HTTP request urls are stripped away before being sent to the server.
3+
4+
import * as common from '../common/index.mjs';
5+
import assert from 'node:assert';
6+
import http from 'node:http';
7+
import { once } from 'events';
8+
import { inspect } from 'node:util';
9+
import fixtures from '../common/fixtures.js';
10+
import { Worker } from 'node:worker_threads';
11+
12+
const expectedProxyLogs = new Set();
13+
const proxyWorker = new Worker(fixtures.path('proxy-server-worker.js'));
14+
15+
proxyWorker.on('message', (message) => {
16+
console.log('Received message from worker:', message.type);
17+
if (message.type === 'proxy-listening') {
18+
startTest(message.port);
19+
} else if (message.type === 'proxy-stopped') {
20+
assert.deepStrictEqual(new Set(message.logs), expectedProxyLogs);
21+
// Close the server after the proxy is stopped.
22+
proxyWorker.terminate();
23+
}
24+
});
25+
26+
const requests = new Set();
27+
// Create a server that records the requests it gets.
28+
const server = http.createServer((req, res) => {
29+
requests.add(`http://localhost:${server.address().port}${req.url}`);
30+
res.writeHead(200, { 'Content-Type': 'text/plain' });
31+
res.end(`Response for ${req.url}`);
32+
});
33+
proxyWorker.on('exit', common.mustCall(() => {
34+
server.close();
35+
}));
36+
37+
async function startTest(proxyPort) {
38+
// Start a minimal proxy server in a worker, we don't do it in this thread
39+
// because we'll mutate the global http agent here.
40+
http.globalAgent = new http.Agent({
41+
proxyEnv: {
42+
HTTP_PROXY: `http://localhost:${proxyPort}`,
43+
},
44+
});
45+
46+
server.listen(0);
47+
await once(server, 'listening');
48+
server.on('error', common.mustNotCall());
49+
const port = server.address().port.toString();
50+
51+
const testCases = [
52+
{ hostname: 'local\rhost', port: port, path: '/carriage-return-in-hostname' },
53+
{ hostname: 'local\nhost', port: port, path: '/newline-in-hostname' },
54+
{ hostname: 'local\r\nhost', port: port, path: '/crlf-in-hostname' },
55+
{ hostname: 'localhost', port: port.substring(0, 1) + '\r' + port.substring(1), path: '/carriage-return-in-port' },
56+
{ hostname: 'localhost', port: port.substring(0, 1) + '\n' + port.substring(1), path: '/newline-in-port' },
57+
{ hostname: 'localhost', port: port.substring(0, 1) + '\r\n' + port.substring(1), path: '/crlf-in-port' },
58+
];
59+
const severHost = `localhost:${server.address().port}`;
60+
61+
let counter = testCases.length;
62+
const expectedUrls = new Set();
63+
for (const testCase of testCases) {
64+
const url = `http://${testCase.hostname}:${testCase.port}${testCase.path}`;
65+
// The invalid characters should all be stripped away before being sent.
66+
const cleanUrl = url.replaceAll(/\r|\n/g, '');
67+
expectedUrls.add(cleanUrl);
68+
expectedProxyLogs.add({
69+
method: 'GET',
70+
url: cleanUrl,
71+
headers: {
72+
'host': severHost,
73+
'connection': 'close',
74+
'proxy-connection': 'close',
75+
},
76+
});
77+
http.request(url, (res) => {
78+
res.on('error', common.mustNotCall());
79+
res.setEncoding('utf8');
80+
res.on('data', () => {});
81+
res.on('end', common.mustCall(() => {
82+
console.log(`#${counter--} ended response for: ${inspect(url)}`);
83+
// Finished all test cases.
84+
if (counter === 0) {
85+
// Check that the requests received by the server have sanitized URLs.
86+
assert.deepStrictEqual(requests, expectedUrls);
87+
console.log('Sending stop-proxy message to worker');
88+
proxyWorker.postMessage({ type: 'stop-proxy' });
89+
}
90+
}));
91+
}).on('error', common.mustNotCall()).end();
92+
}
93+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// This tests that proxy URLs with invalid credentials (containing \r or \n characters)
22
// are rejected with an appropriate error.
3+
import '../common/index.mjs';
34
import assert from 'node:assert';
45
import http from 'node:http';
56

@@ -45,4 +46,4 @@ for (const testCase of testCases) {
4546
},
4647
});
4748
}, testCase.expectedError);
48-
}
49+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// This tests that invalid hostnames or ports with carriage return or newline characters
2+
// in HTTPsS request options would lead to ERR_INVALID_CHAR.
3+
4+
import * as common from '../common/index.mjs';
5+
import assert from 'node:assert';
6+
import https from 'node:https';
7+
import { once } from 'events';
8+
import fixtures from '../common/fixtures.js';
9+
10+
const server = https.createServer({
11+
cert: fixtures.readKey('agent8-cert.pem'),
12+
key: fixtures.readKey('agent8-key.pem'),
13+
}, common.mustNotCall());
14+
server.listen(0);
15+
await once(server, 'listening');
16+
server.on('error', common.mustNotCall());
17+
const port = server.address().port.toString();
18+
19+
const testCases = [
20+
{ hostname: 'local\rhost', port: port, path: '/carriage-return-in-hostname' },
21+
{ hostname: 'local\nhost', port: port, path: '/newline-in-hostname' },
22+
{ hostname: 'local\r\nhost', port: port, path: '/crlf-in-hostname' },
23+
{ hostname: 'localhost', port: port.substring(0, 1) + '\r' + port.substring(1), path: '/carriage-return-in-port' },
24+
{ hostname: 'localhost', port: port.substring(0, 1) + '\n' + port.substring(1), path: '/newline-in-port' },
25+
{ hostname: 'localhost', port: port.substring(0, 1) + '\r\n' + port.substring(1), path: '/crlf-in-port' },
26+
];
27+
28+
const proxy = https.createServer(common.mustNotCall());
29+
proxy.listen(0);
30+
await once(proxy, 'listening');
31+
32+
const agent = new https.Agent({
33+
ca: fixtures.readKey('fake-startcom-root-cert.pem'),
34+
proxyEnv: {
35+
HTTPS_PROXY: `https://localhost:${proxy.address().port}`,
36+
},
37+
});
38+
39+
for (const testCase of testCases) {
40+
const options = { ...testCase, agent };
41+
assert.throws(() => {
42+
https.request(options, common.mustNotCall());
43+
}, {
44+
code: 'ERR_INVALID_CHAR',
45+
});
46+
}
47+
48+
server.close();
49+
proxy.close();
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// This tests that invalid hostnames or ports with carriage return or newline characters
2+
// in HTTPS request urls are stripped away before being sent to the server.
3+
4+
import * as common from '../common/index.mjs';
5+
import assert from 'node:assert';
6+
import fixtures from '../common/fixtures.js';
7+
import https from 'node:https';
8+
import { once } from 'events';
9+
import { inspect } from 'node:util';
10+
import { createProxyServer } from '../common/proxy-server.js';
11+
12+
if (!common.hasCrypto) {
13+
common.skip('missing crypto');
14+
}
15+
const requests = new Set();
16+
17+
const server = https.createServer({
18+
cert: fixtures.readKey('agent8-cert.pem'),
19+
key: fixtures.readKey('agent8-key.pem'),
20+
}, (req, res) => {
21+
requests.add(`https://localhost:${server.address().port}${req.url}`);
22+
res.writeHead(200, { 'Content-Type': 'text/plain' });
23+
res.end(`Response for ${req.url}`);
24+
});
25+
26+
server.listen(0);
27+
await once(server, 'listening');
28+
server.on('error', common.mustNotCall());
29+
const port = server.address().port.toString();
30+
31+
const testCases = [
32+
{ hostname: 'local\rhost', port: port, path: '/carriage-return-in-hostname' },
33+
{ hostname: 'local\nhost', port: port, path: '/newline-in-hostname' },
34+
{ hostname: 'local\r\nhost', port: port, path: '/crlf-in-hostname' },
35+
{ hostname: 'localhost', port: port.substring(0, 1) + '\r' + port.substring(1), path: '/carriage-return-in-port' },
36+
{ hostname: 'localhost', port: port.substring(0, 1) + '\n' + port.substring(1), path: '/newline-in-port' },
37+
{ hostname: 'localhost', port: port.substring(0, 1) + '\r\n' + port.substring(1), path: '/crlf-in-port' },
38+
];
39+
40+
// Start a minimal proxy server
41+
const { proxy, logs } = createProxyServer();
42+
proxy.listen(0);
43+
await once(proxy, 'listening');
44+
45+
https.globalAgent = new https.Agent({
46+
ca: fixtures.readKey('fake-startcom-root-cert.pem'),
47+
proxyEnv: {
48+
HTTPS_PROXY: `http://localhost:${proxy.address().port}`,
49+
},
50+
});
51+
52+
const severHost = `localhost:${server.address().port}`;
53+
54+
let counter = testCases.length;
55+
const expectedUrls = new Set();
56+
const expectedProxyLogs = new Set();
57+
for (const testCase of testCases) {
58+
const url = `https://${testCase.hostname}:${testCase.port}${testCase.path}`;
59+
// The invalid characters should all be stripped away before being sent.
60+
expectedUrls.add(url.replaceAll(/\r|\n/g, ''));
61+
expectedProxyLogs.add({
62+
method: 'CONNECT',
63+
url: severHost,
64+
headers: { host: severHost },
65+
});
66+
https.request(url, (res) => {
67+
res.on('error', common.mustNotCall());
68+
res.setEncoding('utf8');
69+
res.on('data', () => {});
70+
res.on('end', common.mustCall(() => {
71+
console.log(`#${counter--} eneded response for: ${inspect(url)}`);
72+
// Finished all test cases.
73+
if (counter === 0) {
74+
proxy.close();
75+
server.close();
76+
assert.deepStrictEqual(requests, expectedUrls);
77+
assert.deepStrictEqual(new Set(logs), expectedProxyLogs);
78+
}
79+
}));
80+
}).on('error', common.mustNotCall()).end();
81+
}

test/client-proxy/test-https-proxy-request-invalid-credentials.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,4 @@ for (const testCase of testCases) {
5050
},
5151
});
5252
}, testCase.expectedError);
53-
}
53+
}

test/fixtures/proxy-server-worker.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
3+
const { parentPort } = require('worker_threads');
4+
const { createProxyServer } = require('../common/proxy-server');
5+
6+
const { proxy, logs } = createProxyServer();
7+
proxy.listen(0);
8+
proxy.on('listening', () => {
9+
parentPort.postMessage({
10+
type: 'proxy-listening',
11+
port: proxy.address().port,
12+
});
13+
});
14+
15+
parentPort.on('message', (msg) => {
16+
console.log('Received message from main thread:', msg.type);
17+
if (msg.type === 'stop-proxy') {
18+
parentPort.postMessage({
19+
type: 'proxy-stopped',
20+
logs,
21+
});
22+
proxy.close();
23+
}
24+
});

0 commit comments

Comments
 (0)