Feature 2446 no proxy env alternative#2449
Open
unterwegi wants to merge 5 commits into
Open
Conversation
In preparation for NO_PROXY support (yhirose#2446), centralize the proxy-enabled decision in a single helper so the upcoming bypass logic can be added in one place rather than to six divergent call sites. The helper's body for now is identical to the existing condition; the host parameter is unused until set_no_proxy() lands. Refactored sites: ClientImpl::create_client_socket ClientImpl::handle_request (HTTP request rewrite) ClientImpl::setup_redirect_client ClientImpl::process_request (SSL is_proxy_enabled flag) SSLClient::setup_proxy_connection SSLClient::ensure_socket_connection The two prepare_default_headers Proxy-Authorization injection blocks (currently gated only on proxy auth credentials being set) are intentionally not wrapped here. Doing so would change behavior in the rare misconfiguration case where credentials are set without set_proxy, so the gating is deferred to the NO_PROXY commit where it becomes meaningful. No behavior change. All 608 unit tests and the 22 squid-backed proxy tests pass.
27 black-box tests exercising the public Client API only (no detail::
calls, BORDER-friendly; no EXPECT_NO_THROW, -fno-exceptions-friendly).
In-process proxy mock + target server. Each test asserts which side
of the routing decision each request landed on, and what headers (in
particular Proxy-Authorization) the receiving side saw.
Coverage:
Suffix matching (dot-boundary rule)
- exact-host match
- subdomain match
- "evilexample.com" does NOT match "example.com" ← regression
guard for the classic NO_PROXY suffix-match pitfall
- "example.com.evil.com" does NOT match
- leading-dot pattern still matches the bare domain (Go/curl
convention)
- case-insensitive
- trailing-dot host normalization
Wildcard
- "*" bypasses everything
IP normalization
- exact IPv4 match
- "::1" matches "0:0:0:0:0:0:0:1" via inet_pton
- IPv4-mapped IPv6 ("::ffff:127.0.0.1") is NOT cross-matched
against an IPv4 entry
CIDR
- basic v4 in-cidr / not-in-cidr
- "0.0.0.0/0" (prefix=0; verifies no shift UB)
- bare IP treated as /32
- malformed prefix (/33) silently dropped → no NO_PROXY effect
Proxy-Authorization handling
- suppressed when NO_PROXY matches the target
- sent when NO_PROXY does not match
Backward compat
- default behavior unchanged when set_no_proxy is never called
Parsing edge cases
- port-specific entries ("host:port") rejected
- empty / whitespace tokens dropped
Cross-origin redirect (analog of GHSA-6hrp-7fq9-3qv2)
- redirect target in NO_PROXY → redirect leg goes direct, no
Proxy-Authorization carried over
set_proxy_from_env (Unix only — uses setenv/unsetenv)
- lowercase http_proxy applied
- uppercase HTTP_PROXY ignored (httpoxy / CVE-2016-5385)
- NO_PROXY-only env returns true and applies the bypass list
- CRLF in env value rejected (cf. CVE-2026-21428)
- empty env value treated as unset
635 tests (608 prior + 27 new) pass under both the regular and the
split builds.
Provides an alternative implementation for is_proxy_enabled_for_host() that handles wildcard (*), exact host, subdomain, and IPv4/IPv6 CIDR matching. In contrast to PR yhirose#2448, the no_proxy entries and the host URL are not normalized beforehand, parsing and checking only happens lazily as needed.
yhirose
added a commit
that referenced
this pull request
May 14, 2026
Three regression guards added during review of an alternate NO_PROXY implementation (PR #2449). All three pass on the current implementation and surface bugs in the alternate one: - BareIPv6LiteralMatchesIPv6Cidr: a host given as a bare IPv6 literal (no surrounding brackets) must still be recognized as IPv6 for CIDR matching. An implementation that only detects IPv6 when the host string starts with '[' would split the host at the first ':' and misclassify it as a hostname. - TrailingDotOnEntryIsNormalized: trailing dots must be canonicalized on BOTH sides — host and entry. An implementation that strips the host-side trailing dot only would fail to match host "example.com" against entry "example.com." because the substring lengths differ. - ValidEntryWithSurroundingWhitespaceStillMatches: an entry with leading/trailing whitespace must still match. An implementation that feeds raw tokens directly to inet_pton would reject valid CIDRs (" 10.0.0.0/8 ") because of the spaces. 635 unit tests pass.
yhirose
added a commit
that referenced
this pull request
May 14, 2026
Adopts the unified 16-byte address representation suggested by the alternate NO_PROXY implementation in PR #2449. Both v4 and v6 entries now share one storage type and one matcher; the v4/v6 distinction is only the address-family flag and the max prefix length. - detail::NoProxyEntry: replaces in_addr v4_net + in6_addr v6_net with a single IPBytes net (std::array<uint8_t, 16>). v4 occupies the first 4 bytes, v6 fills all 16. - detail::NormalizedTarget: replaces in_addr v4 + in6_addr v6 with a single IPBytes ip. - Replaces detail::ipv4_in_cidr and detail::ipv6_in_cidr with one detail::ip_in_cidr that takes the address, the network, the prefix length and the family's max bits (32 for v4, 128 for v6). The mask is constructed by the byte-fill approach from the previous v6 helper, which is straightforward to read and avoids the shift UB that the v4 helper had to special-case. - The NoProxyKind enum keeps IPv4Cidr / IPv6Cidr as separate values so the match dispatch stays explicit and IPv4 entries cannot accidentally cross-match an IPv6 target (the same address-family isolation the previous code had). Net change: -28 lines + -1 helper function. All 30 NoProxyTest cases plus 643 unit tests pass under both the regular and split builds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR provides an alternative implementation for parsing proxy related environment variables and no_proxy handling in general as discussed in #2446.
It reuses many parts of PR #2448, especially the scaffolding to integrate is_proxy_enabled_for_host() in all the needed parts of the client code, most of the environment variable parsing and also the full NoProxyTests test suite.
This implementation handles wildcard (*), exact host (case insensitive), subdomain (case insensitive), and IPv4/IPv6 CIDR matching.
In contrast to the reference PR, the no_proxy entries and the host URL are not normalized and parsed beforehand, parsing and conversions only happen lazily as needed.