Skip to content

Fix IPv6 Zone ID decoding to correctly handle RFC 6874 %25 separator#1653

Merged
bdraco merged 4 commits into
aio-libs:masterfrom
rodrigobnogueira:fix-ipv6-zone-id
May 26, 2026
Merged

Fix IPv6 Zone ID decoding to correctly handle RFC 6874 %25 separator#1653
bdraco merged 4 commits into
aio-libs:masterfrom
rodrigobnogueira:fix-ipv6-zone-id

Conversation

@rodrigobnogueira

@rodrigobnogueira rodrigobnogueira commented Apr 12, 2026

Copy link
Copy Markdown
Member

What do these changes do?

Fixes incorrect decoding of IPv6 Zone IDs in URLs that use the
:rfc:6874 %25 zone separator.

Two call sites in yarl/_url.py are updated:

  • _encode_host(): when %25 is present in the host string,
    partition on %25 (the RFC 6874 separator) instead of bare
    %. The raw form (raw_host / host_subcomponent /
    str(url)) is preserved verbatim.
  • .host property: when an IP-literal host contains %25,
    decode it back to % so callers receive the human-readable
    zone identifier (e.g. fe80::1%1 rather than fe80::1%251).
    The same decoding flows through .authority and
    .human_repr().

The pre-fix behaviour, where _encode_host split on bare %,
is still accepted as a fallback so that hosts constructed
programmatically (e.g. with_host("fe80::1%1")) keep working.

URL .raw_host .host before .host after
http://[fe80::1%251]/ fe80::1%251 fe80::1%251 fe80::1%1
http://[fe80::1%25eth0]/ fe80::1%25eth0 fe80::1%25eth0 fe80::1%eth0

Are there changes in behavior for the user?

Yes. .host, .authority, and .human_repr() now return
the decoded zone identifier (% instead of %25) for IPv6
URLs that carry an RFC 6874 zone separator. raw_host,
host_subcomponent, and str(url) are unchanged, so the
on-the-wire representation is preserved.

Is it a substantial burden for the maintainers to support this?

No. The change is localised to two small spots in yarl/_url.py;
the dual-path encoder fallback preserves backward compatibility for
programmatically constructed hosts. While not a practical SSRF
vector (zone IDs are link-local scope only), the previous decoding
disagreed with other RFC 6874-aware parsers.

Related issue number

No linked issue; surfaced while reviewing :rfc:6874 compliance
of the host parser.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt (N/A; no CONTRIBUTORS.txt in this repo)
  • Add a new news fragment into the CHANGES/ folder
    • name: 1653.bugfix.rst
    • category: bugfix

Per RFC 6874, an IPv6 Zone ID in a URI is encoded as:
  IPv6addrz = IPv6address "%25" ZoneID

So in 'http://[fe80::1%251]/', the zone ID is '1', not '251'.

Previously, _encode_host() split the host on bare '%', treating '251'
as the zone ID.  The host property also returned the raw (encoded)
value unchanged for IP addresses, so %25 was never decoded.

Fix _encode_host() to partition on '%25' (RFC 6874 separator) when
present, preserving it verbatim in raw_host / str(url), and update
the host property to decode '%25' -> '%' so callers receive the
human-readable zone identifier (e.g. 'fe80::1%1' / 'fe80::1%eth0').

Tests added for:
- Numeric zone ID: http://[fe80::1%251]/  -> host='fe80::1%1'
- String zone ID:  http://[fe80::1%25eth0]/ -> host='fe80::1%eth0'
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Apr 12, 2026
@codspeed-hq

codspeed-hq Bot commented Apr 12, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 97 untouched benchmarks


Comparing rodrigobnogueira:fix-ipv6-zone-id (defc613) with master (fbf8cfd)

Open in CodSpeed

@codecov

codecov Bot commented Apr 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.81%. Comparing base (fbf8cfd) to head (defc613).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1653   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files          21       21           
  Lines        4249     4286   +37     
  Branches      250      251    +1     
=======================================
+ Hits         4241     4278   +37     
  Misses          5        5           
  Partials        3        3           
Flag Coverage Δ
CI-GHA 99.74% <100.00%> (+<0.01%) ⬆️
OS-Linux 99.74% <100.00%> (+<0.01%) ⬆️
OS-Windows 98.53% <100.00%> (+0.01%) ⬆️
OS-macOS 98.67% <100.00%> (+0.01%) ⬆️
Py-3.10 99.69% <100.00%> (+<0.01%) ⬆️
Py-3.11 99.69% <100.00%> (+<0.01%) ⬆️
Py-3.12 99.69% <100.00%> (+<0.01%) ⬆️
Py-3.13 99.69% <100.00%> (+<0.01%) ⬆️
Py-3.14 99.69% <100.00%> (+<0.01%) ⬆️
Py-3.14t 99.69% <100.00%> (+<0.01%) ⬆️
Py-pypy-3.11 99.37% <100.00%> (+<0.01%) ⬆️
VM-macos-latest 98.67% <100.00%> (+0.01%) ⬆️
VM-ubuntu-latest 99.74% <100.00%> (+<0.01%) ⬆️
VM-windows-latest 98.53% <100.00%> (+0.01%) ⬆️
pytest 99.74% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rodrigobnogueira

