Skip to content

[Pulse demo] psf/requests PR #5257: Safely handle, when uri contains IPv6 link local with %zone index - #30

Open
dominic097 wants to merge 1 commit into
mainfrom
pulse-demo/2026-06-24/pr-5257-8
Open

[Pulse demo] psf/requests PR #5257: Safely handle, when uri contains IPv6 link local with %zone index #30
dominic097 wants to merge 1 commit into
mainfrom
pulse-demo/2026-06-24/pr-5257-8

Conversation

@dominic097

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

    Original PR: https://github.com/psf/requests/pull/5257
    Original number: #5257
    Original author: @jjlogu
    Original head: `master`
    Original base: `main`
    Original labels: none

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

    ---

    ## Issue:

When URI contains IPv6 link local with zone index (Ex: fe80::726f:8a26:222a:2bf3%eth0), requote_uri encodes all % to %25. Because of it, requests.get was requesting different resource.

Screenshot:
image

I'm proposing to ignore domain part in utils.unquote_unreserved.

Actual Result

>>> requests.utils.requote_uri('http://[fe80::726f:8a26:222a:2bf3%eth0]:8080/a%20b/index.html')
'http://[fe80::726f:8a26:222a:2bf3%25eth0]:8080/a%2520b/index.html'

Expected Result

>>> requests.utils.requote_uri('http://[fe80::726f:8a26:222a:2bf3%eth0]:8080/a%20b/index.html')
'http://[fe80::726f:8a26:222a:2bf3%eth0]:8080/a%20b/index.html'

System Information

# python3 -m requests.help
{
  "chardet": {
    "version": "3.0.4"
  },
  "cryptography": {
    "version": "2.2.2"
  },
  "idna": {
    "version": "2.8"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.5.2"
  },
  "platform": {
    "release": "4.4.71-UNRELEASED-v4-00050-g76f27ecf9a52",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "1000207f",
    "version": "18.0.0"
  },
  "requests": {
    "version": "2.22.0"
  },
  "system_ssl": {
    "version": "1000207f"
  },
  "urllib3": {
    "version": "1.24.1"
  },
  "using_pyopenssl": true
}

@dominic097 dominic097 added the pulse-demo-seed Generated Pulse demo seed data label Jun 24, 2026
@dominic097

Copy link
Copy Markdown
Owner Author

🔍 Aziron Pulse Review

💬 Commented · Confidence ⭐ 2/5 · 2 file(s) · 0 finding(s)

This PR only adds Pulse demo metadata files and does not modify requests runtime code or tests. There are no actionable correctness issues in the diff itself, but it also does not implement the IPv6 zone-index fix described in the sampled upstream PR payload.

Confidence Score: 2/5

The diff is very small and fully visible, and both changed files are plainly non-runtime metadata assets. There is little ambiguity about the absence of executable code changes or tests.

💬 Pre-merge checks · ✅ 2 · ❌ 2

❌ Failed checks

Check Status Explanation Resolution
Description check ⚠️ Warning Based on the visible diff, the change only adds demo metadata files, so any description implying a runtime IPv6 fix would be misleading for this repository PR. Clarify in the PR description that this is a Pulse demo metadata import, not an implementation of the requests/utils.py fix.
Linked issues check ⚠️ Warning The diff references upstream psf/requests PR psf#5257 inside the added JSON, but there is no visible linkage to a local tracking issue describing why these demo assets are being added. Link a local tracking issue or note the demo/fixture purpose explicitly in the PR description.

✅ Passed checks

Check Status Explanation
Title check ✅ Passed The PR title is coherent and matches the demo metadata being added for upstream PR psf#5257.
Out of scope changes check ✅ Passed The diff is tightly scoped to two .pulse-demo metadata files and contains no unrelated code changes.
📝 Walkthrough

Engineering Review

What changed: The change adds two non-runtime files under .pulse-demo/: a JSON snapshot of upstream psf/requests PR psf#5257 at .pulse-demo/sampled-prs/2026-06-24/upstream-pr-5257.json and a short markdown seed file at .pulse-demo/sampled-prs/upstream-pr-5257.md. No production Python modules, tests, packaging metadata, or CI configuration are changed in this diff.

Correctness: For the codebase under review, the diff is mechanically correct because it only adds static metadata content and a comment file; there is no executable behavior change to validate. Evidence: the only changed paths are .pulse-demo/sampled-prs/2026-06-24/upstream-pr-5257.json and .pulse-demo/sampled-prs/upstream-pr-5257.md, and the JSON content merely records details of an upstream PR whose file list mentions requests/utils.py but does not modify that file in this repository.

Completeness: If the intent of this repository PR is to seed/demo upstream PR metadata, the change is complete enough for that narrow purpose because both the JSON fixture and companion markdown file were added. If the intent is to fix the IPv6 link-local zone-index handling described in the embedded upstream PR body, the change is not complete because no implementation or tests were added in requests/utils.py or the test suite; however, that gap is not actionable against the submitted diff because the current PR does not claim to alter runtime code beyond demo assets.

Risks: Runtime risk is effectively nil because no executable paths are touched. The main project risk is process/expectation confusion: the added JSON describes a bug fix in requests/utils.py, but shipping this PR alone would not change application behavior, which could mislead readers if they infer the sampled upstream PR has been applied locally.

Missing tests: No tests are required for the files actually changed here because they are static demo/fixture artifacts. Separately, if a future PR implements the referenced IPv6 zone-index fix, it should include regression tests for requests.utils.requote_uri covering bracketed IPv6 hosts with zone indices, preservation of %zone in the authority component, and correct handling of percent-encoded path segments such as /a%20b without double-encoding.

Follow-up work: Create a separate implementation PR if the repository intends to actually adopt upstream PR psf#5257 behavior in requests/utils.py, along with targeted unit tests around requote_uri/unquote_unreserved. Consider documenting in the Pulse demo directory that these files are metadata samples only and are not applied patches, to avoid confusion for maintainers and reviewers.

Decision — COMMENT: Per the decision criteria, approve is not appropriate because the root issue described in the embedded upstream PR is not addressed by this repository diff, and there is no test coverage for such a fix. Request_changes is also not appropriate because there is no concrete defect in the submitted changes themselves; they are non-executable demo artifacts. That makes comment the best fit: advisory feedback only, with no blocking inline defects in the actual modified lines.

📁 Files changed

Important Files Changed

Filename Overview
.pulse-demo/sampled-prs/2026-06-24/upstream-pr-5257.json Adds a JSON snapshot of upstream psf/requests PR psf#5257, including its description and a summarized upstream file list; no local runtime behavior is changed.
.pulse-demo/sampled-prs/upstream-pr-5257.md Adds a short markdown seed comment identifying the file as Pulse demo metadata; no functional impact.
🧩 Findings

Findings

No blocking findings were included in the approved draft.

Fix all findings: comment @aziron fix

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pulse-demo-seed Generated Pulse demo seed data

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant