FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(navigation): decode percent-encoded deep-link parameters by Adebowale-Morakinyo · Pull Request #106 · pythonnative/pythonnative · GitHub

Repository navigation

fix(navigation): decode percent-encoded deep-link parameters - #106

Open
Adebowale-Morakinyo wants to merge 1 commit into
pythonnative:mainfrom
Adebowale-Morakinyo:fix/navigation-decode-deep-link-params
Open

Adebowale-Morakinyo wants to merge 1 commit into
pythonnative:mainfrom
Adebowale-Morakinyo:fix/navigation-decode-deep-link-params

Conversation

Copy link
Copy Markdown
Contributor

What

LinkingConfig.state_from_url() now percent-decodes captured path params, once per segment,
after the path is split and before parse converters run. url_from_state() already encoded
them; the read side never decoded. Before this fix, myapp://u/Ada%20Lovelace produced
{"user": "Ada%20Lovelace"}.

Closes #89.

Why captured values only

Decoding every segment before matching looks equivalent but isn't. It never produced a false
match — a decoded %2F is still one segment, and literals are split on / when the config is
built, so none can contain one. But it changes routing in two ways: an encoded literal starts
matching its plain form, so u/%6Eew would reach a literal u/new screen instead of u/:user,
and a configured literal that contains an escape (a%2Fb) stops matching. Decoding only captured
values leaves literal matching exactly as it was, which the acceptance criteria require.
test_linking_literal_segments_are_not_decoded pins this.

Order matters too: decoding the whole path before splitting turns a%2Fb into two segments.

Why strict decoding with a raw fallback

unquote defaults to errors="replace", which turns invalid UTF-8 — %FF, a Latin-1 caf%E9 —
into U+FFFD. That destroys information that survives today, and it cascades: with
parse={"id": int}, item/%FF would become '�', and the converter's except would silently
keep it.

Strict decoding with a fallback to the raw segment is non-regressive by construction: every valid
encoding gets fixed, and any undecodable value stays exactly as it is now. Malformed escapes like
%ZZ or a trailing % pass through unchanged under every mode. Happy to switch to plain
replace if you'd prefer.

Heads-up for apps that worked around the bug

An app that called unquote on its own params will now decode twice — %2541 would reach it as
%41 and then become A. Nothing in this repo does that; the only callers are
navigation/container.py:149 and :177, which consume the returned state. App code outside the
repo may, so the workaround should be removed on upgrade.

Testing

New tests cover the five acceptance values as round trips, %2F staying in one value, %252F
decoding once, %34%32 converting to 42, + staying literal in a path while becoming a space
in the query of the same URL, undecodable values reading back raw, multiple captures in one route
and across a nested navigator, and literal matching unchanged. Existing linking tests pass
unmodified.

Mutation-checked against eight alternative implementations — no decoding, decoding before the
split, decoding twice, errors="replace", decoding literals too, unquote_plus, decoding only
the last capture, and catching Exception instead of UnicodeDecodeError. Each is caught.

Worth knowing for anyone extending the suite: round trips alone don't catch double decoding or
unquote_plus, because the generator never emits a bare + or a decodable %. The hand-written
%252F and + tests exist for exactly that reason.

An independent review fuzzed 4,016 values — ASCII, Unicode, /, %, +, ?, #, &, =,
spaces, NUL, and other control characters — through single-param, multi-param, and nested routes:
8,032 round trips, zero mismatches. Query behavior was identical to main.

./scripts/check.sh passes; mkdocs build --strict passes from a clean site/.

Flagged, not fixed

Empty param values don't round-trip, and in the middle of a path they misroute: url_from_state
emits an empty segment and _split_path drops it. Independent of decoding and identical on
main; filed as #105.

url_from_state percent-encoded captured path values but state_from_url
copied them verbatim, so myapp://u/Ada%20Lovelace read back as
{"user": "Ada%20Lovelace"}. Captured values are now decoded once per
segment, after the path is split and before parse converters run.

Only captured values are decoded. Decoding every segment would change
routing -- u/%6Eew would reach a literal u/new route instead of u/:user
-- and would break configured literals that contain an escape.

Invalid UTF-8 falls back to the raw segment rather than using unquote's
default errors="replace". Every valid encoding is fixed and every
undecodable value stays exactly as it is today, so the change cannot
lose information that currently survives.

Apps that called unquote on these params to work around the bug will
now decode twice and should drop the workaround.

Closes pythonnative#89

This branch has not been deployed

No deployments
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decode percent-encoded path parameters when reading deep links

1 participant


Back | FazBrowse Home | New Git URL