Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -4113,6 +4113,9 @@ changes:
E.G. `'/index.html?page=12'`. An exception is thrown when the request path
contains illegal characters. Currently, only spaces are rejected but that
may change in the future. **Default:** `'/'`.
With `method` set to `'CONNECT'`, `path` must be an authority in the form
`host:port`, such as `'www.example.com:80'` or `'[2001:db8::1]:443'`.
Invalid values throw an `ERR_INVALID_ARG_VALUE` error.
The content in `path` is sent as the [request target][] in the HTTP 1.1 message.
When `path` is an absolute URL, this means the request target in the message in [absolute form][].
If the receiving server is a proxy, the server typically forwards the request to the
Expand Down
37 changes: 35 additions & 2 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const {
const Agent = require('_http_agent');
const { Buffer } = require('buffer');
const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const { URL, urlToHttpOptions, isURL } = require('internal/url');
const { URL, URLParse, urlToHttpOptions, isURL } = require('internal/url');
const {
kOutHeaders,
kNeedDrain,
Expand Down Expand Up @@ -121,6 +121,7 @@ let debug = require('internal/util/debuglog').debuglog('http', (fn) => {
});

const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
const CONNECT_PATH_REGEX = /^(\[[^\]]+\]|[^:]+):(\d+)$/;
const kError = Symbol('kError');
const kPath = Symbol('kPath');
const kAuthority = Symbol('kAuthority');
Expand Down Expand Up @@ -193,6 +194,22 @@ function authoritiesMatch(canonicalHost, hostFromHeader) {
return parsed.host === canonicalHost;
}

function isValidConnectPath(path) {
const match = CONNECT_PATH_REGEX.exec(path);
if (match === null || +match[2] === 0) {
return false;
}

const url = URLParse(`http://${path}`);
return url !== null &&
url.hostname !== '' &&
url.username === '' &&
url.password === '' &&
url.pathname === '/' &&
url.search === '' &&
url.hash === '';
}

// https://datatracker.ietf.org/doc/html/rfc9112#section-3.2
// When the request target is in absolute-form, ensure it is consistent with
// the request authority: same scheme, no userinfo, and an authority
Expand Down Expand Up @@ -466,7 +483,23 @@ function ClientRequest(input, options, cb) {

this.joinDuplicateHeaders = options.joinDuplicateHeaders;

this[kPath] = options.path || '/';
let path = options.path || '/';
if (method === 'CONNECT' && options.path != null) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: path defaults to /, but then this checks options.path.

That means if you set no path, we will send /, but if you set path: '/' then we throw. We should make those consistent.

path = String(options.path);
if (path[0] === '/') {
path = path.slice(1) || '/';
}

if (!isValidConnectPath(path)) {
throw new ERR_INVALID_ARG_VALUE(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't used to throw, in fact it would have sent the path successfully. Servers could potentially accept it and use that (they shouldn't, but it's quite possible that weird ones do anyway) or people might have test suites that send invalid values to confirm they're rejected by their server implementation. With this change that working code would validate and throw instead.

'options.path',
path,
'must be a valid host:port combo',
);
}
}

this[kPath] = path;
if (cb) {
this.once('response', cb);
}
Expand Down
75 changes: 75 additions & 0 deletions test/parallel/test-http-request-connect-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

for (const path of [
'',
'example.com',
'example.com:0',
'example.com:65536',
'example.com:8080/example',
'evil.com:666/good.org:777',
'/example.com',
]) {
assert.throws(() => http.request({
method: 'CONNECT',
path,
}), {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: /^The property 'options\.path' must be a valid host:port combo\./,
});
}

{
const server = http.createServer(common.mustNotCall());

server.on('connect', common.mustCall((req, socket) => {
assert.strictEqual(req.url, 'example.com:80');
socket.end('HTTP/1.1 501 Not Implemented\r\n\r\n');
}));

server.listen(0, common.mustCall(() => {
const port = server.address().port;
const req = http.request(
new URL(`http://localhost:${port}/example.com:80`),
{ method: 'CONNECT' },
);

req.on('connect', common.mustCall((res, socket) => {
assert.strictEqual(res.statusCode, 501);
socket.destroy();
server.close();
}));

req.end();
}));
}

{
const server = http.createServer(common.mustNotCall());

server.on('connect', common.mustCall((req, socket) => {
assert.strictEqual(req.url, '[2001:db8::1]:111');
socket.end('HTTP/1.1 501 Not Implemented\r\n\r\n');
}));

server.listen(0, common.mustCall(() => {
const req = http.request({
host: 'localhost',
port: server.address().port,
method: 'CONNECT',
path: '[2001:db8::1]:111',
});

req.on('connect', common.mustCall((res, socket) => {
assert.strictEqual(res.statusCode, 501);
socket.destroy();
server.close();
}));

req.end();
}));
}
Loading