http: match subdomains for plain NO_PROXY entries

doc/api/http.md

@@ -4559,6 +4559,11 @@ A browser-compatible implementation of {WebSocket}.
 added:
  - v24.5.0
  - v22.21.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65617
+    description: Plain `NO_PROXY` entries now match subdomains, IP entries
+                 are matched exactly, and empty entries are ignored.
 -->
 
 > Stability: 1.1 - Active development
@@ -4615,14 +4620,17 @@ Proxy URLs can use either HTTP or HTTPS protocols:
 The `NO_PROXY` environment variable supports several formats:
 
 * `*` - Bypass proxy for all hosts
-* `example.com` - Exact host name match
+* `example.com` - Host and subdomain match (matches `sub.example.com`)
 * `.example.com` - Domain suffix match (matches `sub.example.com`)
 * `*.example.com` - Wildcard domain match
 * `192.168.1.100` - Exact IP address match
 * `192.168.1.1-192.168.1.100` - IP address range
-* `example.com:8080` - Hostname with specific port
+* `example.com:8080` - Hostname with specific port (exact host match, no
+  subdomains)
 
-Multiple entries should be separated by commas.
+Multiple entries should be separated by commas; empty entries are ignored.
+A plain or leading-dot IP entry matches only that exact IP, and no domain
+entry can bypass a host that is an IP address literal.
 
 ### Example
 

lib/internal/http.js

@@ -19,7 +19,7 @@ const {
 
 const { URL } = require('internal/url');
 const { Buffer } = require('buffer');
-const { isIPv4 } = require('internal/net');
+const { isIP, isIPv4 } = require('internal/net');
 const { ERR_PROXY_INVALID_CONFIG } = require('internal/errors').codes;
 let utcCache;
 
@@ -132,7 +132,11 @@ class ProxyConfig {
       this.auth = `Basic ${Buffer.from(auth).toString('base64')}`;
     }
     if (noProxyList) {
-      this.bypassList = noProxyList.split(',').map((entry) => entry.trim().toLowerCase());
+      // Drop empty entries (e.g. from trailing commas) - an empty string
+      // would suffix-match any host ending with a dot.
+      this.bypassList = noProxyList.split(',')
+        .map((entry) => entry.trim().toLowerCase())
+        .filter((entry) => entry !== '');
     } else {
       this.bypassList = []; // No bypass list provided.
     }
@@ -156,21 +160,33 @@ class ProxyConfig {
 
     const host = hostname.toLowerCase();
     const hostWithPort = port ? `${host}:${port}` : host;
+    const hostIsIP = isIP(host) !== 0;
 
     for (let i = 0; i < bypassList.length; i++) {
       const entry = bypassList[i];
 
       if (entry === '*') return false;  // * bypasses all hosts.
       if (entry === host || entry === hostWithPort) return false;  // Matching host and host:port
 
-      // Follow curl's behavior: strip leading dot before matching suffixes.
-      if (entry[0] === '.') {
-        const suffix = entry.substring(1);
-        if (host === suffix || (host.endsWith(suffix) && host[host.length - suffix.length - 1] === '.')) return false;
+      // Strip a leading "." if present, then match as a suffix with a
+      // label boundary. Suffix matching only applies between domains: IP
+      // literals are matched exactly (or by range, below), as documented.
+      // "*.example.com" is handled below (subdomains only).
+      if (!entry.startsWith('*.')) {
+        const suffix = entry[0] === '.' ? entry.substring(1) : entry;
+        if (host === suffix ||
+            (suffix !== '' && !hostIsIP && !isIP(suffix) &&
+             host.endsWith(suffix) && host[host.length - suffix.length - 1] === '.')) {
+          return false;
+        }
       }
 
-      // Handle wildcards like *.example.com
-      if (entry.startsWith('*.') && host.endsWith(entry.substring(1))) return false;
+      // Handle wildcards like *.example.com. IP hosts never match domain
+      // rules, and a bare "*." must not match every host ending with a dot.
+      if (!hostIsIP && entry.length > 2 && entry.startsWith('*.') &&
+          host.endsWith(entry.substring(1))) {
+        return false;
+      }
 
       // Handle IP ranges (simple format like 192.168.1.0-192.168.1.255)
       // TODO(joyeecheung): support IPv6.

test/client-proxy/test-http-proxy-request-no-proxy-domain.mjs

@@ -10,7 +10,7 @@ import { runProxiedRequest } from '../common/proxy-server.js';
 const server = http.createServer(common.mustCall((req, res) => {
   res.writeHead(200, { 'Content-Type': 'text/plain' });
   res.end('Hello World\n');
-}, 5));
+}, 11));
 server.on('error', common.mustNotCall((err) => { console.error('Server error', err); }));
 server.listen(0, '127.0.0.1');
 await once(server, 'listening');
@@ -78,6 +78,121 @@ await once(proxy, 'listening');
   assert.strictEqual(signal, null);
 }
 
+{
+  // Test NO_PROXY with a plain domain also matching subdomains.
+  const { code, signal, stderr, stdout } = await runProxiedRequest({
+    NODE_USE_ENV_PROXY: 1,
+    REQUEST_URL: `http://test.example.com:${server.address().port}/test`,
+    HTTP_PROXY: `http://localhost:${proxy.address().port}`,
+    RESOLVE_TO_LOCALHOST: 'test.example.com',
+    NO_PROXY: 'example.com',
+  });
+
+  // The request should succeed and bypass proxy.
+  assert.match(stdout, /Status Code: 200/);
+  assert.match(stdout, /Hello World/);
+  assert.match(stdout, /Resolving lookup for test\.example\.com/);
+  assert.strictEqual(stderr.trim(), '');
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
+{
+  // Test NO_PROXY with a trailing comma still matching the real entries.
+  const { code, signal, stderr, stdout } = await runProxiedRequest({
+    NODE_USE_ENV_PROXY: 1,
+    REQUEST_URL: `http://test.example.com:${server.address().port}/test`,
+    HTTP_PROXY: `http://localhost:${proxy.address().port}`,
+    RESOLVE_TO_LOCALHOST: 'test.example.com',
+    NO_PROXY: 'example.com,',
+  });
+
+  // The request should succeed and bypass proxy.
+  assert.match(stdout, /Status Code: 200/);
+  assert.match(stdout, /Hello World/);
+  assert.match(stdout, /Resolving lookup for test\.example\.com/);
+  assert.strictEqual(stderr.trim(), '');
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
+{
+  // Test NO_PROXY with an empty entry (trailing comma) should NOT match
+  // hostnames ending with a dot.
+  const { code, signal, stderr, stdout } = await runProxiedRequest({
+    NODE_USE_ENV_PROXY: 1,
+    REQUEST_URL: `http://evil.com.:${server.address().port}/test`,
+    HTTP_PROXY: `http://localhost:${server.address().port}`,
+    RESOLVE_TO_LOCALHOST: 'evil.com.',
+    NO_PROXY: 'example.com,',
+  });
+
+  // The request should go through the proxy (not bypass it): the empty
+  // entry must not match, and evil.com. does not match example.com.
+  assert.match(stdout, /Status Code: 200/);
+  assert.doesNotMatch(stdout, /Resolving lookup for evil\.com/);
+  assert.strictEqual(stderr.trim(), '');
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
+{
+  // Test NO_PROXY with a lone "." entry should NOT match hostnames ending
+  // with a dot.
+  const { code, signal, stderr, stdout } = await runProxiedRequest({
+    NODE_USE_ENV_PROXY: 1,
+    REQUEST_URL: `http://evil.com.:${server.address().port}/test`,
+    HTTP_PROXY: `http://localhost:${server.address().port}`,
+    RESOLVE_TO_LOCALHOST: 'evil.com.',
+    NO_PROXY: '.',
+  });
+
+  // The request should go through the proxy (not bypass it).
+  assert.match(stdout, /Status Code: 200/);
+  assert.doesNotMatch(stdout, /Resolving lookup for evil\.com/);
+  assert.strictEqual(stderr.trim(), '');
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
+{
+  // Test NO_PROXY with a bare "*." entry should NOT match hostnames ending
+  // with a dot.
+  const { code, signal, stderr, stdout } = await runProxiedRequest({
+    NODE_USE_ENV_PROXY: 1,
+    REQUEST_URL: `http://evil.com.:${server.address().port}/test`,
+    HTTP_PROXY: `http://localhost:${server.address().port}`,
+    RESOLVE_TO_LOCALHOST: 'evil.com.',
+    NO_PROXY: '*.',
+  });
+
+  // The request should go through the proxy (not bypass it).
+  assert.match(stdout, /Status Code: 200/);
+  assert.doesNotMatch(stdout, /Resolving lookup for evil\.com/);
+  assert.strictEqual(stderr.trim(), '');
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
+{
+  // Test NO_PROXY with a plain domain should NOT match partial domain names.
+  const { code, signal, stderr, stdout } = await runProxiedRequest({
+    NODE_USE_ENV_PROXY: 1,
+    REQUEST_URL: `http://badexample.com:${server.address().port}/test`,
+    HTTP_PROXY: `http://localhost:${server.address().port}`,
+    RESOLVE_TO_LOCALHOST: 'badexample.com',
+    NO_PROXY: 'example.com',
+  });
+
+  // The request should go through the proxy (not bypass it),
+  // because badexample.com is not a subdomain of example.com.
+  assert.match(stdout, /Status Code: 200/);
+  assert.doesNotMatch(stdout, /Resolving lookup for badexample\.com/);
+  assert.strictEqual(stderr.trim(), '');
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
 // Test NO_PROXY with leading-dot entry should NOT match partial domain names.
 // Regression test: .example.com must not match notexample.com or badexample.com.
 {
@@ -92,6 +207,7 @@ await once(proxy, 'listening');
   // The request should go through the proxy (not bypass it),
   // because notexample.com is not a subdomain of example.com.
   assert.match(stdout, /Status Code: 200/);
+  assert.doesNotMatch(stdout, /Resolving lookup for notexample\.com/);
   assert.strictEqual(stderr.trim(), '');
   assert.strictEqual(code, 0);
   assert.strictEqual(signal, null);
@@ -109,6 +225,7 @@ await once(proxy, 'listening');
   // The request should go through the proxy (not bypass it),
   // because badexample.com is not a subdomain of example.com.
   assert.match(stdout, /Status Code: 200/);
+  assert.doesNotMatch(stdout, /Resolving lookup for badexample\.com/);
   assert.strictEqual(stderr.trim(), '');
   assert.strictEqual(code, 0);
   assert.strictEqual(signal, null);

test/client-proxy/test-http-proxy-request-no-proxy-ip-suffix.mjs

@@ -0,0 +1,65 @@
+// This tests that NO_PROXY IP entries and IP hosts are matched exactly,
+// never as domain suffixes.
+
+import * as common from '../common/index.mjs';
+import assert from 'node:assert';
+import { once } from 'events';
+import http from 'node:http';
+import { runProxiedRequest } from '../common/proxy-server.js';
+
+// Start a server that should NOT be reached directly.
+const server = http.createServer(common.mustNotCall());
+server.listen(0, '127.0.0.1');
+await once(server, 'listening');
+
+// Start a proxy server that should be used for all requests below.
+const proxy = http.createServer(common.mustCall((req, res) => {
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+  res.end('proxied');
+}, 3));
+proxy.listen(0);
+await once(proxy, 'listening');
+
+// An IP host must not be bypassed by entries that only match it as a
+// string suffix: plain, leading-dot, or wildcard.
+for (const noProxy of ['0.1', '.0.1', '*.0.1']) {
+  const { code, signal, stderr, stdout } = await runProxiedRequest({
+    NODE_USE_ENV_PROXY: 1,
+    REQUEST_URL: `http://127.0.0.1:${server.address().port}/test`,
+    HTTP_PROXY: `http://localhost:${proxy.address().port}`,
+    NO_PROXY: noProxy,
+  });
+
+  // The request should go through the proxy (not bypass it).
+  assert.match(stdout, /Status Code: 200/);
+  assert.match(stdout, /proxied/);
+  assert.strictEqual(stderr.trim(), '');
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
+{
+  // An IP entry must not bypass a domain host that merely ends with it.
+  // foo.127.0.0.1 is not bypassed by the entry 127.0.0.1, so the request
+  // takes the proxy path, which rejects the numeric-TLD hostname when it
+  // builds the request URL. A bypass regression would instead attempt a
+  // direct connection and call the lookup.
+  const agent = new http.Agent({
+    proxyEnv: {
+      HTTP_PROXY: `http://127.0.0.1:${proxy.address().port}`,
+      NO_PROXY: '127.0.0.1',
+    },
+  });
+  assert.throws(() => {
+    http.request({
+      agent,
+      hostname: 'foo.127.0.0.1',
+      lookup: common.mustNotCall(),
+      path: '/',
+    });
+  }, { code: 'ERR_INVALID_URL' });
+  agent.destroy();
+}
+
+proxy.close();
+server.close();