UriTemplate.match() replaces non-UTF-8 percent-escapes with U+FFFD, corrupting the value handed to resource handlers #3721

Activity

1. claude

   added

   bugSomething isn't workingSomething isn't working

   v2Affects the v2 line (2.x on main)Affects the v2 line (2.x on main)

   on Oct 17, 2026
2. nikolas-sapa commented on Oct 17, 2026

   

   nikolas-sapa

   on Oct 17, 2026

   More actions

   Confirmed the repro across all template operators: urllib.parse.unquote defaults to errors='replace', causing non-UTF-8 percent-encoded octets (e.g. file://caf%E9) to be replaced with U+FFFD (caf), destroying the raw octet before it reaches resource handlers and breaking bidirectional invertibility with expand().

   Fix approach: pass errors="surrogateescape" to unquote() at all capture sites and in _encode() for quote(). Verified all 9 operator forms round-trip cleanly and all 316 unit tests in tests/shared/test_uri_template.py pass.

   A working patch with full regression test coverage is ready on nikolas-sapa:fix/uritemplate-surrogateescape-3721 (prepared in #3724). Could a maintainer assign this issue to me so the PR reopens? Thanks!
3. Varshith-Kali commented on Oct 18, 2026

   

   Varshith-Kali

   on Oct 18, 2026

   More actions

   Hi, I would like to pick this up. I reproduced the silent U+FFFD substitution on current main across the operator forms in the table, and traced it to the unquote calls with default errors="replace" in the match path. My plan: decode captures with errors="surrogateescape" and encode with the same setting in _encode, so non-UTF-8 octets survive the round trip the match docstring claims (match(expand(v)) == v). Could a maintainer assign this issue to me?

   AI assistance disclosure: this contribution was drafted with AI coding assistance, and I reviewed and tested every line.
4. nosmile99 commented on Oct 20, 2026

   

   nosmile99

   on Oct 20, 2026

   More actions

   Confirmed on main (2af9e30) — reproduced the substitution across all 8 operator forms:

   file://{name}   file://caf%E9 -> {'name': 'caf�'}
   file://{+path}  file://caf%E9 -> {'path': 'caf�'}
   x{#frag}        x#caf%E9       -> {'frag': 'caf�'}
   x{.ext}         x.caf%E9       -> {'ext': 'caf�'}
   x{/seg}         x/caf%E9       -> {'seg': 'caf�'}
   x{;p}           x;p=caf%E9     -> {'p': 'caf�'}
   x{?q}           x?q=caf%E9     -> {'q': 'caf�'}
   x{/list*}       x/a/caf%E9     -> {'list': ['a', 'caf�']}
   

   The 6 capture sites are unquote(...) with the default errors="replace" (uri_template.py lines 607, 625, 657, 1015, 1048, 1114), so every undecodable octet is silently swapped for U+FFFD and the original byte is unrecoverable.

   For the record: #3724 (the surrogateescape fix) was closed by the require-linked-issue bot for process reasons, not a technical verdict — the approach itself is untested by review. Two shapes look viable: preserve the octet via surrogateescape (keeps the match + round-trip working), or treat undecodable captures as a match failure. Changing the observable behavior here is a design call, so I'd rather implement whichever the maintainers prefer.

   If a maintainer is willing to assign me to this issue, I can put up a PR with regression tests (fail pre-fix / pass post-fix) once the direction is picked.

   AI disclosure: this verification and any follow-up code were produced with AI assistance (Muse, agentic coding assistant).
5. Ben-Home commented on Oct 27, 2026

   

   Ben-Home

   on Oct 27, 2026

   More actions

   Both proposed fixes are correct about where the loss happens: the decode sites call unquote with no errors argument, so the default replace turns any byte that is not valid UTF-8 into U+FFFD and the round-trip documented on match breaks.

   One consequence of errors="surrogateescape" needs correcting before the patch merges, because the failure path is narrower than it looks. unquote("%FF%FE", errors="surrogateescape") returns a str holding lone surrogates in U+DC80 to U+DCFF, and that value does survive unquote and quote with the same setting. It does not fail at json.dumps(...).encode("utf-8"): with the default escaper the surrogate is emitted as the ASCII sequence \udcff, and encoding that succeeds. What fails is anything that encodes the string itself as UTF-8, so value.encode("utf-8"), a text mode file write with encoding="utf-8", or a body built from json.dumps(..., ensure_ascii=False) all raise UnicodeEncodeError: surrogates not allowed. os.fsencode is the one path that survives, which is why the surrogateescape family exists.

   So the invariant is not "use surrogateescape in both directions". It is that every consumer of the matched value has to be known, because the value can no longer be encoded as UTF-8. Two ways to keep the fix honest:

   1. Keep the value as bytes internally. unquote_to_bytes on match, quote_from_bytes on expand, neither leaves the byte domain. I checked the round trip on %FF%FE and it returns b"\xff\xfe" intact. That is what RFC 3986 section 2.1 describes, and the lossy decision happens once, at the boundary where a handler asks for text.

   2. If surrogateescape stays, put the actual serialization path in the test table rather than the default dumps call, since the default call is the one that passes.

   Either way the test that separates a real fix from a passing one is the same: percent-encode a byte that is illegal in UTF-8, expand, serialize it the way the transport will, and assert you get the original octet back. String equality alone passes for both designs and distinguishes neither.