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

fix(navigation): decode percent-encoded deep-link parameters · pythonnative/pythonnative@9ef6cb9 · GitHub

Repository navigation

Commit 9ef6cb9

Browse files
fix(navigation): decode percent-encoded deep-link parameters
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 #89
1 parent ac7b61c commit 9ef6cb9

2 files changed

Lines changed: 170 additions & 3 deletions

File tree

‎src/pythonnative/navigation/linking.py‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,19 @@
2121
and the query string supplies the rest) or a dict with ``path``,
2222
``parse`` (per-param converters), and ``screens`` for a nested
2323
navigator.
24+
25+
Captured path params arrive percent-decoded, once, before any ``parse``
26+
converter runs, so ``u/Ada%20Lovelace`` gives ``"Ada Lovelace"`` and an
27+
encoded ``%2F`` stays inside one value. A ``+`` in a path is a literal
28+
plus, unlike in the query string, where it means a space. Bytes that
29+
don't decode as UTF-8 are left as they arrived. Literal segments are
30+
compared exactly as written.
2431
"""
2532

2633
from __future__ import annotations
2734

2835
from typing import Any, Callable, Dict, List, Mapping, Optional, Sequence, Tuple, Union
29-
from urllib.parse import parse_qsl, quote, urlencode, urlsplit
36+
from urllib.parse import parse_qsl, quote, unquote, urlencode, urlsplit
3037

3138
from .state import NavigationState, Route
3239

@@ -56,7 +63,7 @@ def match(self, parts: Sequence[str]) -> Optional[Dict[str, Any]]:
5663
params: Dict[str, Any] = {}
5764
for pattern, actual in zip(self.segments, parts):
5865
if pattern.startswith(":"):
59-
params[pattern[1:]] = actual
66+
params[pattern[1:]] = _decode_segment(actual)
6067
elif pattern != actual:
6168
return None
6269
return params
@@ -65,6 +72,11 @@ def match(self, parts: Sequence[str]) -> Optional[Dict[str, Any]]:
6572
class LinkingConfig:
6673
"""URL <-> navigation state mapping for a navigator tree.
6774
75+
Captured path params are percent-decoded once, after the path is
76+
split into segments and before ``parse`` converters run; ``+`` in a
77+
path stays a literal plus, and undecodable bytes are left as-is.
78+
Literal segments match exactly as configured.
79+
6880
Args:
6981
prefixes: URL prefixes this app answers to (schemes such as
7082
``"myapp://"`` or web origins). Matching is case-insensitive
@@ -185,6 +197,30 @@ def url_from_state(self, state: NavigationState) -> Optional[str]:
185197
return f"{url}?{query}" if query else url
186198

187199

200+
def _decode_segment(segment: str) -> str:
201+
"""Percent-decode one captured path segment, or return it unchanged.
202+
203+
Strict decoding with a fallback to the raw segment is deliberately
204+
non-regressive: every valid encoding is fixed, and a segment that
205+
isn't valid UTF-8 (``%FF``, Latin-1 ``caf%E9``) stays exactly what it
206+
was before decoding existed. ``errors="replace"`` would instead turn
207+
such a segment into U+FFFD, destroying information that survives
208+
today. It uses ``unquote`` rather than ``unquote_plus`` because a
209+
``+`` is a literal character in a path.
210+
211+
Args:
212+
segment: One path segment, already split on ``/``, so an encoded
213+
``%2F`` decodes to a slash inside the value.
214+
215+
Returns:
216+
The decoded segment, or ``segment`` itself if it isn't valid UTF-8.
217+
"""
218+
try:
219+
return unquote(segment, errors="strict")
220+
except UnicodeDecodeError:
221+
return segment
222+
223+
188224
def _split_path(path: Optional[str]) -> Tuple[str, ...]:
189225
if not path:
190226
return ()

