[Pulse demo] psf/requests PR #7148: Add IPv6 CIDR support to no_proxy handling - #24
[Pulse demo] psf/requests PR #7148: Add IPv6 CIDR support to no_proxy handling#24dominic097 wants to merge 1 commit into
Conversation
dominic097
left a comment
There was a problem hiding this comment.
Aziron Pulse PR Review
This PR's actual code change to src/requests/utils.py is only a trailing comment seed line; it does not implement any IPv6 CIDR no_proxy logic described in the PR metadata. No functional change and no risk, but also nothing that addresses the stated goal.
Confidence Score: 5/5
The diff is tiny and fully visible: the only src change is a trailing comment, and the surrounding proxy functions are unchanged per the symbol spans, so the gap between PR description and actual change is unambiguous.
| Field | Value |
|---|---|
| Verdict | comment |
| Confidence | high |
| Changed files | 2 |
1 actionable finding(s) were posted inline on the diff (0 suggested change(s), 1 fix-session candidate(s)).
To generate one fix PR for all Pulse findings, comment:
@aziron fix
Aziron Pulse generated this from sandbox/MCP-assisted LLM review with native code intelligence context.
| else: | ||
| raise UnrewindableBodyError("Unable to rewind request body for redirect.") | ||
|
|
||
| # Pulse demo seed 2026-06-24: metadata from psf/requests PR #7148 |
There was a problem hiding this comment.
warning: This is the only change to utils.py, but it is just a demo-seed comment — no IPv6 CIDR no_proxy logic is implemented despite the PR title/body. Evidence: should_bypass_proxies (lines 810-870), is_valid_cidr (763-784), address_in_network (726-738), and is_ipv4_address (752-760) are all unchanged in this diff, and the claimed tests/test_ipv6_proxy.py and utils.py +48/-7 changes from the PR metadata are absent. Runtime impact: IPv6 CIDR ranges in the no_proxy env var are still not matched, so proxy bypass behaves exactly as before in IPv6 environments. Fix direction: implement IPv6 CIDR matching by extending the CIDR/address helpers to use ipaddress.ip_network/ip_address for both families inside should_bypass_proxies, and add the corresponding unit tests; or remove this seed comment if no functional change is intended.
Confidence: high
This requires a sandbox fix session.
Fix task: Implement IPv6 CIDR support in no_proxy handling in src/requests/utils.py. Update should_bypass_proxies to also match the target host against IPv6 CIDR entries in no_proxy, generalizing is_valid_cidr/address_in_network to use the stdlib ipaddress module (ip_network/ip_address) for both IPv4 and IPv6 while preserving existing IPv4 CIDR and exact-match behavior. Add tests/test_ipv6_proxy.py covering: host inside an IPv6 CIDR, host outside it, malformed IPv6 CIDR entries, and unchanged IPv4 behavior. Remove the trailing demo-seed comment if it is not intended for the real branch.
To generate a fix PR, comment:
@aziron fix finding:no-feature-impl
dominic097
left a comment
There was a problem hiding this comment.
Aziron Pulse PR Review
The PR does not implement the advertised IPv6 no_proxy CIDR behavior. The actual repo diff only adds metadata and a trailing comment, so the functional issue remains unresolved and no validating tests were added.
Confidence Score: 5/5
The bounded diff is explicit: only two files changed, and neither contains the functional IPv6 CIDR implementation described by the PR metadata. The relevant runtime symbols are present in context and remain outside the changed lines, so the mismatch between intent and implementation is clear.
| Field | Value |
|---|---|
| Verdict | request_changes |
| Confidence | high |
| Changed files | 2 |
1 actionable finding(s) were posted inline on the diff (0 suggested change(s), 1 fix-session candidate(s)).
To generate one fix PR for all Pulse findings, comment:
@aziron fix
Aziron Pulse generated this from sandbox/MCP-assisted LLM review with native code intelligence context.
| else: | ||
| raise UnrewindableBodyError("Unable to rewind request body for redirect.") | ||
|
|
||
| # Pulse demo seed 2026-06-24: metadata from psf/requests PR #7148 |
There was a problem hiding this comment.
error: This PR claims to add IPv6 CIDR support for no_proxy, but the actual code diff here only adds a trailing comment after rewind_body(). The proxy-bypass implementation lives in should_bypass_proxies() (src/requests/utils.py:810-870) with supporting helpers such as is_valid_cidr() (src/requests/utils.py:763-784) and address_in_network() (src/requests/utils.py:726-738), and none of those code paths are modified in this PR. The .pulse-demo/.../upstream-pr-7148.json file also claims tests were added, but no test file is part of the real diff. Runtime impact: users still will not get the advertised IPv6 CIDR bypass behavior, while the PR metadata suggests the bug is fixed. Fix direction: port the actual upstream logic into the proxy-bypass code path and add the corresponding unit coverage in the real test suite.
Confidence: high
This requires a sandbox fix session.
Fix task: Implement the real IPv6 CIDR no_proxy handling in src/requests/utils.py by updating should_bypass_proxies() and any required helpers, then add repository tests covering matching and non-matching IPv6 CIDR cases plus regressions for existing IPv4/domain behavior.
To generate a fix PR, comment:
@aziron fix finding:missing-ipv6-no-proxy-implementation
tests/test_ipv6_proxy.py(ADDED)This PR adds support for IPv6 CIDR ranges in the
no_proxyenvironmentvariable when determining proxy bypass behavior.
Motivation
While Requests already supports IPv4 CIDR ranges and exact IPv6 matches,
IPv6 CIDR ranges were not handled, leading to inconsistent behavior in
IPv6-enabled environments.
Testing