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

When `options.path` passed to `http.request()` contains an absolute
URL, `http.request` has been sending it directly as the request target
in the HTTP 1.1 message. If the receiving server is a proxy,
the proxy server typically forwards the request to the destination
specified in the request target and ignores the Host header.
This means eventually the request can be forwarded to a destination
that is not consistent with `options.host`, depending on how the
receiving server behaves. Mimatched Host header and request target
also violates RFC 9112 Section 3.2, which we have been entirely
leaving to the users to verify.

This patch documents this behavior and warns that the user needs
to ensure the `path`, `option` and `headers` conform to the RFC.
If the receiving server is known to be a proxy server because
the request is routed by Node.js' built-in HTTP proxy support,
we now do a best-effort check to verify that the authority in
`options.path` (if absolute), Host headers and `options.host`
agree at request construction time. Node.js will give up on the
require target rewriting and throw an error when they don't match
at request construction.

Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #64108
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>

Author: joyeecheung

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-interval-count
+[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();