fix: reject non-finite numbers before they reach _meta
`StrictJsonValue` removed `undefined`, but the type cannot express the rest of
the gap: TypeScript has one `number`, and JSON has no encoding for `NaN` or
`±Infinity`, so `JSON.stringify` rewrites them to `null`.
This is reachable from ordinary input, not a careless caller.
`JSON.parse('{"a":1e400}')` returns `Infinity` on overflow — so text a user
types parses cleanly, satisfies the type, and reaches the server as `null`
while the editor still shows `1e400`.
`isSerializableJson` in `core/json` is the runtime half of `StrictJsonValue`,
applied at both boundaries that turn text into metadata:
- the editor's draft parser, which now reports "Numbers must be finite" inline
and withholds the value rather than emitting one it cannot send;
- the CLI's `--metadata` / `--tool-metadata` parser, which throws. Silently
sending `null` would be wrong, and so would falling back to the literal
string — neither is what was asked for.
Tests cover the validator directly (including that `JSON.stringify` really does
write `null`, which is the reason it matters), the editor parser, and the CLI
flag.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
129a435fd4
commit
7f5992081e
6 changed files with 156 additions and 0 deletions
|
|
@ -880,6 +880,37 @@ describe("Metadata Tests", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
["an overflowing literal", "n=1e400"],
|
||||
["a negative overflow", "n=-1e400"],
|
||||
["one nested in an object", 'o={"a":1e400}'],
|
||||
])("rejects %s rather than sending null", async (_label, pair) => {
|
||||
// `JSON.parse` accepts these and yields ±Infinity, which
|
||||
// `JSON.stringify` writes as `null` — so accepting the flag would
|
||||
// transmit a value the user did not ask for.
|
||||
const server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
tools: [createEchoTool()],
|
||||
});
|
||||
try {
|
||||
await server.start();
|
||||
const result = await runCli([
|
||||
server.url,
|
||||
"--cli",
|
||||
"--method",
|
||||
"tools/list",
|
||||
"--metadata",
|
||||
pair,
|
||||
"--transport",
|
||||
"http",
|
||||
]);
|
||||
expect(result.exitCode).not.toBe(0);
|
||||
expect(`${result.stderr}${result.stdout}`).toMatch(/finite/i);
|
||||
} finally {
|
||||
await server.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("should handle metadata parsing validation", async () => {
|
||||
const server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import {
|
|||
} from "@inspector/core/mcp/node/index.js";
|
||||
import type { JsonValue } from "@inspector/core/mcp/index.js";
|
||||
import type { StrictJsonValue } from "@inspector/core/json/jsonUtils.js";
|
||||
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
||||
import {
|
||||
canonicalUrlHost,
|
||||
isAllInterfacesHost,
|
||||
|
|
@ -492,9 +493,22 @@ function parseKeyValuePair(
|
|||
try {
|
||||
parsedValue = JSON.parse(val) as StrictJsonValue;
|
||||
} catch {
|
||||
// Not JSON at all — a bare word or an unquoted string. Sent as a string,
|
||||
// which is what the user plainly meant.
|
||||
parsedValue = val;
|
||||
}
|
||||
|
||||
// Valid JSON syntax is not the same as sendable JSON: `1e400` parses to
|
||||
// `Infinity`, which `JSON.stringify` writes as `null`. Rejecting is better
|
||||
// than accepting the flag and silently transmitting a different value —
|
||||
// and better than falling back to the literal string, which would also not
|
||||
// be what was asked for.
|
||||
if (!isSerializableJson(parsedValue)) {
|
||||
throw new Error(
|
||||
`Invalid parameter value: ${value}. Numbers must be finite (a value like 1e400 overflows and cannot be sent).`,
|
||||
);
|
||||
}
|
||||
|
||||
return { ...previous, [key as string]: parsedValue };
|
||||
}
|
||||
|
||||
|
|
|
|||
47
clients/web/src/test/core/json/isSerializableJson.test.ts
Normal file
47
clients/web/src/test/core/json/isSerializableJson.test.ts
Normal file
|
|
@ -0,0 +1,47 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
||||
|
||||
describe("isSerializableJson", () => {
|
||||
it.each([
|
||||
["a string", "a"],
|
||||
["a finite number", 1.5],
|
||||
["zero", 0],
|
||||
["a negative number", -42],
|
||||
["a boolean", true],
|
||||
["null", null],
|
||||
["a nested object", { a: { b: [1, 2, { c: "d" }] } }],
|
||||
["an empty array", []],
|
||||
])("accepts %s", (_label, value) => {
|
||||
expect(isSerializableJson(value)).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["NaN", NaN],
|
||||
["Infinity", Infinity],
|
||||
["-Infinity", -Infinity],
|
||||
])("rejects %s, which JSON.stringify rewrites to null", (_label, value) => {
|
||||
expect(isSerializableJson(value)).toBe(false);
|
||||
// The reason it matters, asserted rather than asserted-about.
|
||||
expect(JSON.stringify({ v: value })).toBe('{"v":null}');
|
||||
});
|
||||
|
||||
it.each([
|
||||
["undefined", undefined],
|
||||
["a function", () => {}],
|
||||
["a symbol", Symbol("s")],
|
||||
["a bigint", 10n],
|
||||
])("rejects %s, which is not JSON at all", (_label, value) => {
|
||||
expect(isSerializableJson(value)).toBe(false);
|
||||
});
|
||||
|
||||
it("descends into arrays and objects", () => {
|
||||
expect(isSerializableJson({ a: [1, { b: Infinity }] })).toBe(false);
|
||||
expect(isSerializableJson([[[NaN]]])).toBe(false);
|
||||
expect(isSerializableJson({ a: [1, { b: 2 }] })).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects what `JSON.parse` produces for an overflowing literal", () => {
|
||||
// The reachable path: valid JSON text that parses to an unsendable value.
|
||||
expect(isSerializableJson(JSON.parse('{"n":1e400}'))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -24,6 +24,25 @@ describe("parseJsonObjectDraft", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
["an overflowing literal", '{"n":1e400}'],
|
||||
["a negative overflow", '{"n":-1e400}'],
|
||||
["one nested in an array", '{"a":[1,1e400]}'],
|
||||
])("rejects %s, which would be sent as null", (_label, text) => {
|
||||
// `JSON.parse` accepts these; `JSON.stringify` then writes `null`, so the
|
||||
// editor would show one value and the wire would carry another.
|
||||
const result = parseJsonObjectDraft(text);
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.ok === false && result.error).toMatch(/finite/);
|
||||
});
|
||||
|
||||
it("still accepts large-but-finite numbers", () => {
|
||||
expect(parseJsonObjectDraft('{"n":1e308}')).toEqual({
|
||||
ok: true,
|
||||
value: { n: 1e308 },
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects text that is not JSON", () => {
|
||||
expect(parseJsonObjectDraft('{"a":')).toEqual({
|
||||
ok: false,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import type { StrictJsonObject } from "@inspector/core/json/jsonUtils.js";
|
||||
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
||||
|
||||
/**
|
||||
* What a JSON-object editor's draft text currently means: the object to emit,
|
||||
|
|
@ -49,5 +50,15 @@ export function parseJsonObjectDraft(text: string): JsonObjectDraft {
|
|||
error: "Must be a JSON object (`{ … }`) — changes are not applied",
|
||||
};
|
||||
}
|
||||
// `JSON.parse` accepts numeric literals it cannot represent: `1e400` parses
|
||||
// to `Infinity`, which `JSON.stringify` then writes as `null`. Accepting it
|
||||
// here would show the user one value and send another.
|
||||
if (!isSerializableJson(parsed)) {
|
||||
return {
|
||||
ok: false,
|
||||
error:
|
||||
"Numbers must be finite — a value like `1e400` overflows and would be sent as null",
|
||||
};
|
||||
}
|
||||
return { ok: true, value: parsed };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -37,6 +37,40 @@ export type StrictJsonValue =
|
|||
|
||||
export type StrictJsonObject = { [key: string]: StrictJsonValue };
|
||||
|
||||
/**
|
||||
* Whether a value is JSON that survives `JSON.stringify` unchanged.
|
||||
*
|
||||
* {@link StrictJsonValue} is the *type*-level half of this and cannot express
|
||||
* the rest: TypeScript has one `number`, but JSON has no encoding for `NaN` or
|
||||
* `±Infinity`, so `JSON.stringify` rewrites them to `null`.
|
||||
*
|
||||
* That is reachable from ordinary input, not just from a careless caller —
|
||||
* `JSON.parse("1e400")` returns `Infinity` on overflow, so text a user typed
|
||||
* parses successfully, satisfies the type, and then reaches the wire as `null`
|
||||
* while the editor still shows what they wrote. Checking at the boundary is the
|
||||
* only place the distinction exists.
|
||||
*/
|
||||
export function isSerializableJson(value: unknown): value is StrictJsonValue {
|
||||
if (value === null) return true;
|
||||
switch (typeof value) {
|
||||
case "string":
|
||||
case "boolean":
|
||||
return true;
|
||||
case "number":
|
||||
// Rejects NaN and ±Infinity; every other number round-trips.
|
||||
return Number.isFinite(value);
|
||||
case "object":
|
||||
return Array.isArray(value)
|
||||
? value.every(isSerializableJson)
|
||||
: Object.values(value as Record<string, unknown>).every(
|
||||
isSerializableJson,
|
||||
);
|
||||
default:
|
||||
// `undefined`, functions, symbols and bigints are not JSON at all.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Widen a typed object to a generic string-keyed record so its keys can be
|
||||
* iterated or read/written generically. Many of the project's config/SDK types
|
||||
|
|
|
|||
Loading…
Reference in a new issue