modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix: keep a pct-encoded UTF-8 sequence whole, and bound the preview too

Browse source 
Addresses Copilot's round-16 review on #2035. Two suppressed comments, both
real, and both refinements of fixes from the previous two rounds rather than
defects in the original code.

1. Round 11 settled on "a pct-encoded triplet counts as one character", but
   a multi-octet character spans several triplets -- `%C3%A9` is one `é`.
   Measured on a3dd8563:

     x{+v:1}  v = "%C3%A9x"  =>  "x%C3"   a lone lead byte, decoding to nothing

   `prefixUnits` now recognizes a well-formed pct-encoded UTF-8 sequence from
   its lead byte (RFC 3629: C2-DF two octets, E0-EF three, F0-F4 four;
   continuations 80-BF), longest alternative first so a four-octet sequence
   is not read as a two-octet one followed by loose triplets.

   A malformed sequence -- a lead byte with too few continuations, or a stray
   continuation -- matches no sequence alternative and falls back to
   per-triplet counting. That is the conservative direction: it counts as
   MORE units, so truncation keeps less rather than emitting octets that
   were never in the value.

2. Round 14's per-value ceiling only covered the read. The panel calls
   `previewUriTemplate` on every keystroke, and that path expands expression
   by expression, so it reached the allocation-heavy encoders with the same
   oversized value -- submission refused while the UI froze on the paste
   anyway. Both paths now share one exported `valueLengthError`, checked
   before any encoding happens.

npm run ci passes.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-17 01:13:17 -04:00
commit 7d9410cbe0
1 changed files with 102 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

58
core/mcp/uriTemplate.ts
View file

@ -512,6 +512,27 @@ function encodeValue(value: string, operator: string): string {
: encodeUnreserved(value);
}
/**
* One unit a prefix modifier counts: a code point, a pct-encoded octet, or a
* whole pct-encoded UTF-8 *sequence*.
*
* The sequence alternatives are what stop a multi-octet character being cut in
* half. `%C3%A9` is one character — `é` — so `{+v:1}` over `%C3%A9x` must keep
* both octets; counting per triplet returned `%C3`, a lone lead byte that
* decodes to nothing. The lead byte announces the length (RFC 3629: `C2-DF`
* two octets, `E0-EF` three, `F0-F4` four) and each continuation is `80-BF`,
* so a well-formed sequence is recognizable without decoding.
*
* Order matters: longest first, so a four-octet sequence is not matched as a
* two-octet one followed by loose triplets. A malformed sequence — a lead byte
* with too few continuations, or a stray continuation — matches no sequence
* alternative and falls through to the single-triplet rule, which is the
* conservative behavior: it counts as more units, so truncation keeps less
* rather than emitting something that was never in the value.
*/
const PREFIX_UNIT =
/%F[0-4](?:%[89AB][0-9A-F]){3}|%E[0-9A-F](?:%[89AB][0-9A-F]){2}|%[CD][0-9A-F]%[89AB][0-9A-F]|%[0-9A-F]{2}|[\s\S]/giu;
/**
* Splits a value into the units a prefix modifier counts.
*
@ -532,12 +553,12 @@ function encodeValue(value: string, operator: string): string {
* still escapes the `%`, so the retained `%61` goes out as `%2561`), not what
* counts as one.
*
* The `u` flag makes the alternation match by code point, so an astral
* The `u` flag makes the final alternative match by code point, so an astral
* character is one unit rather than a surrogate pair that truncation could
* split.
*/
function prefixUnits(value: string): string[] {
return value.match(/%[0-9A-Fa-f]{2}|[\s\S]/gu) ?? [];
return value.match(PREFIX_UNIT) ?? [];
}
/**
@ -687,17 +708,8 @@ export function expandUriTemplateStrict(
);
}
// Replacing `UriTemplate.expand` also dropped its per-value ceiling, and
// nothing else bounded what reaches the allocation-heavy encoders below: a
// pasted or programmatically supplied megabyte-plus value hit them unguarded.
// Kept at the SDK's own limit so this is the guard it was, not a new policy.
for (const [name, value] of Object.entries(values)) {
if (value.length > MAX_VALUE_LENGTH) {
throw new Error(
`The value for "${name}" exceeds the ${MAX_VALUE_LENGTH}-character limit.`,
);
}
}
const tooLong = valueLengthError(values);
if (tooLong) throw new Error(tooLong);
const bad = parts.find((part) => part.kind === "expression" && part.invalid);
if (bad) {
@ -723,6 +735,26 @@ export function expandUriTemplateStrict(
*/
const MAX_VALUE_LENGTH = 1_000_000;
/**
* The first value exceeding {@link MAX_VALUE_LENGTH}, as a message, or `null`.
*
* Exported because the *preview* needs the same verdict and cannot get it from
* {@link expandUriTemplateStrict}: it expands expression by expression, so it
* would otherwise hand an oversized value straight to the encoders during
* render — the guard would hold for the read while the UI still froze on the
* paste that triggered it.
*/
export function valueLengthError(
values: Record<string, string>,
): string | null {
for (const [name, value] of Object.entries(values)) {
if (value.length > MAX_VALUE_LENGTH) {
return `The value for "${name}" exceeds the ${MAX_VALUE_LENGTH}-character limit.`;
}
}
return null;
}
/**
* The outcome of an expansion: the URI, or the reason there isn't one.
*