Skip to content

Commit 1987340

Browse files
committed
fix: correct ; operator matching and expansion per RFC 6570
Three fixes to the path-style parameter operator: Non-explode {;id}: the regex used =? (optional equals), which let {;id} match ;identity=john by consuming 'id' as a prefix. Added a lookahead asserting the name ends at = or a delimiter. Explode {;keys*} matching: captured segments included the name= prefix (returning ['keys=a', 'keys=b'] instead of ['a', 'b']) and did not validate the parameter name (so ;admin=true matched). Now strips the prefix and rejects wrong names in post-processing. Explode {;keys*} expansion: emitted name= for empty items. RFC 3.2.7's ifemp rule says ; omits the = for empty values, so ['a', '', 'b'] now expands to ;keys=a;keys;keys=b. All three are covered by new round-trip tests including the empty-item edge case.
1 parent 674783f commit 1987340

File tree

2 files changed

+44
-5
lines changed

2 files changed

+44
-5
lines changed

src/mcp/shared/uri_template.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,12 @@ def _expand_expression(expr: _Expression, variables: Mapping[str, str | Sequence
185185
if var.explode:
186186
# Each item gets the operator's separator; named ops repeat the key.
187187
if spec.named:
188-
rendered.append(spec.separator.join(f"{var.name}={v}" for v in items))
188+
# RFC §3.2.7 ifemp: ; omits the = for empty values.
189+
rendered.append(
190+
spec.separator.join(
191+
var.name if (v == "" and expr.operator == ";") else f"{var.name}={v}" for v in items
192+
)
193+
)
189194
else:
190195
rendered.append(spec.separator.join(items))
191196
else:
@@ -394,14 +399,26 @@ def match(self, uri: str, *, max_uri_length: int = DEFAULT_MAX_URI_LENGTH) -> di
394399

395400
if var.explode:
396401
# Explode capture holds the whole run including separators,
397-
# e.g. "/a/b/c". Split, decode each segment, check each.
402+
# e.g. "/a/b/c" or ";keys=a;keys=b". Split, decode each
403+
# segment, check each.
398404
if not raw:
399405
result[var.name] = []
400406
continue
401407
segments: list[str] = []
408+
prefix = f"{var.name}="
402409
for seg in raw.split(spec.separator):
403410
if not seg: # leading separator produces an empty first item
404411
continue
412+
if spec.named:
413+
# Named explode emits name=value per item (or bare
414+
# name for ; with empty value). Validate the name
415+
# and strip the prefix before decoding.
416+
if seg.startswith(prefix):
417+
seg = seg[len(prefix) :]
418+
elif seg == var.name:
419+
seg = ""
420+
else:
421+
return None
405422
decoded = unquote(seg)
406423
if any(c in decoded for c in forbidden):
407424
return None
@@ -464,9 +481,15 @@ def _expression_pattern(expr: _Expression) -> str:
464481
# Non-greedy so a trailing literal can terminate the run.
465482
pieces.append(f"((?:{sep}{body})*?)")
466483
elif spec.named:
467-
# ;name=val or ?name=val — the = is optional for ; with empty value
468-
eq = "=?" if expr.operator == ";" else "="
469-
pieces.append(f"{lead}{re.escape(var.name)}{eq}({body})")
484+
name = re.escape(var.name)
485+
if expr.operator == ";":
486+
# RFC ifemp: ; emits bare name for empty values, so = is
487+
# optional. The lookahead asserts the name ends at = or a
488+
# delimiter, preventing {;id} from matching ;identity.
489+
pieces.append(f"{lead}{name}(?==|[;/?#]|$)=?({body})")
490+
else:
491+
# ? and & always emit name=, even for empty values.
492+
pieces.append(f"{lead}{name}=({body})")
470493
else:
471494
pieces.append(f"{lead}({body})")
472495

tests/shared/test_uri_template.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,8 @@ def test_frozen():
280280
("{/path*}", {"path": ["a", "b", "c"]}, "/a/b/c"),
281281
("{.labels*}", {"labels": ["x", "y"]}, ".x.y"),
282282
("{;keys*}", {"keys": ["a", "b"]}, ";keys=a;keys=b"),
283+
# RFC §3.2.7 ifemp: ; omits = for empty explode items
284+
("{;keys*}", {"keys": ["a", "", "b"]}, ";keys=a;keys;keys=b"),
283285
# Undefined variables omitted
284286
("{?q,page}", {"q": "x"}, "?q=x"),
285287
("{a}{b}", {"a": "x"}, "x"),
@@ -333,6 +335,10 @@ def test_expand_rejects_invalid_value_types(value: object):
333335
# Level 3: path-style param
334336
("item{;id}", "item;id=42", {"id": "42"}),
335337
("item{;id}", "item;id", {"id": ""}),
338+
# Explode: ; emits name=value per item, match strips the prefix
339+
("item{;keys*}", "item;keys=a;keys=b", {"keys": ["a", "b"]}),
340+
("item{;keys*}", "item;keys=a;keys;keys=b", {"keys": ["a", "", "b"]}),
341+
("item{;keys*}", "item", {"keys": []}),
336342
# Level 3: query
337343
("search{?q}", "search?q=hello", {"q": "hello"}),
338344
("search{?q}", "search?q=", {"q": ""}),
@@ -367,6 +373,12 @@ def test_match(template: str, uri: str, expected: dict[str, str | list[str]]):
367373
("/users/{id}/posts/{pid}", "/users/1/posts/2/extra"),
368374
# Repeated-slash literal with wrong slash count
369375
("///{a}////{b}////", "//x////y////"),
376+
# ; name boundary: {;id} must not match a longer parameter name
377+
("item{;id}", "item;identity=john"),
378+
("item{;id}", "item;ident"),
379+
# ; explode: wrong parameter name in any segment rejects the match
380+
("item{;keys*}", "item;admin=true"),
381+
("item{;keys*}", "item;keys=a;admin=true"),
370382
],
371383
)
372384
def test_match_no_match(template: str, uri: str):
@@ -482,6 +494,10 @@ def test_match_structural_integrity_per_explode_segment():
482494
("file{.ext}", {"ext": "txt"}),
483495
("/files{/path*}", {"path": ["a", "b", "c"]}),
484496
("{var}", {"var": "hello world"}),
497+
("item{;id}", {"id": "42"}),
498+
("item{;id}", {"id": ""}),
499+
("item{;keys*}", {"keys": ["a", "b", "c"]}),
500+
("item{;keys*}", {"keys": ["a", "", "b"]}),
485501
],
486502
)
487503
def test_roundtrip_expand_then_match(template: str, variables: dict[str, str | list[str]]):

0 commit comments

Comments
 (0)