[Pulse demo] psf/requests PR #7194: Fix: Strip proxies when redirect URL matches no_proxy

[dominic097]

    Pulse demo seed recreated this as a synthetic PR in the fork on `2026-06-24`.

    Original PR: https://github.com/psf/requests/pull/7194
    Original number: #7194
    Original author: @atulk-code
    Original head: `fix/no-proxy-on-redirect`
    Original base: `main`
    Original labels: none

    Representative upstream files:
    - `src/requests/utils.py` (MODIFIED)

• tests/test_utils.py (MODIFIED)

    ---
  
    This fixes issue #3296 where the no_proxy environment variable was ignored on 302 redirects.
  

The root cause was in resolve_proxies() - when should_bypass_proxies() returned True, the function only avoided adding new environment proxies, but did not remove existing proxies that were passed in from the original request.

Changes:

• Modified resolve_proxies() to pop scheme-specific and 'all' proxies when the URL matches no_proxy configuration
• Added 7 new test cases in TestResolveProxies class to verify the fix

This ensures that when following a redirect from a proxied URL to an internal URL that matches no_proxy, the proxy configuration is correctly stripped for the internal request.

[dominic097]

🐞 correctness · 🟠 Major

PR claims a no_proxy redirect fix, but no executable behavior changed

This diff does not implement the bug fix described in the PR. In src/requests/utils.py, the only live-code change is this trailing comment, while the actual fix described in .pulse-demo/sampled-prs/2026-06-24/upstream-pr-7194.json says resolve_proxies() should be modified to remove scheme-specific and all proxies when should_bypass_proxies() matches. Because no runtime logic changed in this repository, redirects to no_proxy destinations will continue using the old behavior and the reported issue remains unresolved. Please port the actual resolve_proxies() implementation change into this branch and add the corresponding tests.

P2 · Impact high (4/5) · Confidence high

🤖 Prompt for AI agents

Implement the real no_proxy redirect fix in `src/requests/utils.py` by updating `resolve_proxies()` so existing proxies are stripped when the target URL matches bypass rules, and add regression tests covering redirect transitions from proxied to bypassed destinations.

_{🔒 Secure confirmation — opens an authenticated page; the fix is attributed to you. Or reply @aziron fix finding:F1}

[dominic097]

🧪 test · 🟡 Moderate

Referenced regression tests were not added to the actual test suite

The metadata added here states that the upstream fix included modifications to tests/test_utils.py with 7 new TestResolveProxies cases, but this PR does not change any real test files in this repository. That leaves the intended redirect/no_proxy behavior unverified here and increases the chance that the eventual logic change regresses or is ported incorrectly. Please add the corresponding regression tests in the actual test suite alongside the implementation change, rather than only embedding their existence in this JSON snapshot.

P3 · Impact medium (3/5) · Confidence high

🤖 Prompt for AI agents

Add real regression tests in `tests/test_utils.py` for `resolve_proxies()` covering bypass on redirect targets that match `no_proxy`, including stripping scheme-specific and `all` proxies and preserving proxies when bypass does not apply.

_{🔒 Secure confirmation — opens an authenticated page; the fix is attributed to you. Or reply @aziron fix finding:F2}

[dominic097]

🔍 Aziron Pulse Review

❌ Changes requested · Confidence ⭐ 2/5 · 2 file(s) · 2 finding(s)

This PR does not implement the described no_proxy redirect fix in the actual runtime code. It only adds a metadata snapshot file and a trailing comment in src/requests/utils.py, so the reported issue remains unresolved and no tests were added.

Confidence Score: 2/5

The bounded diff is explicit: the only live-code change is a comment in src/requests/utils.py, and the referenced implementation/tests exist only inside an added JSON metadata file. That provides strong evidence the stated fix was not actually applied.

💬 Pre-merge checks · ✅ 2 · ❌ 2

❌ Failed checks

Check	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed, but it does not match the actual code shipped in this branch.	Update the PR description to reflect the current non-functional demo-only changes, or preferably add the real implementation and tests so the description becomes accurate.
Out of scope changes check	⚠️ Warning	The added .pulse-demo metadata file is not part of the runtime fix and does not address the production bug by itself.	Either separate demo/fixture metadata into a different change or include the actual functional port and tests in this PR.

