You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
UriTemplate.match() misparses {;var} when following literal begins with '=' #3852
UriTemplate.match() breaks match(expand(variables)) == variables when a {;var} path-style parameter expression (ifemp = True, RFC 6570 §3.2.7) is immediately followed by a literal that begins with "=" (for example "{;a}=end{+rest}" or "/api{;mode}=fast{/id}").
In src/mcp/shared/uri_template.py (_scan_prefix), the atom.ifemp check runs:
Checking uri.startswith(nxt.text, pos) unconditionally before uri[pos] == "=" handles the empty-value case (a == "" -> ";a=end/x"), except when a equals the stem of nxt.text (a == "end" -> expand() emits ";a=end=end/x"). At pos right after ";a", uri[pos:] is "=end=end/x", which also starts with nxt.text ("=end"). _scan_prefix prematurely treats a as "" and leaves the second "=end" to be absorbed by a downstream greedy variable {+rest} (or fails suffix matching when no greedy variable follows).
I'd like to take this issue and ask to be assigned.
UriTemplate.match() currently misparses {;var} when the following literal starts with =, because the empty-value check in _scan_prefix can treat that literal as an empty path-style value. I plan to fix that check in src/mcp/shared/uri_template.py and add tests so match(expand(variables)) == variables holds for this case (including examples like {;a}=end{+rest}).
Please assign this issue to me. Once it is assigned, I will open a PR with Fixes #3852, as described in the contributing guide (external PRs are reviewed when assigned or labeled help wanted).
/api{;mode}=fast{/id} with mode=fast -> {'mode': 'fast', 'id': '7'}. Correct, and it is correct for slow too.
{;a}=end{+rest} with a=x -> correct.
{;a}=end/z, {;a}=end, {;a}=end{#f}, {;a}=end{?q} -> all correct.
So the second example in the report does not reproduce, and the reason is in _scan_prefix. After the premature empty capture, the next atom is a literal =fast which consumes the first =fast; the leftover =fast/7 then fails the /id literal, _scan_prefix returns None, and the caller retries with a different suffix split, which recovers the right parse. A reserved expansion absorbs whatever is left instead of failing, so no retry happens and the wrong capture survives. That makes the class: an ifemp capture followed by a literal, where the value equals the literal's stem and the remainder is absorbable downstream.
On the fix itself, reordering the two checks moves the bug rather than removing it. ;a=end=end/x genuinely has two readings: a empty followed by the literal =end, or a equal to end. Checking uri[pos] == '=' first breaks the case of an empty value followed by a literal that begins with =. What the scanner needs is ordered alternatives with a fallback: try the = reading, and if the rest of the atom sequence fails, retry that position with the empty-value reading. That keeps expand(match(x)) == x without needing a heuristic about which reading looks more likely.
A property test over a small template corpus would pin all of this better than individual cases: for each template and value set, assert match(expand(v)) == v, plus a round-trip on the string. The suite appears to cover the empty-value form of ; and the plain {;id} case, but not the stem collision, and the corpus would have caught it because the template and the value only collide for one value.
For what it is worth, this matters in practice for servers that build path-style templates around a mode or namespace segment, where the value and the literal after it can repeat.
Confirmed on main (77001c9). One nuance: ;a=end=end/x is genuinely ambiguous for {;a}=end{+rest}, since {a: "", rest: "=end/x"} expands to the same string, so the fix is really about consistency: the suffix scan already prefers ;name=value ({;a}=end and {;a}=end{/rest} both match a="end"), only _scan_prefix takes the empty reading first.
Approach I’d take: in _scan_prefix’s ifemp branch, when uri[pos] == "=", try the ;name=value reading first (bounded by the ; stop chars) and fall back to the empty check only if the next literal isn’t found. Prototype is ~15 lines in that one function and passes the existing 307 template tests. Happy to send it if you’d like an outside PR. (Investigated with AI assistance.)
Your consistency reading is right, and one measured detail narrows the fix further.
Taking the ;name=value reading first is necessary but not sufficient if the value scan uses the stop-char maximum span. On ;a=end=end/x that reading takes value end=end (for the ; operator the stop set is ;/?#, so = does not end the value), the following =end literal then fails, and you fall back to the empty reading, which is the same wrong parse as today.
The reading that round-trips is the minimum span to the following literal, the same bounded find() the non-ifemp branch already uses. In the ifemp branch, when uri[pos] == "=", do a bounded find of the next literal, capture up to it, and fall back to the empty reading only when that find misses. For ;a=end=end/x that yields a="end", rest="/x". For ;a=end/x the find misses, so the empty reading still wins and nothing regresses.
Measured with that change (about 12 lines, ifemp branch only):
{;a}=end{+rest} with a=end, rest=/x: expand ;a=end=end/x, match returns the same values. Before the change it returned {'a': '', 'rest': '=end/x'}.
{;a}=end{+rest} with a="" , rest=/x: unchanged, {'a': '', 'rest': '/x'}.
{;a}=end{/b} and {;mode}=fast{/id} and {;a}=end{#f}: unchanged, both the empty and the non-empty form.
tests/shared/test_uri_template.py: 307 passed.
Happy to send that as a PR if it saves ZYZ666-RGB time, and either way the corpus property test should carry the stem-collision values, since that combination is the one the current suite does not cover.
Thanks @media-ctrl, good catch. The stop-char span takes "end=end" and falls back to the wrong parse, so the bounded find to the next literal is the right fix. I'll step back and leave this to @ZYZ666-RGB / you. Happy to review the PR.
The change is in the ifemp branch of _scan_prefix: when uri[pos] == "=", try the ;name=value reading first, bounded to the next literal, then fall back to the empty reading. The bounded span is what carries the fix. The stop-char span takes end=end from ;a=end=end/x because = does not end a ; value, and the following =end literal then fails and drops back to the reading that is wrong today.
Three regression tests, each confirmed to fail on 77001c9 and pass with the patch. The suite that touches uri_template plus the shared and issues trees is 1047 passed, 1 skipped.
;a=end=end/x stays ambiguous by construction, since {a: "end", rest: "/x"} and {a: "", rest: "=end/x"} expand to the same string. The value reading wins, matching what the suffix scan already does for {;a}=end{/rest}, and a test names that trade so it cannot flip silently.
@ZYZ666-RGB if you already have a branch going, please say so and I will compare the two rather than leave a duplicate open.
Thanks @media-ctrl. I had an earlier fork branch, but it didn't use the bounded find to the next literal, so #3860 is the better fix. I'll close mine out rather than open a duplicate. Please go ahead with #3860.
State update, since the close happened before your message landed: #3860 was auto-closed at 10:09Z by the require-linked-issue workflow. This repo only keeps pull requests open from maintainers, or from a contributor a maintainer has assigned to the linked issue, and I am not assigned to #3852, so it closed itself. The patch is not rejected and nothing is wrong with it.
What unblocks it is assignment, not a new PR: a maintainer assigning either you (requested at 03:30Z) or me to #3852 reopens the PR automatically. If you get assigned first, carry the bounded find fix, since you already have the branch and @shlokbam offered to review.
For the record, the measured core stays the same: under {;a}=end{+rest} with ;a=end=end/x, the ;name=value reading has to be tried first and bounded to the next literal. The stop-char span reads end=end and falls back to the wrong parse.
Activity
ZYZ666-RGB commentedon Nov 13, 2026
I'd like to take this issue and ask to be assigned.
UriTemplate.match()currently misparses{;var}when the following literal starts with=, because the empty-value check in_scan_prefixcan treat that literal as an empty path-style value. I plan to fix that check insrc/mcp/shared/uri_template.pyand add tests somatch(expand(variables)) == variablesholds for this case (including examples like{;a}=end{+rest}).Please assign this issue to me. Once it is assigned, I will open a PR with
Fixes #3852, as described in the contributing guide (external PRs are reviewed when assigned or labeled help wanted).media-ctrl commentedon Nov 14, 2026
Reproduced at 77001c9, and the failing set is narrower than the report suggests, which helps whoever fixes it.
Measured on that tree, expanding and then matching:
{;a}=end{+rest}witha=end, rest=/x->;a=end=end/x->{'a': '', 'rest': '=end/x'}. Broken./api{;mode}=fast{/id}withmode=fast->{'mode': 'fast', 'id': '7'}. Correct, and it is correct forslowtoo.{;a}=end{+rest}witha=x-> correct.{;a}=end/z,{;a}=end,{;a}=end{#f},{;a}=end{?q}-> all correct.So the second example in the report does not reproduce, and the reason is in
_scan_prefix. After the premature empty capture, the next atom is a literal=fastwhich consumes the first=fast; the leftover =fast/7 then fails the /id literal,_scan_prefixreturns None, and the caller retries with a different suffix split, which recovers the right parse. A reserved expansion absorbs whatever is left instead of failing, so no retry happens and the wrong capture survives. That makes the class: anifempcapture followed by a literal, where the value equals the literal's stem and the remainder is absorbable downstream.On the fix itself, reordering the two checks moves the bug rather than removing it.
;a=end=end/xgenuinely has two readings:aempty followed by the literal=end, oraequal toend. Checkinguri[pos] == '='first breaks the case of an empty value followed by a literal that begins with=. What the scanner needs is ordered alternatives with a fallback: try the=reading, and if the rest of the atom sequence fails, retry that position with the empty-value reading. That keepsexpand(match(x)) == xwithout needing a heuristic about which reading looks more likely.A property test over a small template corpus would pin all of this better than individual cases: for each template and value set, assert
match(expand(v)) == v, plus a round-trip on the string. The suite appears to cover the empty-value form of;and the plain{;id}case, but not the stem collision, and the corpus would have caught it because the template and the value only collide for one value.For what it is worth, this matters in practice for servers that build path-style templates around a mode or namespace segment, where the value and the literal after it can repeat.
shlokbam commentedon Nov 14, 2026
Confirmed on main (77001c9). One nuance: ;a=end=end/x is genuinely ambiguous for {;a}=end{+rest}, since {a: "", rest: "=end/x"} expands to the same string, so the fix is really about consistency: the suffix scan already prefers ;name=value ({;a}=end and {;a}=end{/rest} both match a="end"), only _scan_prefix takes the empty reading first.
Approach I’d take: in _scan_prefix’s ifemp branch, when uri[pos] == "=", try the ;name=value reading first (bounded by the ; stop chars) and fall back to the empty check only if the next literal isn’t found. Prototype is ~15 lines in that one function and passes the existing 307 template tests. Happy to send it if you’d like an outside PR. (Investigated with AI assistance.)
media-ctrl commentedon Nov 14, 2026
Your consistency reading is right, and one measured detail narrows the fix further.
Taking the
;name=valuereading first is necessary but not sufficient if the value scan uses the stop-char maximum span. On;a=end=end/xthat reading takes valueend=end(for the;operator the stop set is;/?#, so=does not end the value), the following=endliteral then fails, and you fall back to the empty reading, which is the same wrong parse as today.The reading that round-trips is the minimum span to the following literal, the same bounded
find()the non-ifemp branch already uses. In the ifemp branch, whenuri[pos] == "=", do a bounded find of the next literal, capture up to it, and fall back to the empty reading only when that find misses. For;a=end=end/xthat yieldsa="end", rest="/x". For;a=end/xthe find misses, so the empty reading still wins and nothing regresses.Measured with that change (about 12 lines, ifemp branch only):
{;a}=end{+rest}witha=end, rest=/x: expand;a=end=end/x, match returns the same values. Before the change it returned{'a': '', 'rest': '=end/x'}.{;a}=end{+rest}witha="" , rest=/x: unchanged,{'a': '', 'rest': '/x'}.{;a}=end{/b}and{;mode}=fast{/id}and{;a}=end{#f}: unchanged, both the empty and the non-empty form.tests/shared/test_uri_template.py: 307 passed.Happy to send that as a PR if it saves ZYZ666-RGB time, and either way the corpus property test should carry the stem-collision values, since that combination is the one the current suite does not cover.
shlokbam commentedon Nov 14, 2026
Thanks @media-ctrl, good catch. The stop-char span takes "end=end" and falls back to the wrong parse, so the bounded find to the next literal is the right fix. I'll step back and leave this to @ZYZ666-RGB / you. Happy to review the PR.
media-ctrl commentedon Nov 14, 2026
Opened #3860 for this.
The change is in the
ifempbranch of_scan_prefix: whenuri[pos] == "=", try the;name=valuereading first, bounded to the next literal, then fall back to the empty reading. The bounded span is what carries the fix. The stop-char span takesend=endfrom;a=end=end/xbecause=does not end a;value, and the following=endliteral then fails and drops back to the reading that is wrong today.Three regression tests, each confirmed to fail on
77001c9and pass with the patch. The suite that touchesuri_templateplus the shared and issues trees is 1047 passed, 1 skipped.;a=end=end/xstays ambiguous by construction, since{a: "end", rest: "/x"}and{a: "", rest: "=end/x"}expand to the same string. The value reading wins, matching what the suffix scan already does for{;a}=end{/rest}, and a test names that trade so it cannot flip silently.@ZYZ666-RGB if you already have a branch going, please say so and I will compare the two rather than leave a duplicate open.
ZYZ666-RGB commentedon Nov 14, 2026
Thanks @media-ctrl. I had an earlier fork branch, but it didn't use the bounded find to the next literal, so #3860 is the better fix. I'll close mine out rather than open a duplicate. Please go ahead with #3860.
media-ctrl commentedon Nov 14, 2026
State update, since the close happened before your message landed: #3860 was auto-closed at 10:09Z by the require-linked-issue workflow. This repo only keeps pull requests open from maintainers, or from a contributor a maintainer has assigned to the linked issue, and I am not assigned to #3852, so it closed itself. The patch is not rejected and nothing is wrong with it.
What unblocks it is assignment, not a new PR: a maintainer assigning either you (requested at 03:30Z) or me to #3852 reopens the PR automatically. If you get assigned first, carry the bounded find fix, since you already have the branch and @shlokbam offered to review.
For the record, the measured core stays the same: under
{;a}=end{+rest}with;a=end=end/x, the;name=valuereading has to be tried first and bounded to the next literal. The stop-char span readsend=endand falls back to the wrong parse.