Skip to content

Commit b2f6aa3

Browse files
authored
inspector: fix absolute URLs in network http
Some HTTP clients send an absolute URL in request.path. The network inspection code always prefixed request.protocol and host, which produced duplicated URLs such as http://hosthttp://host/path in Network.requestWillBeSent and Network.responseReceived. Handle absolute URL paths directly and keep the existing relative-path behavior unchanged. Extend the HTTP inspector test to cover both GET and POST requests that use an absolute URL as options.path and verify that getResponseBody still returns the expected payload. Signed-off-by: GrinZero <[email protected]> PR-URL: #62955 Reviewed-By: Ryuhei Shima <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent 13e90d0 commit b2f6aa3

2 files changed

Lines changed: 175 additions & 31 deletions

File tree

lib/internal/inspector/network_http.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const {
55
DateNow,
66
ObjectEntries,
77
String,
8+
StringPrototypeStartsWith,
89
Symbol,
910
} = primordials;
1011

@@ -21,6 +22,19 @@ const EventEmitter = require('events');
2122

2223
const kRequestUrl = Symbol('kRequestUrl');
2324

25+
function isAbsoluteURLPath(path) {
26+
return typeof path === 'string' &&
27+
(StringPrototypeStartsWith(path, 'http://') ||
28+
StringPrototypeStartsWith(path, 'https://'));
29+
}
30+
31+
function getRequestURL(request, host) {
32+
if (isAbsoluteURLPath(request.path)) {
33+
return request.path;
34+
}
35+
return `${request.protocol}//${host}${request.path}`;
36+
}
37+
2438
// Convert a Headers object (Map<string, number | string | string[]>) to a plain object (Map<string, string>)
2539
const convertHeaderObject = (headers = {}) => {
2640
// The 'host' header that contains the host and port of the URL.
@@ -62,7 +76,7 @@ function onClientRequestCreated({ request }) {
6276
request[kInspectorRequestId] = getNextRequestId();
6377

6478
const { 0: headers, 1: host, 2: charset } = convertHeaderObject(request.getHeaders());
65-
const url = `${request.protocol}//${host}${request.path}`;
79+
const url = getRequestURL(request, host);
6680
request[kRequestUrl] = url;
6781

6882
Network.requestWillBeSent({

test/parallel/test-inspector-network-http.js

Lines changed: 160 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,28 @@ const setResponseHeaders = (res) => {
3232

3333
const kTimeout = 1000;
3434
const kDelta = 200;
35+
const kDefaultResponseHeaders = {
36+
'server': 'node',
37+
'etag': '12345',
38+
'set-cookie': 'key1=value1\nkey2=value2',
39+
'x-header2': 'value1, value2',
40+
};
41+
42+
function getDefaultResponseExpect(url) {
43+
return {
44+
url,
45+
mimeType: 'text/plain',
46+
charset: 'utf-8',
47+
responseHeaders: kDefaultResponseHeaders,
48+
};
49+
}
50+
51+
function getPathName(req) {
52+
return new URL(req.url, `http://${req.headers.host}`).pathname;
53+
}
3554

3655
const handleRequest = (req, res) => {
37-
const path = req.url;
56+
const path = getPathName(req);
3857
switch (path) {
3958
case '/hello-world':
4059
setResponseHeaders(res);
@@ -46,6 +65,22 @@ const handleRequest = (req, res) => {
4665
res.end('hello world\n');
4766
}, kTimeout);
4867
break;
68+
case '/echo-post': {
69+
const chunks = [];
70+
req.on('data', (chunk) => {
71+
chunks.push(chunk);
72+
});
73+
req.on('end', () => {
74+
const body = Buffer.concat(chunks).toString();
75+
res.setHeader('Content-Type', 'application/json; charset=utf-8');
76+
res.writeHead(200);
77+
res.end(JSON.stringify({
78+
method: req.method,
79+
body,
80+
}));
81+
});
82+
break;
83+
}
4984
default:
5085
assert.fail(`Unexpected path: ${path}`);
5186
}
@@ -77,7 +112,7 @@ function verifyRequestWillBeSent({ method, params }, expect) {
77112

78113
assert.ok(params.requestId.startsWith('node-network-event-'));
79114
assert.strictEqual(params.request.url, expect.url);
80-
assert.strictEqual(params.request.method, 'GET');
115+
assert.strictEqual(params.request.method, expect.method ?? 'GET');
81116
assert.strictEqual(typeof params.request.headers, 'object');
82117
assert.strictEqual(params.request.headers['accept-language'], 'en-US');
83118
assert.strictEqual(params.request.headers.cookie, 'k1=v1; k2=v2');
@@ -103,12 +138,20 @@ function verifyResponseReceived({ method, params }, expect) {
103138
assert.strictEqual(params.response.statusText, 'OK');
104139
assert.strictEqual(params.response.url, expect.url);
105140
assert.strictEqual(typeof params.response.headers, 'object');
106-
assert.strictEqual(params.response.headers.server, 'node');
107-
assert.strictEqual(params.response.headers.etag, '12345');
108-
assert.strictEqual(params.response.headers['set-cookie'], 'key1=value1\nkey2=value2');
109-
assert.strictEqual(params.response.headers['x-header2'], 'value1, value2');
110-
assert.strictEqual(params.response.mimeType, 'text/plain');
111-
assert.strictEqual(params.response.charset, 'utf-8');
141+
if (expect.responseHeaders?.server) {
142+
assert.strictEqual(params.response.headers.server, expect.responseHeaders.server);
143+
}
144+
if (expect.responseHeaders?.etag) {
145+
assert.strictEqual(params.response.headers.etag, expect.responseHeaders.etag);
146+
}
147+
if (expect.responseHeaders?.['set-cookie']) {
148+
assert.strictEqual(params.response.headers['set-cookie'], expect.responseHeaders['set-cookie']);
149+
}
150+
if (expect.responseHeaders?.['x-header2']) {
151+
assert.strictEqual(params.response.headers['x-header2'], expect.responseHeaders['x-header2']);
152+
}
153+
assert.strictEqual(params.response.mimeType, expect.mimeType);
154+
assert.strictEqual(params.response.charset, expect.charset);
112155

113156
return params;
114157
}
@@ -151,17 +194,46 @@ function verifyHttpResponse(response) {
151194
}));
152195
}
153196

154-
async function testHttpGet() {
155-
const url = `http://127.0.0.1:${httpServer.address().port}/hello-world`;
197+
function drainHttpResponse(response) {
198+
response.resume();
199+
}
200+
201+
function createRequestTracker(url, responseExpect, requestExpect = {}) {
156202
const requestWillBeSentFuture = once(session, 'Network.requestWillBeSent')
157-
.then(([event]) => verifyRequestWillBeSent(event, { url }));
203+
.then(([event]) => verifyRequestWillBeSent(event, {
204+
url,
205+
method: requestExpect.method,
206+
}));
158207

159208
const responseReceivedFuture = once(session, 'Network.responseReceived')
160-
.then(([event]) => verifyResponseReceived(event, { url }));
209+
.then(([event]) => verifyResponseReceived(event, responseExpect));
161210

162211
const loadingFinishedFuture = once(session, 'Network.loadingFinished')
163212
.then(([event]) => verifyLoadingFinished(event));
164213

214+
return {
215+
requestWillBeSentFuture,
216+
responseReceivedFuture,
217+
loadingFinishedFuture,
218+
};
219+
}
220+
221+
async function assertResponseBody(responseReceived, expectedBody, expectedBase64Encoded = false) {
222+
const responseBody = await session.post('Network.getResponseBody', {
223+
requestId: responseReceived.requestId,
224+
});
225+
assert.strictEqual(responseBody.base64Encoded, expectedBase64Encoded);
226+
assert.strictEqual(responseBody.body, expectedBody);
227+
}
228+
229+
async function testHttpGet() {
230+
const url = `http://127.0.0.1:${httpServer.address().port}/hello-world`;
231+
const {
232+
requestWillBeSentFuture,
233+
responseReceivedFuture,
234+
loadingFinishedFuture,
235+
} = createRequestTracker(url, getDefaultResponseExpect(url));
236+
165237
http.get({
166238
host: '127.0.0.1',
167239
port: httpServer.address().port,
@@ -175,24 +247,83 @@ async function testHttpGet() {
175247

176248
const delta = (loadingFinished.timestamp - responseReceived.timestamp) * 1000;
177249
assert.ok(delta > kDelta);
250+
await assertResponseBody(responseReceived, '\nhello world\n');
251+
}
178252

179-
const responseBody = await session.post('Network.getResponseBody', {
180-
requestId: responseReceived.requestId,
253+
async function testHttpGetWithAbsoluteUrlPath() {
254+
const url = `http://127.0.0.1:${httpServer.address().port}/hello-world`;
255+
const {
256+
requestWillBeSentFuture,
257+
responseReceivedFuture,
258+
loadingFinishedFuture,
259+
} = createRequestTracker(url, getDefaultResponseExpect(url));
260+
261+
http.get({
262+
host: '127.0.0.1',
263+
port: httpServer.address().port,
264+
path: url,
265+
headers: requestHeaders,
266+
}, common.mustCall(verifyHttpResponse));
267+
268+
await requestWillBeSentFuture;
269+
const responseReceived = await responseReceivedFuture;
270+
const loadingFinished = await loadingFinishedFuture;
271+
272+
const delta = (loadingFinished.timestamp - responseReceived.timestamp) * 1000;
273+
assert.ok(delta > kDelta);
274+
await assertResponseBody(responseReceived, '\nhello world\n');
275+
}
276+
277+
async function testHttpPostWithAbsoluteUrlPath() {
278+
const requestBody = JSON.stringify({ title: 'foo', type: 'post' });
279+
const url = `http://127.0.0.1:${httpServer.address().port}/echo-post`;
280+
const {
281+
requestWillBeSentFuture,
282+
responseReceivedFuture,
283+
loadingFinishedFuture,
284+
} = createRequestTracker(url, {
285+
url,
286+
mimeType: 'application/json',
287+
charset: 'utf-8',
288+
}, {
289+
method: 'POST',
181290
});
182-
assert.strictEqual(responseBody.base64Encoded, false);
183-
assert.strictEqual(responseBody.body, '\nhello world\n');
291+
292+
const responsePromise = new Promise((resolve, reject) => {
293+
const req = http.request({
294+
host: '127.0.0.1',
295+
port: httpServer.address().port,
296+
path: url,
297+
method: 'POST',
298+
headers: {
299+
...requestHeaders,
300+
'Content-Type': 'application/json',
301+
'Content-Length': Buffer.byteLength(requestBody),
302+
},
303+
}, resolve);
304+
req.on('error', reject);
305+
req.end(requestBody);
306+
});
307+
308+
const response = await responsePromise;
309+
drainHttpResponse(response);
310+
311+
await requestWillBeSentFuture;
312+
const responseReceived = await responseReceivedFuture;
313+
await loadingFinishedFuture;
314+
await assertResponseBody(responseReceived, JSON.stringify({
315+
method: 'POST',
316+
body: requestBody,
317+
}));
184318
}
185319

186320
async function testHttpsGet() {
187321
const url = `https://127.0.0.1:${httpsServer.address().port}/hello-world`;
188-
const requestWillBeSentFuture = once(session, 'Network.requestWillBeSent')
189-
.then(([event]) => verifyRequestWillBeSent(event, { url }));
190-
191-
const responseReceivedFuture = once(session, 'Network.responseReceived')
192-
.then(([event]) => verifyResponseReceived(event, { url }));
193-
194-
const loadingFinishedFuture = once(session, 'Network.loadingFinished')
195-
.then(([event]) => verifyLoadingFinished(event));
322+
const {
323+
requestWillBeSentFuture,
324+
responseReceivedFuture,
325+
loadingFinishedFuture,
326+
} = createRequestTracker(url, getDefaultResponseExpect(url));
196327

197328
https.get({
198329
host: '127.0.0.1',
@@ -208,12 +339,7 @@ async function testHttpsGet() {
208339

209340
const delta = (loadingFinished.timestamp - responseReceived.timestamp) * 1000;
210341
assert.ok(delta > kDelta);
211-
212-
const responseBody = await session.post('Network.getResponseBody', {
213-
requestId: responseReceived.requestId,
214-
});
215-
assert.strictEqual(responseBody.base64Encoded, false);
216-
assert.strictEqual(responseBody.body, '\nhello world\n');
342+
await assertResponseBody(responseReceived, '\nhello world\n');
217343
}
218344

219345
async function testHttpError() {
@@ -257,6 +383,10 @@ async function testHttpsError() {
257383
const testNetworkInspection = async () => {
258384
await testHttpGet();
259385
session.removeAllListeners();
386+
await testHttpGetWithAbsoluteUrlPath();
387+
session.removeAllListeners();
388+
await testHttpPostWithAbsoluteUrlPath();
389+
session.removeAllListeners();
260390
await testHttpsGet();
261391
session.removeAllListeners();
262392
await testHttpError();

0 commit comments

Comments
 (0)