Skip to content

Commit 8f32746

Browse files
joyeecheungtargos
authored andcommitted
test: guard write to proxy client if proxy connection is ended
In the testing proxy server for proxy client tests, the proxy client might have already closed the connection when the upstream connection fails. In that case, there's no need for the proxy server to inform the proxy client about the error. PR-URL: #59742 Fixes: #59741 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 728472a commit 8f32746

File tree

2 files changed

+25
-15
lines changed

2 files changed

+25
-15
lines changed

test/client-proxy/test-https-proxy-request-invalid-char-in-url.mjs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@ const server = https.createServer({
2121
cert: fixtures.readKey('agent8-cert.pem'),
2222
key: fixtures.readKey('agent8-key.pem'),
2323
}, (req, res) => {
24+
console.log(`[Upstream server] responding to request for ${inspect(req.url)}`);
2425
requests.add(`https://localhost:${server.address().port}${req.url}`);
2526
res.writeHead(200, { 'Content-Type': 'text/plain' });
26-
res.end(`Response for ${req.url}`);
27+
res.end(`Response for ${inspect(req.url)}`);
2728
});
2829

2930
server.listen(0);
@@ -54,7 +55,7 @@ https.globalAgent = new https.Agent({
5455

5556
const severHost = `localhost:${server.address().port}`;
5657

57-
let counter = testCases.length;
58+
let counter = 0;
5859
const expectedUrls = new Set();
5960
const expectedProxyLogs = new Set();
6061
for (const testCase of testCases) {
@@ -69,15 +70,20 @@ for (const testCase of testCases) {
6970
https.request(url, (res) => {
7071
res.on('error', common.mustNotCall());
7172
res.setEncoding('utf8');
72-
res.on('data', () => {});
73-
res.on('end', common.mustCall(() => {
74-
console.log(`#${counter--} eneded response for: ${inspect(url)}`);
73+
res.on('data', (data) => {
74+
console.log(`[Proxy client] Received response from server for ${inspect(url)}: ${data.toString()}`);
75+
});
76+
res.on('close', common.mustCall(() => {
77+
console.log(`[Proxy client] #${++counter} closed request for: ${inspect(url)}`);
7578
// Finished all test cases.
76-
if (counter === 0) {
77-
proxy.close();
78-
server.close();
79-
assert.deepStrictEqual(requests, expectedUrls);
80-
assert.deepStrictEqual(new Set(logs), expectedProxyLogs);
79+
if (counter === testCases.length) {
80+
setImmediate(() => {
81+
console.log('All requests completed, shutting down.');
82+
proxy.close();
83+
server.close();
84+
assert.deepStrictEqual(requests, expectedUrls);
85+
assert.deepStrictEqual(new Set(logs), expectedProxyLogs);
86+
});
8187
}
8288
}));
8389
}).on('error', common.mustNotCall()).end();

test/common/proxy-server.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ exports.createProxyServer = function(options = {}) {
6363
});
6464

6565
res.on('error', (err) => {
66-
logs.push({ error: err, source: 'proxy response' });
66+
logs.push({ error: err, source: 'client response for request' });
6767
});
6868

6969
req.pipe(proxyReq, { end: true });
@@ -75,7 +75,7 @@ exports.createProxyServer = function(options = {}) {
7575
const [hostname, port] = req.url.split(':');
7676

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

8181
const proxyReq = net.connect(port, hostname, () => {
@@ -90,9 +90,13 @@ exports.createProxyServer = function(options = {}) {
9090
});
9191

9292
proxyReq.on('error', (err) => {
93-
logs.push({ error: err, source: 'proxy request' });
94-
res.write('HTTP/1.1 500 Connection Error\r\n\r\n');
95-
res.end('Proxy error: ' + err.message);
93+
logs.push({ error: err, source: 'proxy connect' });
94+
// The proxy client might have already closed the connection
95+
// when the upstream connection fails.
96+
if (!res.writableEnded) {
97+
res.write('HTTP/1.1 500 Connection Error\r\n\r\n');
98+
res.end('Proxy error: ' + err.message);
99+
}
96100
});
97101
});
98102

0 commit comments

Comments
 (0)