Skip to content

fix: assume http/https scheme for scheme-less proxy env vars#4914

Open
travisbreaks wants to merge 2 commits intonodejs:mainfrom
travisbreaks:fix/schema-less-proxy-env
Open

fix: assume http/https scheme for scheme-less proxy env vars#4914
travisbreaks wants to merge 2 commits intonodejs:mainfrom
travisbreaks:fix/schema-less-proxy-env

Conversation

@travisbreaks
Copy link
Contributor

Summary

Fixes #4736.

When HTTP_PROXY or HTTPS_PROXY environment variables (or the httpProxy/httpsProxy constructor options) are set without a URL scheme (e.g. localhost:8080 instead of http://localhost:8080), EnvHttpProxyAgent now prepends the appropriate scheme before passing the value to ProxyAgent.

  • HTTP_PROXY / httpProxy values default to http://
  • HTTPS_PROXY / httpsProxy values default to https://
  • Values that already include a scheme (e.g. http://, https://, socks5://) are unchanged

This matches the behavior of curl and Go's httpproxy package, which both assume http:// for scheme-less proxy values. The split behavior (http:// for HTTP_PROXY, https:// for HTTPS_PROXY) was requested by a maintainer in #4860 (comment) and aligns with what curl does.

Changes

  • Added normalizeProxyUrl() helper in lib/dispatcher/env-http-proxy-agent.js
  • Applied normalization to both env var and constructor option code paths
  • Added 6 new test cases covering scheme-less values for env vars, options, hostname-only values, and the no-op case for URLs that already have a scheme

Test plan

  • All 40 tests in test/env-http-proxy-agent.js pass (34 existing + 6 new)
  • Lint passes

When HTTP_PROXY or HTTPS_PROXY env vars (or httpProxy/httpsProxy options)
are set without a URL scheme (e.g. "localhost:8080" instead of
"http://localhost:8080"), EnvHttpProxyAgent now prepends the appropriate
scheme rather than passing the value verbatim to ProxyAgent, which would
fail on URL parsing.

HTTP_PROXY values default to http://, and HTTPS_PROXY values default to
https://. This matches the behavior of curl and Go's httpproxy package.
Values that already have a scheme are left unchanged.

Fixes: nodejs#4736

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.91%. Comparing base (51fd661) to head (806eaed).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4914      +/-   ##
==========================================
- Coverage   92.92%   92.91%   -0.01%     
==========================================
  Files         112      112              
  Lines       35646    35668      +22     
==========================================
+ Hits        33123    33141      +18     
- Misses       2523     2527       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Schema-less HTTP_PROXY and HTTPS_PROXY env vars should maybe assume http

3 participants