http: document and validate options.path when it's in absolute-form

doc/api/http.md

@@ -4096,6 +4096,18 @@ 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:** `'/'`.
+    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
+    destination specified in the request target, and ignores the `Host` header.
+    The user needs to make sure that `path`, `host` and the Host headers conform to the
+    requirement of the [request target][] in the HTTP specification.
+    When the receiving server is known to be a proxy because the request is routed through
+    [Built-in Proxy Support][], `http.request` will additionally perform a best-effort
+    check to see that the `host` option or `Host` in `headers` agrees with the authority
+    in `path` during the initial construction of the request. It gives up rewriting the
+    request target for proxying and throws an error if they don't match at request
+    construction time, though there won't be checks for later header mutations done by the user.
   * `port` {number} Port of remote server. **Default:** `defaultPort` if set,
     else `80`.
   * `protocol` {string} Protocol to use. **Default:** `'http:'`.
@@ -4792,5 +4804,7 @@ const agent2 = new http.Agent({ proxyEnv: process.env });
 [`writable.destroyed`]: stream.md#writabledestroyed
 [`writable.uncork()`]: stream.md#writableuncork
 [`writable.write()`]: stream.md#writablewritechunk-encoding-callback
+[absolute form]: https://datatracker.ietf.org/doc/html/rfc9112#section-3.2.2
 [information event]: #event-information
 [initial delay]: net.md#socketsetkeepaliveenable-initialdelay
+[request target]: https://datatracker.ietf.org/doc/html/rfc9112#section-3.2

lib/_http_client.js

@@ -84,6 +84,7 @@ const {
   validateInteger,
   validateBoolean,
   validateOneOf,
+  validatePort,
   validateString,
 } = require('internal/validators');
 const { getTimerDuration } = require('internal/timers');
@@ -139,15 +140,134 @@ class HTTPClientAsyncResource {
   }
 }
 
+// The only documented shape is [k, v, k, v, ...]. Here we also accept [[k, v], [k, v], ...].
+// for backward compatibility, and reject others. Also reject if there are duplicate Host entries.
+// Returns the Host header value, or undefined if absent.
+function getHostFromHeaderArray(headers) {
+  let host;
+  const isPairs = headers.length > 0 && ArrayIsArray(headers[0]);
+  if (isPairs) {
+    for (let i = 0; i < headers.length; i++) {
+      const entry = headers[i];
+      if (!ArrayIsArray(entry)) {
+        throw new ERR_INVALID_ARG_VALUE(`options.headers[${i}]`, typeof entry,
+                                        'must be an array when headers is passed as an array of pairs');
+      }
+      if (`${entry[0]}`.toLowerCase() === 'host') {
+        if (host !== undefined) {
+          throw new ERR_INVALID_ARG_VALUE('options.headers', '(redacted)',
+                                          'must not contain duplicate Host headers');
+        }
+        host = `${entry[1]}`;
+      }
+    }
+  } else {
+    for (let i = 0; i + 1 < headers.length; i += 2) {
+      if (`${headers[i]}`.toLowerCase() === 'host') {
+        if (host !== undefined) {
+          throw new ERR_INVALID_ARG_VALUE('options.headers', '(redacted)',
+                                          'must not contain duplicate Host headers');
+        }
+        host = `${headers[i + 1]}`;
+      }
+    }
+  }
+  return host;
+}
+
+function authoritiesMatch(canonicalHost, hostFromHeader) {
+  let parsed;
+  try {
+    parsed = new URL(`http://${hostFromHeader}`);
+  } catch {
+    return false;
+  }
+  if (parsed.username || parsed.password ||
+      parsed.pathname !== '/' || parsed.search || parsed.hash) {
+    return false;
+  }
+  return parsed.host === canonicalHost;
+}
+
+// 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
+// component agree with options.host[:port].
+function validateRequestAuthority(pathOption, proxyAuthority, userHostHeader, headerArray) {
+  validatePort(proxyAuthority.port, 'options.port', true);
+  pathOption = `${pathOption}`;
+  const requestBase = new URL(`http://${proxyAuthority.host}`);
+  requestBase.port = proxyAuthority.port;
+
+  const result = { requestBase };
+  if (headerArray !== undefined) {
+    const host = getHostFromHeaderArray(headerArray);
+    // Since we don't mutate the header array to normalize the Host value, unlike
+    // in the case of other shapes of headers provided, we check that it is identical
+    // to the authority from the requestBase.
+    if (host !== undefined && host !== requestBase.host) {
+      throw new ERR_INVALID_ARG_VALUE(
+        'Host in options.headers', host,
+        `must match the request authority (${requestBase.host})`);
+    }
+  } else if (userHostHeader !== undefined) {
+    if (!authoritiesMatch(requestBase.host, userHostHeader)) {
+      throw new ERR_INVALID_ARG_VALUE(
+        'Host in options.headers', userHostHeader,
+        `must match the request authority (${requestBase.host})`);
+    }
+  }
+
+  // Per RFC 9112 Section 3.2, if request target is in absolute-form its authority
+  // must agree with the request authority.
+  let requestURL;
+  let isAbsoluteForm = false;
+  try {
+    requestURL = new URL(pathOption);
+    isAbsoluteForm = true;
+  } catch {
+    if (pathOption.charCodeAt(0) !== 0x2F) {
+      throw new ERR_INVALID_ARG_VALUE(
+        'options.path', pathOption, 'must be in absolute-form or start with /');
+    }
+    requestURL = new URL(requestBase.origin + pathOption);
+  }
+  result.requestURL = requestURL;
+  if (!isAbsoluteForm) {
+    return result;
+  }
+
+  if (requestURL.username || requestURL.password) {
+    requestURL.username = '';
+    requestURL.password = '';
+    throw new ERR_INVALID_ARG_VALUE(
+      'options.path', requestURL.href, 'must not contain userinfo, use options.auth instead');
+  }
+
+  if (requestURL.protocol !== 'http:') {
+    throw new ERR_INVALID_ARG_VALUE(
+      'options.path', requestURL.protocol, 'must use http: scheme when specified as an absolute URL');
+  }
+
+  if (requestBase.host !== requestURL.host) {
+    throw new ERR_INVALID_ARG_VALUE(
+      'options.path', requestURL, `must match the request authority (${requestBase.host})`);
+  }
+
+  return result;
+}
+
 // When proxying a HTTP request, the following needs to be done:
-// https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2
+// https://datatracker.ietf.org/doc/html/rfc9112#section-3.2.2
 // 1. Rewrite the request path to absolute-form.
 // 2. Add proxy-connection and proxy-authorization headers appropriately.
 //
 // This function checks whether the request should be rewritten for proxying
 // and modifies the headers as well as req.path if necessary.
 // The handling of the proxy server connection is done in createConnection.
-function rewriteForProxiedHttp(req, reqOptions) {
+// It also validates that the Host header and absolute-form path authority match the
+// request authority specified by reqOptions.
+function rewriteForProxiedHttp(req, reqOptions, proxyAuthority, userHostHeader, headerArray) {
   if (req._header) {
     debug('request._header is already sent, skipping rewriteForProxiedHttp', reqOptions);
     return false;
@@ -165,6 +285,25 @@ function rewriteForProxiedHttp(req, reqOptions) {
   if (!shouldUseProxy) {
     return false;
   }
+
+  // Per RFC 9112 Section 3.2.2, we don't need to rewrite CONNECT or OPTIONS * requests.
+  let requestURL;
+  if (req.method !== 'CONNECT' && !(req.method === 'OPTIONS' && req.path === '*')) {
+    // Validate Host header values agree with the request authority before mutating req,
+    // so a rejected request doesn't leave proxy-* headers stuck on the outgoing header store.
+    // XXX(joyeecheung): This validates whether the request conforms to the RFC, but here
+    // we only do it for proxied requests for backward compatibility. For non-proxied requests,
+    // ensuring thst the request is well formed has been entirely left to the user.
+    const result = validateRequestAuthority(req.path, proxyAuthority, userHostHeader, headerArray);
+    if (headerArray === undefined) {
+      const currentHost = req.getHeader('host');
+      if (currentHost !== undefined && currentHost !== result.requestBase.host) {
+        req.setHeader('Host', result.requestBase.host);
+      }
+    }
+    requestURL = result.requestURL;
+  }
+
   // Add proxy headers.
   const { auth, href } = agent[kProxyConfig];
   if (auth) {
@@ -176,15 +315,10 @@ function rewriteForProxiedHttp(req, reqOptions) {
     req.setHeader('proxy-connection', 'close');
   }
 
-  // Convert the path to absolute-form.
-  // https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2
-  const requestHost = req.getHeader('host') || 'localhost';
-  const requestBase = `http://${requestHost}`;
-  const requestURL = new URL(req.path, requestBase);
-  if (reqOptions.port) {
-    requestURL.port = reqOptions.port;
+  if (requestURL !== undefined) {
+    // Convert the path to absolute-form. The authority is built from options.
+    req.path = requestURL.href;
   }
-  req.path = requestURL.href;
   debug(`updated request for HTTP proxy ${href} with ${req.path} `, req[kOutHeaders]);
   return true;
 };
@@ -360,6 +494,21 @@ function ClientRequest(input, options, cb) {
     }
   }
 
+  let hostHeaderFromOptions = host;
+  // For the Host header, ensure that IPv6 addresses are enclosed
+  // in square brackets, as defined by URI formatting
+  // https://tools.ietf.org/html/rfc3986#section-3.2.2
+  const posColon = hostHeaderFromOptions.indexOf(':');
+  if (posColon !== -1 &&
+      hostHeaderFromOptions.includes(':', posColon + 1) &&
+      hostHeaderFromOptions.charCodeAt(0) !== 91/* '[' */) {
+    hostHeaderFromOptions = `[${hostHeaderFromOptions}]`;
+  }
+  const proxyAuthority = { host: hostHeaderFromOptions, port };
+
+  if (port && +port !== defaultPort) {
+    hostHeaderFromOptions += ':' + port;
+  }
   const headersArray = ArrayIsArray(options.headers);
   if (!headersArray) {
     if (options.headers) {
@@ -372,23 +521,12 @@ function ClientRequest(input, options, cb) {
       }
     }
 
-    if (host && !this.getHeader('host') && setHost) {
-      let hostHeader = host;
-
-      // For the Host header, ensure that IPv6 addresses are enclosed
-      // in square brackets, as defined by URI formatting
-      // https://tools.ietf.org/html/rfc3986#section-3.2.2
-      const posColon = hostHeader.indexOf(':');
-      if (posColon !== -1 &&
-          hostHeader.includes(':', posColon + 1) &&
-          hostHeader.charCodeAt(0) !== 91/* '[' */) {
-        hostHeader = `[${hostHeader}]`;
-      }
+    // Save the Host header before the implicit auto-set below, so the
+    // proxy validator can tell user-explicit values from Node-generated ones.
+    const userHostHeader = this.getHeader('host');
 
-      if (port && +port !== defaultPort) {
-        hostHeader += ':' + port;
-      }
-      this.setHeader('Host', hostHeader);
+    if (host && !this.getHeader('host') && setHost) {
+      this.setHeader('Host', hostHeaderFromOptions);
     }
 
     if (options.auth && !this.getHeader('Authorization')) {
@@ -401,14 +539,14 @@ function ClientRequest(input, options, cb) {
         throw new ERR_HTTP_HEADERS_SENT('render');
       }
 
-      rewriteForProxiedHttp(this, optsWithoutSignal);
+      rewriteForProxiedHttp(this, optsWithoutSignal, proxyAuthority, userHostHeader);
       this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
                         this[kOutHeaders]);
     } else {
-      rewriteForProxiedHttp(this, optsWithoutSignal);
+      rewriteForProxiedHttp(this, optsWithoutSignal, proxyAuthority, userHostHeader);
     }
   } else {
-    rewriteForProxiedHttp(this, optsWithoutSignal);
+    rewriteForProxiedHttp(this, optsWithoutSignal, proxyAuthority, undefined, options.headers);
     this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
                       options.headers);
   }

test/client-proxy/test-http-proxy-request-absolute-path-authority-match.mjs

@@ -0,0 +1,70 @@
+// This tests that proxied HTTP requests succeed end-to-end when the
+// absolute-form request-target matches the request authority derived from options.
+
+import * as common from '../common/index.mjs';
+import assert from 'node:assert';
+import http from 'node:http';
+import { once } from 'events';
+import { createProxyServer } from '../common/proxy-server.js';
+
+const server = http.createServer(common.mustCall((req, res) => {
+  res.end('Hello world');
+}, 6));
+server.on('error', common.mustNotCall());
+server.listen(0);
+await once(server, 'listening');
+
+const { proxy, logs } = createProxyServer();
+proxy.listen(0);
+await once(proxy, 'listening');
+
+const port = server.address().port;
+const serverHost = `localhost:${port}`;
+const requestUrl = `http://${serverHost}/test`;
+
+const agent = new http.Agent({
+  proxyEnv: {
+    HTTP_PROXY: `http://localhost:${proxy.address().port}`,
+  },
+});
+
+async function roundTrip(options) {
+  const req = http.request({ agent, ...options });
+  req.end();
+  const [res] = await once(req, 'response');
+  res.setEncoding('utf8');
+  let body = '';
+  for await (const chunk of res) body += chunk;
+  assert.strictEqual(body, 'Hello world');
+}
+
+const baseAbsolute = { host: 'localhost', port, path: requestUrl };
+
+const options = [
+  // No user-supplied headers.
+  baseAbsolute,
+  // Object form with an explicit Host that matches.
+  { ...baseAbsolute, headers: { Host: serverHost } },
+  // Flat array form.
+  { ...baseAbsolute, headers: ['Host', serverHost] },
+  // Array-of-pairs form.
+  { ...baseAbsolute, headers: [['Host', serverHost]] },
+  // Contains defaultPort that matches options.port.
+  { host: 'localhost', port, defaultPort: port, path: '/test' },
+  // Stringifiable non-string path object.
+  { host: 'localhost', port, path: { toString() { return '/test'; } } },
+];
+
+for (const opts of options) {
+  await roundTrip(opts);
+  // Check what the proxy server received.
+  const log = logs.pop();
+  assert.strictEqual(logs.length, 0);
+  assert.strictEqual(log.method, 'GET');
+  assert.strictEqual(log.url, requestUrl);
+  assert.strictEqual(log.headers.host, serverHost);
+}
+
+server.close();
+proxy.close();
+agent.destroy();