diff --git a/docs/servers/uri-templates.md b/docs/servers/uri-templates.md index 1d6e13c0a0..f53cbc8b64 100644 --- a/docs/servers/uri-templates.md +++ b/docs/servers/uri-templates.md @@ -239,7 +239,8 @@ Three things are happening in the highlighted lines: template; `template.match(uri)` returns the extracted variables as a `dict`, or `None` if the URI doesn't fit. URL decoding happens inside `match()`; the decoded values are returned as-is without path-safety - validation. Values come out as strings: convert them yourself + validation. A URI with a percent-escape that isn't valid UTF-8 + (`%FF`) doesn't match. Values come out as strings: convert them yourself (`int(matched["id"])`, `Path(matched["path"])`). * **Apply the safety checks yourself.** The `..` and absolute-path checks `MCPServer` runs by default live in `mcp.shared.path_security`. diff --git a/src/mcp/shared/uri_template.py b/src/mcp/shared/uri_template.py index 20d6fa9c2e..226182d4a6 100644 --- a/src/mcp/shared/uri_template.py +++ b/src/mcp/shared/uri_template.py @@ -472,7 +472,9 @@ def match(self, uri: str, *, max_uri_length: int = DEFAULT_MAX_URI_LENGTH) -> di will not match across a literal ``/`` in the URI (the scan stops there), but a percent-encoded ``%2F`` that decodes to ``/`` is accepted as part of the value. Path-safety validation belongs at - a higher layer; see :mod:`mcp.shared.path_security`. + a higher layer; see :mod:`mcp.shared.path_security`. A URI whose + percent-escapes don't decode as UTF-8 (e.g. ``%FF``) does not + match. Example:: @@ -518,6 +520,14 @@ def match(self, uri: str, *, max_uri_length: int = DEFAULT_MAX_URI_LENGTH) -> di if len(uri) > max_uri_length: return None + try: + return self._match(uri) + except UnicodeDecodeError: + # A %XX escape that isn't valid UTF-8 would otherwise decode + # to U+FFFD and hand the handler a different value. + return None + + def _match(self, uri: str) -> dict[str, str | list[str]] | None: if self._query_variables: # Two-phase: scan matches the path, the query is split and # decoded manually. Query params may be partial, reordered, @@ -583,6 +593,10 @@ def __str__(self) -> str: return self.template +def _decode(value: str) -> str: + return unquote(value, errors="strict") + + def _parse_query(query: str) -> dict[str, str]: """Parse a query string into a name→value mapping. @@ -604,7 +618,7 @@ def _parse_query(query: str) -> dict[str, str]: for pair in query.split("&"): name, _, value = pair.partition("=") if name and name not in result: - result[name] = unquote(value) + result[name] = _decode(value) return result @@ -622,7 +636,7 @@ def _extract_greedy(var: Variable, raw: str) -> str | list[str] | None: if not var.explode: if any(c in stops for c in raw): return None - return unquote(raw) + return _decode(raw) sep = spec.separator if not raw: @@ -654,7 +668,7 @@ def _extract_greedy(var: Variable, raw: str) -> str | list[str] | None: seg = "" else: return None - segments.append(unquote(seg)) + segments.append(_decode(seg)) return segments @@ -1012,7 +1026,7 @@ def _scan_suffix( eq = uri.find("=", earliest, pos) if eq == -1: return None - result[var.name] = unquote(uri[eq + 1 : pos]) + result[var.name] = _decode(uri[eq + 1 : pos]) pos = eq i -= 1 continue @@ -1045,7 +1059,7 @@ def _scan_suffix( return None start = idx + len(prev.text) - result[var.name] = unquote(uri[start:pos]) + result[var.name] = _decode(uri[start:pos]) pos = start i -= 1 return result, pos @@ -1111,6 +1125,6 @@ def _scan_prefix( if end == -1: return None - result[var.name] = unquote(uri[pos:end]) + result[var.name] = _decode(uri[pos:end]) pos = end return result, pos diff --git a/tests/shared/test_uri_template.py b/tests/shared/test_uri_template.py index cddbb82b0b..32ee1bb4b0 100644 --- a/tests/shared/test_uri_template.py +++ b/tests/shared/test_uri_template.py @@ -764,6 +764,21 @@ def test_match_decodes_percent_encoding(): assert t.match("file://docs/hello%20world.txt") == {"name": "hello world.txt"} +@pytest.mark.parametrize( + ("template", "uri"), + [ + ("file://docs/{name}", "file://docs/%FF"), + ("file://docs/{+path}", "file://docs/a/%C3"), + ("/files{/path*}", "/files/a/%FFb"), + ("{;filter}", ";filter=%80"), + ("logs://{service}{?level}", "logs://api?level=%FE"), + ("{+path}/end{.ext}", "a/end.%FF"), + ], +) +def test_match_rejects_invalid_utf8_escapes(template: str, uri: str): + assert UriTemplate.parse(template).match(uri) is None + + def test_match_escapes_template_literals(): # Regression: previous impl didn't escape . in literals, making it # a regex wildcard. "fileXtxt" should NOT match "file.txt/{id}".