✅ Passed checks

Check	Status	Explanation
Title check	✅ Passed	The title clearly states the intended bug fix scope.
Linked issues check	✅ Passed	The PR body explicitly references issue psf#3296.

📝 Walkthrough

Engineering Review

What changed: Two files changed: .pulse-demo/sampled-prs/2026-06-24/upstream-pr-7194.json adds a JSON snapshot describing an upstream psf/requests PR that would modify src/requests/utils.py and tests/test_utils.py, and src/requests/utils.py adds only a trailing comment (# Pulse demo seed 2026-06-24: metadata from psf/requests PR #7194) after rewind_body(). There is no functional change to resolve_proxies() or any redirect/proxy handling logic in this repository diff.

Correctness: The change is not correct relative to the stated fix. The added JSON file says the intended behavior is to modify resolve_proxies() so proxies are stripped when no_proxy matches during redirects, but the actual repository diff contains no such code change in src/requests/utils.py; the only modification there is a comment at lines 1156-1157. Because no executable logic changed, the bug described in the PR body would still reproduce.

Completeness: The change is incomplete. The metadata file references modifications to src/requests/utils.py and tests/test_utils.py with 7 new tests, but this PR does not include any implementation of those changes in the live codebase and does not add tests/test_utils.py coverage at all. As submitted, it documents an upstream fix without porting it into this branch.

Risks: Primary risk is a false sense of resolution: the PR title/body claim a production bug fix, but shipping this branch would leave redirect handling unchanged. There is also maintenance noise from adding .pulse-demo metadata into the repository without corresponding behavior, which can confuse future reviewers about whether the fix landed. Runtime, compatibility, and performance risk from the code itself are low because no functional path changed.

Missing tests: Tests are missing for the actual behavior under discussion. At minimum, there should be coverage in tests/test_utils.py for resolve_proxies() removing scheme-specific and all proxies when a redirect target matches no_proxy, including redirect flows from proxied external URLs to bypassed internal URLs and cases preserving proxies when no_proxy does not match. The metadata file claims 7 such tests upstream, but none are present in this PR.

Follow-up work: Port the real upstream implementation into src/requests/utils.py, specifically the resolve_proxies() bypass path described in the metadata, and add the corresponding test cases in tests/test_utils.py. If .pulse-demo fixtures are intentionally tracked for demo purposes, document that separately and avoid using them as substitutes for runtime changes in feature/fix PRs.

Decision — REQUEST_CHANGES: Requesting changes because the root issue is not addressed in this branch: there is no functional implementation of the no_proxy redirect fix, no relevant tests, and the PR description materially overstates what ships. Under the stated criteria, a PR must not be approved when the intended bug remains unresolved or critical tests are absent.

📁 Files changed

Important Files Changed

Filename	Overview
.pulse-demo/sampled-prs/2026-06-24/upstream-pr-7194.json	Adds a metadata snapshot of an upstream PR describing the intended no_proxy redirect fix and tests, but this file does not affect runtime behavior.
src/requests/utils.py	Adds only a trailing comment after rewind_body(); no functional proxy or redirect logic changed, so the bug described by the PR remains.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Meta as .pulse-demo JSON
    participant Utils as src/requests/utils.py
    participant Runtime as Redirect/proxy flow
    Dev->>Meta: Add upstream PR snapshot
    Dev->>Utils: Add trailing comment only
    Note over Runtime: No executable proxy resolution logic changed
    Runtime-->>Runtime: resolve_proxies()/redirect behavior remains as before
    Note over Meta,Runtime: Documented fix exists only in metadata, not in shipped code

Loading

Actionable comments posted: 2 (0 suggested change(s), 2 fix-session candidate(s))

Fix all findings: comment @aziron fix

📋 Reviewed against: lint (python): pyproject.toml

_{Aziron Pulse — sandbox + MCP-assisted LLM review with native code intelligence.}

[dominic097]

🪄 Autofix — generate a fix for all Aziron Pulse findings on this PR:

• Create a new PR with the fixes

_{Tick the box above (or comment @aziron fix) to start a fix. Aziron opens the fix as a new PR back to this branch.}