Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds RFC 6874–aware IPv6 zone identifier handling: detects zone-ids in URLs, preserves/normalizes %25 encoding during URL preparation, and routes adapter pool-key parsing through urllib3 parsing for zone-id cases. Includes expanded tests covering detection, pool-key behavior, and adapter send flows. Changes
Sequence DiagramsequenceDiagram
actor Client
participant PreparedRequest
participant HTTPAdapter
participant _urllib3_request_context
participant urllib3_PoolManager as urllib3 PoolManager
Client->>PreparedRequest: prepare_url(url with IPv6 zone-id)
Note over PreparedRequest: Detect bracketed authority with zone-id\nReconstruct host to ensure %25 encoding
PreparedRequest-->>Client: Prepared URL (canonical encoding)
Client->>HTTPAdapter: send(prepared_request)
HTTPAdapter->>HTTPAdapter: build_connection_pool_key_attributes()
HTTPAdapter->>_urllib3_request_context: call with URL (poolmanager optional)
Note over _urllib3_request_context: Detect IPv6 zone-id via regex
alt Zone-ID Present
_urllib3_request_context->>_urllib3_request_context: Use urllib3.util.parse_url()
_urllib3_request_context->>_urllib3_request_context: Derive host from parsed.host and strip brackets
else No Zone-ID
_urllib3_request_context->>_urllib3_request_context: Use urlparse()
_urllib3_request_context->>_urllib3_request_context: Use hostname directly
end
_urllib3_request_context-->>HTTPAdapter: return pool key attrs (host, scheme, port)
HTTPAdapter->>urllib3_PoolManager: connection_from_host(host, scheme, port)
urllib3_PoolManager-->>HTTPAdapter: Connection pool
HTTPAdapter-->>Client: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/requests/adapters.py (1)
117-178:⚠️ Potential issue | 🟠 MajorZone-ID URL parsing is incomplete — request preparation fails before reaching the pool key builder.
The new
_urllib3_request_context()switches tourllib3.parse_url(), but request preparation itself still uses stdlib parsing at three sites:PreparedRequest.prepare_auth()callsget_auth_from_url(),RequestEncodingMixin.path_urlusesurlsplit(), and proxied requests callurldefragauth(). All three use stdliburlparse(), which validates and rejects IPv6 zone IDs on Python 3.14+. The test attests/test_adapters.py:349-353 already expects failure (ValueErrororInvalidURL`) on Python 3.14+ rather than proving end-to-end support. Either route all URL reparses through urllib3-safe helpers or restructure to avoid reparsing the prepared URL.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/requests/adapters.py` around lines 117 - 178, PreparedRequest.prepare_auth(), RequestEncodingMixin.path_url, and places calling urldefragauth still use stdlib urlparse/urlsplit and reject IPv6 zone IDs even though _urllib3_request_context now uses urllib3.parse_url; update the code to avoid reparsing the prepared URL with stdlib parsers by routing reparses through urllib3-safe helpers or by changing callers to accept the already-prepared parsed result: replace usages in PreparedRequest.prepare_auth (and its helper get_auth_from_url), RequestEncodingMixin.path_url, and any proxied request code that calls urldefragauth/urlsplit so they call an urllib3-safe parse helper (or accept/propagate the parsed_request_url built in _urllib3_request_context) to preserve IPv6 zone-id support and prevent ValueError/InvalidURL on Python 3.14+. Ensure you reference and update get_auth_from_url, RequestEncodingMixin.path_url, and any urldefragauth callers to use the new helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/requests/models.py`:
- Around line 90-96: Reformat the two regex definitions so they comply with
ruff-format: run ruff-format (or your project's formatting command) on the file
containing _RFC6874_ZONE_ID_RE and _RAW_ZONE_ID_RE to let the tool rewrite the
re.compile(...) blocks into the CI-expected layout, then re-stage and commit the
updated file; ensure you target the re.compile calls for _RFC6874_ZONE_ID_RE and
_RAW_ZONE_ID_RE so the lint job stops rewriting them.
In `@tests/test_adapters.py`:
- Around line 19-421: The file tests/test_adapters.py fails CI style checks; run
the project formatter (ruff format) on the file and commit the resulting changes
to fix signature/parametrize formatting in the test classes (notably
TestIPv6ZoneIDDetection, TestIPv6ZoneIDParsing, TestIPv6ZoneIDRequests and their
test_* methods like test_has_ipv6_zone_id and test_ipv6_zone_id_url_parsing);
simply run `ruff format tests/test_adapters.py` (or the repo's configured
ruff-format command), review the diff, and add/commit the formatted file.
---
Outside diff comments:
In `@src/requests/adapters.py`:
- Around line 117-178: PreparedRequest.prepare_auth(),
RequestEncodingMixin.path_url, and places calling urldefragauth still use stdlib
urlparse/urlsplit and reject IPv6 zone IDs even though _urllib3_request_context
now uses urllib3.parse_url; update the code to avoid reparsing the prepared URL
with stdlib parsers by routing reparses through urllib3-safe helpers or by
changing callers to accept the already-prepared parsed result: replace usages in
PreparedRequest.prepare_auth (and its helper get_auth_from_url),
RequestEncodingMixin.path_url, and any proxied request code that calls
urldefragauth/urlsplit so they call an urllib3-safe parse helper (or
accept/propagate the parsed_request_url built in _urllib3_request_context) to
preserve IPv6 zone-id support and prevent ValueError/InvalidURL on Python 3.14+.
Ensure you reference and update get_auth_from_url,
RequestEncodingMixin.path_url, and any urldefragauth callers to use the new
helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7f02d69-94ba-45e2-ac12-9ed4557164a0
📒 Files selected for processing (3)
src/requests/adapters.pysrc/requests/models.pytests/test_adapters.py
Adds support for IPv6 zone identifiers (scope IDs) in URLs, enabling correct handling of link-local IPv6 addresses such as http://[fe80::1%25eth0]:8080/. Changes: - adapters.py: Add _has_ipv6_zone_id() helper with precompiled regex (_IPV6_ZONE_ID_RE) to detect zone IDs in the URL authority section. Update _urllib3_request_context() to use urllib3's parse_url() for zone ID URLs and urlparse() for all others, preserving backward compatibility. The poolmanager parameter is kept as an optional default-None argument to avoid breaking subclass callers. - models.py: Add zone ID re-encoding logic in prepare_url() with three precompiled patterns (_AUTHORITY_BRACKET_RE, _RFC6874_ZONE_ID_RE, _RAW_ZONE_ID_RE). Reconstructs the bracketed host from the original URL to prevent parse_url's %25->% decoding from creating ambiguous percent sequences downstream (e.g. %2550 -> %50 on Python 3.14). - tests/test_adapters.py: Comprehensive test coverage including zone ID detection (38 parametrized cases), URL parsing (15 cases), and integration tests covering pool key generation, client certificates, full mocked request flow, encoding equivalence, URL preparation preservation, request_url() path extraction, and proxy code path. Fixes psf#6927, psf#6735, psf#7088. Made-with: Cursor
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
1 participant
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.
Adds support for IPv6 zone identifiers (scope IDs) in URLs, enabling correct handling of link-local IPv6 addresses such as http://[fe80::1%25eth0]:8080/.
Changes:
Fixes psf#6927, psf#6735, psf#7088.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests