Skip to content

http: add IPv6 range support for NO_PROXY environment variable - #59950

Open
haramj wants to merge 2 commits into
nodejs:mainfrom
haramj:haramjeong-patch-250920
Open

http: add IPv6 range support for NO_PROXY environment variable#59950
haramj wants to merge 2 commits into
nodejs:mainfrom
haramj:haramjeong-patch-250920

Conversation

@haramj

@haramj haramj commented Sep 20, 2025

Copy link
Copy Markdown
Member

This PR resolves the // TODO(joyeecheung): support IPv6. comment in internal/http.js by adding comprehensive support for IPv6 address ranges to the NO_PROXY environment variable.

Previously, the ProxyConfig class's shouldUseProxy method could only handle IPv4 address ranges. With this change, users can now specify IPv6 ranges (e.g., ::1-::100) to correctly bypass proxies for hosts within those ranges.

The implementation involves:

  • A new helper function ipv6ToBigInt to convert 128-bit IPv6 addresses into BigInts for reliable numerical comparison.
  • Updated shouldUseProxy logic to check if a host is within a given IPv6 range.

This update ensures the proxy bypass feature is robust and consistent with both IPv4 and IPv6 network standards.

New test cases have been added to test/client-proxy/test-http-proxy-request-no-proxy-ipv6.mjs to verify the new functionality.

Test

  • A host within the specified IPv6 range (::1-::100) correctly bypasses the proxy.
  • A host outside the specified IPv6 range correctly uses the proxy.

This ensures that the new feature works as expected and does not introduce regressions to existing functionality.

/cc @joyeecheung

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@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 Sep 20, 2025
@haramj haramj changed the title http: Add IPv6 range support for NO_PROXY environment variable http: add IPv6 range support for NO_PROXY environment variable Sep 20, 2025
@haramj
haramj force-pushed the haramjeong-patch-250920 branch from 82b9bef to 397aac8 Compare September 20, 2025 19:00
@codecov

codecov Bot commented Sep 20, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.74359% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.15%. Comparing base (b2a024b) to head (e6bf6c6).

Files with missing lines Patch % Lines
lib/internal/http.js 89.74% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59950      +/-   ##
==========================================
- Coverage   90.15%   90.15%   -0.01%     
==========================================
  Files         744      744              
  Lines      242542   242577      +35     
  Branches    45690    45700      +10     
==========================================
+ Hits       218672   218697      +25     
- Misses      15362    15370       +8     
- Partials     8508     8510       +2     
Files with missing lines Coverage Δ
lib/internal/http.js 91.92% <89.74%> (-0.41%) ⬇️

... and 25 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.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

@github-actions github-actions Bot added the stale label Jul 28, 2026
@trivikr trivikr removed the stale label Jul 28, 2026
@trivikr
trivikr requested a review from joyeecheung July 28, 2026 03:04
@trivikr

trivikr commented Jul 28, 2026

Copy link
Copy Markdown
Member

@haramj Can you please rebase, sign the first commit and force push?

It's required as per updated instructions

Your commit must contain the Signed-off-by line with your name and email address as an acknowledgement that you agree to the Developer Certificate of Origin

haramj added 2 commits July 28, 2026 18:35
Signed-off-by: haramjeong <04harams77@gmail.com>
Signed-off-by: haramjeong <04harams77@gmail.com>
@haramj
haramj force-pushed the haramjeong-patch-250920 branch from e8735e8 to e6bf6c6 Compare July 28, 2026 09:36
@haramj

haramj commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

@haramj Can you please rebase, sign the first commit and force push?

It's required as per updated instructions

Your commit must contain the Signed-off-by line with your name and email address as an acknowledgement that you agree to the Developer Certificate of Origin

Thanks! I’ve rebased the branch onto the latest main and added the required Signed-off-by line to both commits.

@haramj

haramj commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

@trivikr It looks like the failing test is unrelated to this change (parallel/test-repl-use-global failed with ERR_INSPECTOR_COMMAND in the BoringSSL shared-library job). Could someone please rerun the failed job?

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.

3 participants