‎tests/test_navigation.py‎

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""Tests for the navigation package: state, core, navigators, hooks, linking."""
22

3-
from typing import Any, Dict, List, NotRequired, TypedDict
3+
from typing import Any, Dict, List, NotRequired, Optional, TypedDict
44

55
import pytest
66

@@ -29,6 +29,7 @@
2929
use_navigation,
3030
use_route,
3131
)
32+
from pythonnative.navigation import linking as navigation_linking
3233
from pythonnative.testing import FakeHost, render, render_hook
3334

3435
# ======================================================================
@@ -1164,6 +1165,136 @@ def test_linking_url_from_state() -> None:
11641165
assert cfg.url_from_state(NavigationState([Route("Nowhere")])) is None
11651166

11661167

1168+
def _profile_linking() -> LinkingConfig:
1169+
return LinkingConfig(prefixes=["myapp://"], screens={"Profile": "u/:user"})
1170+
1171+
1172+
def _leaf(state: Optional[NavigationState]) -> Route:
1173+
assert state is not None
1174+
route = state.current
1175+
while route.state is not None:
1176+
route = route.state.current
1177+
return route
1178+
1179+
1180+
@pytest.mark.parametrize(
1181+
"value",
1182+
[
1183+
pytest.param("Ada Lovelace", id="space"),
1184+
pytest.param("Zo\u00eb \u03a9mega \u65e5\u672c", id="unicode"),
1185+
pytest.param("c++", id="literal-plus"),
1186+
pytest.param("100%", id="percent-sign"),
1187+
pytest.param("a/b", id="slash"),
1188+
],
1189+
)
1190+
def test_linking_path_param_round_trips(value: str) -> None:
1191+
cfg = _profile_linking()
1192+
url = cfg.url_from_state(NavigationState([Route("Profile", {"user": value})]))
1193+
assert url is not None
1194+
1195+
assert _leaf(cfg.state_from_url(url)).params == {"user": value}
1196+
1197+
1198+
def test_linking_every_capture_is_decoded() -> None:
1199+
# Two captured segments, each encoded differently, so a decode applied
1200+
# to only one of them (say, the last) leaves the other raw.
1201+
cfg = LinkingConfig(prefixes=["myapp://"], screens={"Thread": "org/:org/repo/:repo"})
1202+
params = {"org": "Ada Lovelace", "repo": "a/b"}
1203+
url = cfg.url_from_state(NavigationState([Route("Thread", params)]))
1204+
assert url == "myapp://org/Ada%20Lovelace/repo/a%2Fb"
1205+
1206+
route = _leaf(cfg.state_from_url(url))
1207+
assert route.name == "Thread"
1208+
assert route.params == params
1209+
1210+
1211+
def test_linking_captures_are_decoded_across_a_nested_navigator() -> None:
1212+
# One capture belongs to the outer navigator's path and one to the
1213+
# inner screen's; both land on the leaf and both must be decoded.
1214+
cfg = LinkingConfig(
1215+
prefixes=["myapp://"],
1216+
screens={"Org": {"path": "org/:org", "screens": {"Repo": "repo/:repo"}}},
1217+
)
1218+
params = {"org": "Zo\u00eb", "repo": "c++"}
1219+
state = NavigationState([Route("Org", state=NavigationState([Route("Repo", params)]))])
1220+
url = cfg.url_from_state(state)
1221+
assert url == "myapp://org/Zo%C3%AB/repo/c%2B%2B"
1222+
1223+
read = cfg.state_from_url(url)
1224+
assert read is not None and read.current.name == "Org"
1225+
route = _leaf(read)
1226+
assert route.name == "Repo"
1227+
assert route.params == params
1228+
1229+
1230+
def test_linking_encoded_slash_stays_in_one_value() -> None:
1231+
# Decoding happens per segment, after the path is split, so %2F can't
1232+
# become a segment boundary and shift the match.
1233+
route = _leaf(_profile_linking().state_from_url("myapp://u/a%2Fb"))
1234+
assert route.name == "Profile"
1235+
assert route.params == {"user": "a/b"}
1236+
1237+
1238+
def test_linking_double_encoded_value_decodes_once() -> None:
1239+
assert _leaf(_profile_linking().state_from_url("myapp://u/%252F")).params == {"user": "%2F"}
1240+
1241+
1242+
def test_linking_converter_sees_the_decoded_value() -> None:
1243+
route = _leaf(_linking().state_from_url("myapp://item/%34%32"))
1244+
assert route.name == "Detail"
1245+
assert route.params == {"id": 42}
1246+
1247+
1248+
def test_linking_plus_is_literal_in_the_path_and_a_space_in_the_query() -> None:
1249+
cfg = _profile_linking()
1250+
route = _leaf(cfg.state_from_url("myapp://u/a+b?q=a+b"))
1251+
assert route.params == {"user": "a+b", "q": "a b"}
1252+
1253+
1254+
@pytest.mark.parametrize(
1255+
"raw",
1256+
[pytest.param("%FF", id="invalid-utf8"), pytest.param("caf%E9", id="latin-1")],
1257+
)
1258+
def test_linking_undecodable_path_param_is_left_raw(raw: str) -> None:
1259+
# Strict decoding with a raw fallback: a value that isn't valid UTF-8
1260+
# reads back exactly as it did before decoding existed, rather than
1261+
# being replaced with U+FFFD.
1262+
assert _leaf(_profile_linking().state_from_url(f"myapp://u/{raw}")).params == {"user": raw}
1263+
1264+
1265+
def test_linking_failing_converter_keeps_the_raw_undecodable_value() -> None:
1266+
# int() fails and the converter's except leaves the value in place, so
1267+
# what's left must be the original "%FF", not an unrecoverable "\ufffd".
1268+
route = _leaf(_linking().state_from_url("myapp://item/%FF"))
1269+
assert route.params == {"id": "%FF"}
1270+
1271+
1272+
def test_linking_decode_fallback_only_swallows_unicode_errors(monkeypatch: pytest.MonkeyPatch) -> None:
1273+
# The raw-segment fallback is for undecodable bytes only. Anything else
1274+
# going wrong inside the decode is a bug and must surface, not quietly
1275+
# hand the screen an undecoded value.
1276+
def _boom(*args: Any, **kwargs: Any) -> str:
1277+
raise RuntimeError("not a decoding problem")
1278+
1279+
monkeypatch.setattr(navigation_linking, "unquote", _boom)
1280+
1281+
with pytest.raises(RuntimeError, match="not a decoding problem"):
1282+
_profile_linking().state_from_url("myapp://u/ada")
1283+
1284+
1285+
def test_linking_literal_segments_are_not_decoded() -> None:
1286+
# Only captured values are decoded. An encoded "new" still falls through
1287+
# to the :param route, exactly as before, rather than matching the
1288+
# literal route. Decoding every segment would route this to New instead.
1289+
cfg = LinkingConfig(prefixes=["myapp://"], screens={"New": "u/new", "Profile": "u/:user"})
1290+
1291+
encoded = _leaf(cfg.state_from_url("myapp://u/%6Eew"))
1292+
assert encoded.name == "Profile"
1293+
assert encoded.params == {"user": "new"}
1294+
1295+
assert _leaf(cfg.state_from_url("myapp://u/new")).name == "New"
1296+
1297+
11671298
def test_container_seeds_from_launch_url_and_follows_later_links(monkeypatch: pytest.MonkeyPatch) -> None:
11681299
monkeypatch.setattr(linking_module, "_initial_url", "myapp://item/5")
11691300
monkeypatch.setattr(linking_module, "_url_listeners", [])

0 commit comments

Comments
 (0)

Back | FazBrowse Home | New Git URL