Copy link
Copy Markdown
Member Author

The codecov/project/typing check is failing at 97.63% against a 100% target, but this is a pre-existing issue unrelated to this PR.

@bdraco

bdraco commented May 17, 2026

Copy link
Copy Markdown
Member

@aiolibsbot review

@aiolibsbot

aiolibsbot commented May 17, 2026

Copy link
Copy Markdown
Contributor

PR Review — Fix IPv6 Zone ID decoding to correctly handle RFC 6874 %25 separator

Focused, well-scoped RFC 6874 bugfix. The dual-path encoder cleanly preserves backward compatibility for programmatically constructed hosts while teaching the parser the correct %25 zone separator, and the .host decode is appropriately limited to %25 so other percent-encoded characters can't be smuggled through (the author's safety analysis on #3260235846 covers this well). Tests cover the canonical numeric zone, named zone, port-attached, and the subtle multi-%25 case where the zone identifier itself contains an encoded percent; each asserts URL(str(url)) == url for round-trip. CHANGES/1653.bugfix.rst follows the project style (past tense, rST, -- by :user: trailer, no PR number in body). Two non-blocking nits inline: the docstring/comment around the new replace understates what the code does, and the encoder fallback path that the PR explicitly preserves is not directly exercised by a new test. Merge-ready otherwise.


🟢 Suggestions

1. Comment understates what the replace actually does (`yarl/_url.py`, L792-794)

The comment says only the RFC 6874 zone separator needs to be decoded, but raw.replace("%25", "%") decodes every %25 in the raw host string, not just the separator. This is actually intentional and correct (the test test_ipv6_zone_rfc6874_pct_encoded_inside_zone covers it), but the comment as written suggests a single, separator-only decode. Consider rewording to match what the code does, e.g. only %25 sequences need to be decoded back to % (the RFC 6874 zone separator, plus any %25 inside the zone identifier that encodes a literal %).

if raw and raw[-1].isdigit() or ":" in raw:
    # IP addresses are never IDNA encoded; only the RFC 6874
    # zone separator needs to be decoded.
    if "%25" in raw:
        return raw.replace("%25", "%")
2. Consider also exercising the bare-% encoder fallback (`tests/test_url.py`, L478-522)

The PR description and _encode_host comment both call out that the bare-% partition path is preserved on purpose so that programmatic construction (URL.build(host="fe80::1%1") / with_host("fe80::1%eth0")) keeps working. None of the new tests pin that path down, though, so a future refactor of _encode_host could quietly drop it and CI would stay green. A small additional case that asserts URL.build(host="fe80::1%eth0") round-trips through raw_host / .host / str(url) the way the existing code does today would lock in the contract you explicitly preserved.


Checklist

  • Behavior change is documented in CHANGES and the property docstring
  • New behavior is covered by tests (including round-trip and edge cases)
  • Backward-compat path (bare % in programmatic hosts) is preserved — suggestion #2
  • No security regression (decode is bounded to %25, raw_host/str(url) unchanged)
  • Comments accurately describe what the code does — suggestion #1
  • News fragment follows aio-libs conventions (past tense, rST, :user: trailer, no PR# in body)

Summary

Focused, well-scoped RFC 6874 bugfix. The dual-path encoder cleanly preserves backward compatibility for programmatically constructed hosts while teaching the parser the correct %25 zone separator, and the .host decode is appropriately limited to %25 so other percent-encoded characters can't be smuggled through (the author's safety analysis on #3260235846 covers this well). Tests cover the canonical numeric zone, named zone, port-attached, and the subtle multi-%25 case where the zone identifier itself contains an encoded percent; each asserts URL(str(url)) == url for round-trip. CHANGES/1653.bugfix.rst follows the project style (past tense, rST, -- by :user: trailer, no PR number in body). Two non-blocking nits inline: the docstring/comment around the new replace understates what the code does, and the encoder fallback path that the PR explicitly preserves is not directly exercised by a new test. Merge-ready otherwise.


Automated review by Kōan97ac79d
3c5905e
8b749ce
defc613

Comment thread yarl/_url.py Outdated
@bdraco

bdraco commented May 18, 2026

Copy link
Copy Markdown
Member

@aiolibsbot review

- Collapse the %25/% partition selection in _encode_host
  into a single ternary, per @bdraco's suggestion.
- Add an inline rationale: %25 is the RFC 6874 separator; bare
  % is still accepted for hosts constructed programmatically
  (e.g. via with_host('fe80::1%1')).
- Document the %25 -> % decoding in the .host docstring so
  callers see that the encoded form remains available via
  raw_host / host_subcomponent.
- Broaden tests to cover named zones, zone + port,
  host_port_subcomponent, authority, human_repr,
  round-tripping through str(url), and a percent-encoded byte
  inside the zone identifier.
@bdraco

bdraco commented May 26, 2026

Copy link
Copy Markdown
Member

Thanks @rodrigobnogueira

@bdraco bdraco merged commit ddc827b into aio-libs:master May 26, 2026
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants