http: fix perf_hooks detail.req.url port and proxied path · nodejs/node@55e2c2d · GitHub
Skip to content

Commit 55e2c2d

Browse files
stefanobaghinoaduh95
stefanobaghino
authored and
aduh95
committed
http: fix perf_hooks detail.req.url port and proxied path
The perf_hooks HTTP client entry built the reported URL from the bare hostname, dropping non-default ports and IPv6 brackets, and appended the request path even after it had been rewritten to absolute-form for proxying, duplicating the protocol and authority. Report the connection authority captured at request creation and, for proxied requests, use the rewritten absolute-form target as-is. Fixes: #59625 Signed-off-by: Stefano Baghino <stefano@baghino.me> PR-URL: #64311 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: theanarkh <theratliter@gmail.com>
1 parent d31c168 commit 55e2c2d

3 files changed

Lines changed: 86 additions & 7 deletions

File tree

‎lib/_http_client.js‎

Lines changed: 12 additions & 1 deletion

‎test/client-proxy/test-http-proxy-perf-hooks.mjs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// This tests that when a request path is rewritten to absolute-form for
2+
// proxying, the perf_hooks HTTP entries report it as the URL as-is, instead
3+
// of appending it to the protocol and authority again.
4+
// Refs: https://github.com/nodejs/node/issues/59625
5+
import * as common from '../common/index.mjs';
6+
import assert from 'node:assert';
7+
import { once } from 'events';
8+
import http from 'node:http';
9+
import { PerformanceObserver } from 'node:perf_hooks';
10+
import { createProxyServer } from '../common/proxy-server.js';
11+
12+
const entries = [];
13+
const obs = new PerformanceObserver(common.mustCallAtLeast((items) => {
14+
entries.push(...items.getEntries());
15+
}));
16+
obs.observe({ type: 'http' });
17+
18+
// Start a server to process the final request.
19+
const server = http.createServer(common.mustCall((req, res) => {
20+
res.end('Hello world');
21+
}));
22+
server.on('error', common.mustNotCall((err) => { console.error('Server error', err); }));
23+
server.listen(0);
24+
await once(server, 'listening');
25+
26+
// Start a minimal proxy server.
27+
const { proxy, logs } = createProxyServer();
28+
proxy.listen(0);
29+
await once(proxy, 'listening');
30+
31+
const requestUrl = `http://localhost:${server.address().port}/test`;
32+
const agent = new http.Agent({
33+
proxyEnv: {
34+
HTTP_PROXY: `http://localhost:${proxy.address().port}`,
35+
},
36+
});
37+
38+
const res = await new Promise((resolve, reject) => {
39+
http.request(requestUrl, { agent }, resolve).on('error', reject).end();
40+
});
41+
res.resume();
42+
await once(res, 'end');
43+
44+
// Verify that the request went through the proxy.
45+
assert.strictEqual(logs.length, 1);
46+
assert.strictEqual(logs[0].url, requestUrl);
47+
48+
proxy.close();
49+
server.close();
50+
51+
process.on('exit', () => {
52+
// Two HttpClient entries are expected: one for the proxied request, one for
53+
// the request the proxy makes to forward it. Both should report the full
54+
// URL, including the port.
55+
const clientUrls = entries.filter((entry) => entry.name === 'HttpClient')
56+
.map((entry) => entry.detail.req.url);
57+
assert.deepStrictEqual(clientUrls, [requestUrl, requestUrl]);
58+
// Two HttpRequest entries are expected: the proxy server receives the
59+
// request target in absolute-form, the final server in origin-form.
60+
const requestUrls = entries.filter((entry) => entry.name === 'HttpRequest')
61+
.map((entry) => entry.detail.req.url).sort();
62+
assert.deepStrictEqual(requestUrls, ['/test', requestUrl].sort());
63+
});

‎test/parallel/test-http-perf_hooks.js‎

Lines changed: 11 additions & 6 deletions

0 commit comments

Comments
 (0)