Skip to content

fix(uri-template): pct-encode literals per RFC 6570 §3.1 - #2828

Open
LGUIUX wants to merge 3 commits into
modelcontextprotocol:mainfrom
LGUIUX:fix/uri-template-literal-encoding
Open

LGUIUX wants to merge 3 commits into
modelcontextprotocol:mainfrom
LGUIUX:fix/uri-template-literal-encoding

Conversation

@LGUIUX

@LGUIUX LGUIUX commented Sep 18, 2026

Copy link
Copy Markdown

UriTemplate copies literal runs verbatim — into expand() and into the pattern
match() builds. RFC 6570 §3.1 requires a literal outside the reserved/unreserved sets
(ucschar such as café, or a space) to be pct-encoded as UTF-8 when the template is
expanded, so expand() currently returns a string that is not a valid RFC 3986 URI:

new UriTemplate('café/{v}').expand({ v: 'value' });
// 'café/value'   (uritemplate-test "Literal Encoding" expects caf%C3%A9/value)

Why it matters

The resource is listed and then unreadable — for every URI a client can send.

resources/read resolves the requested URI through new URL() before matching
(packages/server/src/server/mcp.ts:533:555), and new URL() pct-encodes the path.
The pattern still holds the raw literal, so it cannot match the normalized URI. With a
template registered as file:///docs/café/{name}, both of these fail today:

resources/read file:///docs/caf%C3%A9/a.txt  ->  -32602 Resource not found: file:///docs/caf%C3%A9/a.txt
resources/read file:///docs/café/a.txt       ->  -32602 Resource not found: file:///docs/café/a.txt

The raw request fails too, because the server normalizes it before matching. Same for a
literal space (file:///my docs/{name}). Non-ASCII path segments are ordinary for
non-English servers — this is where I hit it.

The change

encodeLiteral() applies §3.1 to literal runs in both directions, so an expanded URI —
and the pct-encoded URI a new URL() round-trip produces — matches the template it came
from. Existing %XX triplets pass through unchanged, since encodeURI alone would turn
caf%C3%A9 into caf%25C3%25A9. Reserved and unreserved characters stay as written.

After the change both reads above succeed and return
uri: 'file:///docs/caf%C3%A9/a.txt'.

Tests

  • packages/core-internal/test/shared/uriTemplate.test.ts: expansion (including the
    uritemplate-test "Literal Encoding" vector), no double-encoding, reserved characters
    preserved, matching the encoded form, and an expand→match round-trip.
  • packages/server/test/server/resourceTemplateLiteralEncoding.test.ts: the end-to-end
    routing case, asserted for both the pct-encoded and the raw request.
  • Suites green: core-internal 1463, server 513, client 887. Lint, Prettier and
    typecheck clean on both touched packages. The new unit assertions and three of the
    four new server assertions fail on main.

Two follow-up commits come from reviewing the first myself.

encodeURI escapes [ and ], so an IPv6 host literal (http://[::1]:8080/docs/{name}) came out as
%5B::1%5D, which new URL() never produces. RFC 3986 reserves both for the host and
§3.1 leaves reserved characters alone, so they are restored after encodeURI — per gap
between %XX triplets, so an author-written %5B still passes through.

And the literals are now encoded once in the constructor rather than on every call:
re-encoding per call took match() from 1.28 to 2.35 µs on a two-variable template, and
doing it at construction brings it back to 1.31 µs.

Note on behavior

Through the server path nothing regresses — both request forms work after the change,
where both failed before. A direct uriTemplate.match() call with a raw (unencoded)
IRI now returns null instead of matching; that string is not a valid RFC 3986 URI and
is not what reaches a server, but I'm flagging it since UriTemplate is exported.

The Python SDK has the same defect and I reported it there:
modelcontextprotocol/python-sdk#3526.


Disclosure: found and written with Claude Code — it ran the official
uritemplate-test vectors against the
module, reduced the failure to this case, and drafted the patch and tests. I reviewed it
before opening and can walk through the change.

`UriTemplate` copied literal runs verbatim, both into `expand()` and into
the pattern `match()` builds. RFC 6570 §3.1 requires a literal outside the
reserved/unreserved sets — `ucschar` (`café`), a space — to be pct-encoded
as UTF-8 on expansion, so `expand()` returned a string that is not a valid
RFC 3986 URI.

The consequence was a resource that could be listed but never read:
`resources/read` resolves the requested URI through `new URL()`, which
pct-encodes it, and the raw literal in the pattern could not match that.
A template such as `file:///docs/café/{name}` answered `-32602 Resource
not found` for every URI a client could send, encoded or raw.

Encode literals in both directions, passing existing `%XX` triplets
through unchanged so an already-encoded literal is not encoded twice
(`encodeURI` alone would turn `caf%C3%A9` into `caf%25C3%25A9`).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LGUIUX
LGUIUX requested a review from a team as a code owner September 18, 2026 00:57
@changeset-bot

changeset-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 37d9164

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@modelcontextprotocol/core-internal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 18, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2828

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2828

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2828

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2828

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2828

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2828

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2828

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2828

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2828

commit: 37d9164

LGUIUX and others added 2 commits September 18, 2026 16:24
`encodeURI` escapes `[` and `]`, so the previous commit turned the host of
`http://[::1]:8080/docs/{name}` into `%5B::1%5D` — which `new URL()` never
produces, leaving that template unmatchable.

RFC 3986 reserves both for the host literal and RFC 6570 §3.1 leaves reserved
characters to the template author, so restore them after `encodeURI`. The
restore runs per gap between `%XX` triplets, so a `%5B` the author wrote
themselves still passes through untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Literals come from the template, so re-encoding them on every expand() and
match() repeated identical work: match() went from 1.28 to 2.35 µs per call
on a two-variable template. Encoding them once in the constructor brings it
back to 1.31 µs, and takes the encode call out of both hot paths.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant