fixup! http: match subdomains for plain NO_PROXY entries

Keep the change scoped to plain domain entries. Preserve the existing
leading-dot and wildcard behavior, while preventing the new suffix
match from applying to empty entries or IP literals.

Signed-off-by: Nikita Snetkov <lukyanish@gmail.com>

Author: dibenkobit

doc/api/http.md

@@ -4562,8 +4562,7 @@ added:
 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.
+    description: Plain domain entries in `NO_PROXY` now match subdomains.
 -->
 
 > Stability: 1.1 - Active development
@@ -4625,12 +4624,9 @@ The `NO_PROXY` environment variable supports several formats:
 * `*.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 (exact host match, no
-  subdomains)
+* `example.com:8080` - Hostname with specific port
 
-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.
+Multiple entries should be separated by commas.
 
 ### Example
 

lib/internal/http.js

@@ -132,11 +132,7 @@ class ProxyConfig {
       this.auth = `Basic ${Buffer.from(auth).toString('base64')}`;
     }
     if (noProxyList) {
-      // 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 !== '');
+      this.bypassList = noProxyList.split(',').map((entry) => entry.trim().toLowerCase());
     } else {
       this.bypassList = []; // No bypass list provided.
     }
@@ -168,26 +164,22 @@ class ProxyConfig {
       if (entry === '*') return false;  // * bypasses all hosts.
       if (entry === host || entry === hostWithPort) return false;  // Matching host and host:port
 
-      // 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;
-        }
+      // Plain domain entries also match subdomains at a label boundary.
+      // Keep IP hosts and entries out of this new suffix match.
+      if (entry !== '' && entry[0] !== '.' && !entry.startsWith('*.') &&
+          !hostIsIP && isIP(entry) === 0 && host.endsWith(`.${entry}`)) {
+        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;
+      // 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;
       }
 
+      // Handle wildcards like *.example.com
+      if (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.
       if (entry.includes('-') && isIPv4(host)) {

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');
-}, 11));
+}, 8));
 server.on('error', common.mustNotCall((err) => { console.error('Server error', err); }));
 server.listen(0, '127.0.0.1');
 await once(server, 'listening');
@@ -98,72 +98,32 @@ await once(proxy, 'listening');
 }
 
 {
-  // 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.
+  // 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://evil.com.:${server.address().port}/test`,
+    REQUEST_URL: `http://badexample.com:${server.address().port}/test`,
     HTTP_PROXY: `http://localhost:${server.address().port}`,
-    RESOLVE_TO_LOCALHOST: 'evil.com.',
-    NO_PROXY: '.',
+    RESOLVE_TO_LOCALHOST: 'badexample.com',
+    NO_PROXY: 'example.com',
   });
 
-  // The request should go through the proxy (not bypass it).
+  // 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 evil\.com/);
+  assert.doesNotMatch(stdout, /Resolving lookup for badexample\.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.
+  // Test that an empty plain entry does not match a hostname ending in 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: '*.',
+    NO_PROXY: 'example.com,',
   });
 
   // The request should go through the proxy (not bypass it).
@@ -174,25 +134,6 @@ await once(proxy, 'listening');
   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.
 {

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

@@ -1,5 +1,4 @@
-// This tests that NO_PROXY IP entries and IP hosts are matched exactly,
-// never as domain suffixes.
+// This tests that plain NO_PROXY entries do not suffix-match IP addresses.
 
 import * as common from '../common/index.mjs';
 import assert from 'node:assert';
@@ -16,18 +15,17 @@ await once(server, 'listening');
 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']) {
+{
+  // A plain entry must not bypass an IP host by matching a string suffix.
   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,
+    NO_PROXY: '0.1',
   });
 
   // The request should go through the proxy (not bypass it).