Skip to content

Commit 096f459

Browse files
committed
fixup! test: guard write to proxy client if proxy connection is ended
1 parent 41eb0a6 commit 096f459

File tree

2 files changed

+19
-13
lines changed

2 files changed

+19
-13
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: 3 additions & 3 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,7 +90,7 @@ exports.createProxyServer = function(options = {}) {
9090
});
9191

9292
proxyReq.on('error', (err) => {
93-
logs.push({ error: err, source: 'proxy request' });
93+
logs.push({ error: err, source: 'proxy connect' });
9494
// The proxy client might have already closed the connection
9595
// when the upstream connection fails.
9696
if (!res.writableEnded) {

0 commit comments

Comments
 (0)