Skip to content

http: fix no_proxy leading-dot suffix matching#62333

Open
watilde wants to merge 1 commit intonodejs:mainfrom
watilde:fix/http-no-proxy
Open

http: fix no_proxy leading-dot suffix matching#62333
watilde wants to merge 1 commit intonodejs:mainfrom
watilde:fix/http-no-proxy

Conversation

@watilde
Copy link
Member

@watilde watilde commented Mar 19, 2026

Fix NO_PROXY suffix matching to prevent partial domain matches

This PR improves NO_PROXY logic by ensuring suffix matches occur only on exact domains or subdomains. Previously, using endsWith() caused false positives (e.g., myexample.com matching .example.com). This aligns with the expected domain boundary matching behavior.

// Run with: ./node --expose-internals -e "$(cat this_file.js)"
'use strict';

const { parseProxyConfigFromEnv } = require('internal/http');
const assert = require('assert');

const config = parseProxyConfigFromEnv({
  http_proxy: 'http://proxy.test:8080',
  no_proxy: '.example.com',
}, 'http:', false);

// These should bypass proxy (false)
assert.strictEqual(config.shouldUseProxy('sub.example.com'), false);
assert.strictEqual(config.shouldUseProxy('example.com'), false);

// These should use proxy (true) — currently returns false without the fix
assert.strictEqual(config.shouldUseProxy('notexample.com'), true);
assert.strictEqual(config.shouldUseProxy('badexample.com'), true);

console.log('All assertions passed');

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Mar 19, 2026
@codecov
Copy link

codecov bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.68%. Comparing base (56aba88) to head (abbc102).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62333      +/-   ##
==========================================
- Coverage   91.64%   89.68%   -1.96%     
==========================================
  Files         337      676     +339     
  Lines      140643   206579   +65936     
  Branches    21810    39563   +17753     
==========================================
+ Hits       128893   185272   +56379     
- Misses      11526    13439    +1913     
- Partials      224     7868    +7644     
Files with missing lines Coverage Δ
lib/internal/http.js 92.11% <100.00%> (ø)

... and 460 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants