fix(web): reject a negative-zero literal, which loses its sign when serialized
Copilot's twelfth pass. The suppressed comment is right and is fixed here; the inline one is real too, but needs a core API change and is filed as #2171. `-0` is represented exactly, so the digit comparison had nothing to say about it — but `JSON.stringify` writes it as `0`, so the loss happens on the way *out* rather than on the way in. The editor showed `-0` and the wire carried `0`, and the viewer rewrote a received `-0` while pretty-printing. `hasImpreciseIntegerLiteral` becomes `findUnsendableNumberLiteral`, checking `Object.is(Number(literal), -0)` for every numeric spelling — `-0`, `-0.0`, `-0e1` — before the integer-only digit check. Returning the offending literal rather than a boolean lets the message name it, which is more use than either of the two rules behind it and matches how the duplicate-key message reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
1b28fc8218
commit
8aa6f01505
7 changed files with 85 additions and 58 deletions
|
|
@ -9,7 +9,7 @@
|
|||
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
||||
import {
|
||||
findDuplicateObjectKey,
|
||||
hasImpreciseIntegerLiteral,
|
||||
findUnsendableNumberLiteral,
|
||||
} from "../../../utils/jsonObjectDraft";
|
||||
|
||||
/**
|
||||
|
|
@ -169,7 +169,7 @@ export function formatJson(content: string): string {
|
|||
function reformatsFaithfully(text: string, parsed: unknown): boolean {
|
||||
return (
|
||||
isSerializableJson(parsed) &&
|
||||
!hasImpreciseIntegerLiteral(text) &&
|
||||
findUnsendableNumberLiteral(text) === null &&
|
||||
// A duplicate member name is the same class and the starkest instance:
|
||||
// `{"role":"user","role":"admin"}` would be *shown* as only the second,
|
||||
// which is the viewer hiding what the server sent.
|
||||
|
|
|
|||
|
|
@ -218,9 +218,7 @@ describe("EditReplayModal", () => {
|
|||
renderWithMantine(<EditReplayModal {...baseProps} method="tools/call" />);
|
||||
await setAceText('{"name":"echo","arguments":{"id":9007199254740993}}');
|
||||
expect(screen.getByRole("button", { name: "Send" })).toBeDisabled();
|
||||
expect(
|
||||
screen.getByText(/cannot be represented exactly/),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.getByText(/would not arrive as written/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("disables Send for a non-string prompt argument", async () => {
|
||||
|
|
|
|||
|
|
@ -5,9 +5,9 @@ import { JsonEditor } from "../../elements/JsonEditor/JsonEditor";
|
|||
import {
|
||||
duplicateKeyError,
|
||||
findDuplicateObjectKey,
|
||||
hasImpreciseIntegerLiteral,
|
||||
findUnsendableNumberLiteral,
|
||||
isJsonObject,
|
||||
IMPRECISE_INTEGER_ERROR,
|
||||
unsendableNumberError,
|
||||
} from "../../../utils/jsonObjectDraft";
|
||||
import {
|
||||
replayableParams,
|
||||
|
|
@ -113,8 +113,9 @@ function parseParamsDraft(
|
|||
"Numbers must be finite — a value like `1e400` overflows and would be sent as null",
|
||||
};
|
||||
}
|
||||
if (hasImpreciseIntegerLiteral(text)) {
|
||||
return { ok: false, error: IMPRECISE_INTEGER_ERROR };
|
||||
const unsendable = findUnsendableNumberLiteral(text);
|
||||
if (unsendable !== null) {
|
||||
return { ok: false, error: unsendableNumberError(unsendable) };
|
||||
}
|
||||
const duplicate = findDuplicateObjectKey(text);
|
||||
if (duplicate !== null) {
|
||||
|
|
|
|||
|
|
@ -789,7 +789,7 @@ describe("SchemaForm", () => {
|
|||
|
||||
await setAceTextByLabel(/Config/, "[9007199254740993]");
|
||||
expect(
|
||||
screen.getByText(/^This whole number cannot be represented exactly/),
|
||||
screen.getByText(/^`9007199254740993` would not arrive as written/),
|
||||
).toBeInTheDocument();
|
||||
|
||||
await setAceTextByLabel(/Config/, '[{"a":1,"a":2}]');
|
||||
|
|
@ -1976,9 +1976,7 @@ describe("SchemaForm raw JSON (#2151)", () => {
|
|||
|
||||
await setAceTextByLabel(/Arguments JSON/, '{"id":9007199254740993}');
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(true);
|
||||
expect(
|
||||
screen.getByText(/cannot be represented exactly/),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.getByText(/would not arrive as written/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// `JSON.parse` keeps only the last occurrence, so the editor would show a
|
||||
|
|
|
|||
|
|
@ -27,9 +27,9 @@ import { JsonEditor } from "../../elements/JsonEditor/JsonEditor";
|
|||
import {
|
||||
duplicateKeyError,
|
||||
findDuplicateObjectKey,
|
||||
hasImpreciseIntegerLiteral,
|
||||
findUnsendableNumberLiteral,
|
||||
isJsonObject,
|
||||
IMPRECISE_INTEGER_ERROR,
|
||||
unsendableNumberError,
|
||||
} from "../../../utils/jsonObjectDraft";
|
||||
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
||||
import { useValueChange } from "../../../hooks/useValueChange";
|
||||
|
|
@ -331,8 +331,9 @@ function describeJsonDraftProblem(text: string): string | null {
|
|||
if (!isSerializableJson(parsed)) {
|
||||
return "Numbers must be finite — this field will be omitted";
|
||||
}
|
||||
if (hasImpreciseIntegerLiteral(text)) {
|
||||
return `${IMPRECISE_INTEGER_ERROR} — this field will be omitted`;
|
||||
const unsendable = findUnsendableNumberLiteral(text);
|
||||
if (unsendable !== null) {
|
||||
return `${unsendableNumberError(unsendable)} — this field will be omitted`;
|
||||
}
|
||||
const duplicate = findDuplicateObjectKey(text);
|
||||
if (duplicate !== null) {
|
||||
|
|
@ -652,8 +653,9 @@ function parseRawArgumentsDraft(
|
|||
}
|
||||
// The quieter half of the same defect: a whole number past 2^53−1 parses to
|
||||
// the nearest double, so the draft shows digits the wire will not carry.
|
||||
if (hasImpreciseIntegerLiteral(text)) {
|
||||
return { ok: false, error: IMPRECISE_INTEGER_ERROR };
|
||||
const unsendable = findUnsendableNumberLiteral(text);
|
||||
if (unsendable !== null) {
|
||||
return { ok: false, error: unsendableNumberError(unsendable) };
|
||||
}
|
||||
const duplicate = findDuplicateObjectKey(text);
|
||||
if (duplicate !== null) {
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
findDuplicateObjectKey,
|
||||
hasImpreciseIntegerLiteral,
|
||||
findUnsendableNumberLiteral,
|
||||
parseJsonObjectDraft,
|
||||
} from "./jsonObjectDraft";
|
||||
|
||||
|
|
@ -9,65 +9,83 @@ import {
|
|||
// `Infinity` → `null` on the wire). This is the quieter one: a whole number
|
||||
// past 2^53−1 parses to the nearest double, so the draft shows digits the wire
|
||||
// will not carry.
|
||||
describe("hasImpreciseIntegerLiteral", () => {
|
||||
describe("findUnsendableNumberLiteral", () => {
|
||||
it("accepts whole numbers inside the safe range", () => {
|
||||
expect(hasImpreciseIntegerLiteral('{"id":9007199254740991}')).toBe(false);
|
||||
expect(hasImpreciseIntegerLiteral('{"id":-9007199254740991}')).toBe(false);
|
||||
expect(hasImpreciseIntegerLiteral('{"id":0}')).toBe(false);
|
||||
expect(findUnsendableNumberLiteral('{"id":9007199254740991}')).toBeNull();
|
||||
expect(findUnsendableNumberLiteral('{"id":-9007199254740991}')).toBeNull();
|
||||
expect(findUnsendableNumberLiteral('{"id":0}')).toBeNull();
|
||||
});
|
||||
|
||||
it("rejects a whole number that loses digits when parsed", () => {
|
||||
// Typed as …93, parsed as …92.
|
||||
expect(hasImpreciseIntegerLiteral('{"id":9007199254740993}')).toBe(true);
|
||||
expect(findUnsendableNumberLiteral('{"id":9007199254740993}')).toBe(
|
||||
"9007199254740993",
|
||||
);
|
||||
});
|
||||
|
||||
// The case the previous, value-based check could not see: at or above 1e21
|
||||
// JS stringifies in exponent form, so inferring the literal from the parsed
|
||||
// number said "exponent form, nothing lost" about digits that were lost.
|
||||
it("rejects a full-form literal at or above 1e21", () => {
|
||||
expect(hasImpreciseIntegerLiteral('{"id":1000000000000000000001}')).toBe(
|
||||
true,
|
||||
expect(findUnsendableNumberLiteral('{"id":1000000000000000000001}')).toBe(
|
||||
"1000000000000000000001",
|
||||
);
|
||||
});
|
||||
|
||||
// …and the false positive it also had. Written out in full, past the safe
|
||||
// range, and exactly representable, so it is sent as written.
|
||||
it("accepts a full-form literal that is exactly representable", () => {
|
||||
expect(hasImpreciseIntegerLiteral('{"id":18014398509481984}')).toBe(false);
|
||||
expect(findUnsendableNumberLiteral('{"id":18014398509481984}')).toBeNull();
|
||||
});
|
||||
|
||||
// A fractional or exponent-form value is a double by nature and is sent as
|
||||
// the double it parsed to, so nothing is lost between typing and sending.
|
||||
it("accepts fractional and exponent-form values", () => {
|
||||
expect(hasImpreciseIntegerLiteral('{"n":1.5}')).toBe(false);
|
||||
expect(hasImpreciseIntegerLiteral('{"n":1e308}')).toBe(false);
|
||||
expect(hasImpreciseIntegerLiteral('{"n":1e21}')).toBe(false);
|
||||
expect(hasImpreciseIntegerLiteral('{"n":-2.5e-7}')).toBe(false);
|
||||
expect(findUnsendableNumberLiteral('{"n":1.5}')).toBeNull();
|
||||
expect(findUnsendableNumberLiteral('{"n":1e308}')).toBeNull();
|
||||
expect(findUnsendableNumberLiteral('{"n":1e21}')).toBeNull();
|
||||
expect(findUnsendableNumberLiteral('{"n":-2.5e-7}')).toBeNull();
|
||||
});
|
||||
|
||||
// The reason this scans rather than running a regex over the text: those
|
||||
// digits are a *string*, sent back exactly as written.
|
||||
it("ignores digits inside a string", () => {
|
||||
expect(hasImpreciseIntegerLiteral('{"id":"9007199254740993"}')).toBe(false);
|
||||
expect(findUnsendableNumberLiteral('{"id":"9007199254740993"}')).toBeNull();
|
||||
});
|
||||
|
||||
it("ignores digits inside a key, and an escaped quote inside a string", () => {
|
||||
expect(hasImpreciseIntegerLiteral('{"9007199254740993":1}')).toBe(false);
|
||||
expect(findUnsendableNumberLiteral('{"9007199254740993":1}')).toBeNull();
|
||||
expect(
|
||||
hasImpreciseIntegerLiteral('{"a":"say \\"9007199254740993\\"","b":1}'),
|
||||
).toBe(false);
|
||||
findUnsendableNumberLiteral('{"a":"say \\"9007199254740993\\"","b":1}'),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("looks through arrays and nesting", () => {
|
||||
expect(hasImpreciseIntegerLiteral('[{"a":[9007199254740993]}]')).toBe(true);
|
||||
expect(hasImpreciseIntegerLiteral('{"a":{"b":[1,2,"x",null,true]}}')).toBe(
|
||||
false,
|
||||
expect(findUnsendableNumberLiteral('[{"a":[9007199254740993]}]')).toBe(
|
||||
"9007199254740993",
|
||||
);
|
||||
expect(
|
||||
findUnsendableNumberLiteral('{"a":{"b":[1,2,"x",null,true]}}'),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
// Represented exactly, so the digit comparison has nothing to say — but
|
||||
// `JSON.stringify` writes it as `0`, losing the sign on the way *out*.
|
||||
it.each(["-0", "-0.0", "-0e1", "-0.0e-5"])(
|
||||
"rejects negative zero written as %s",
|
||||
(literal) => {
|
||||
expect(findUnsendableNumberLiteral(`{"n":${literal}}`)).toBe(literal);
|
||||
},
|
||||
);
|
||||
|
||||
it("accepts a positive zero", () => {
|
||||
expect(findUnsendableNumberLiteral('{"n":0}')).toBeNull();
|
||||
expect(findUnsendableNumberLiteral('{"n":0.0}')).toBeNull();
|
||||
});
|
||||
|
||||
it("says nothing about text that is not JSON", () => {
|
||||
expect(hasImpreciseIntegerLiteral("")).toBe(false);
|
||||
expect(hasImpreciseIntegerLiteral("{")).toBe(false);
|
||||
expect(findUnsendableNumberLiteral("")).toBeNull();
|
||||
expect(findUnsendableNumberLiteral("{")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -21,8 +21,8 @@ export function isJsonObject(value: unknown): value is StrictJsonObject {
|
|||
}
|
||||
|
||||
/**
|
||||
* Whether the draft text writes a whole number out in full that `JSON.parse`
|
||||
* cannot represent exactly.
|
||||
* The first numeric literal in the draft that would not arrive as written, or
|
||||
* `null` when every number survives the trip.
|
||||
*
|
||||
* `isSerializableJson` catches a literal too large to represent at all —
|
||||
* `1e400` parses to `Infinity` and writes back as `null`. This catches the
|
||||
|
|
@ -38,19 +38,25 @@ export function isJsonObject(value: unknown): value is StrictJsonObject {
|
|||
* refused `2^54` (written in full and exactly representable) and accepted every
|
||||
* full-form literal at or above 1e21, where JS switches to exponent form.
|
||||
*
|
||||
* Only integer literals written out in full are checked. An exponent or a
|
||||
* fraction is a double by nature and is sent as the double it parsed to, so
|
||||
* nothing is lost between typing and sending — and `{"n":1e308}` stays
|
||||
* acceptable, which `parseJsonObjectDraft`'s own tests pin.
|
||||
* Only integer literals written out in full are checked for lost digits. An
|
||||
* exponent or a fraction is a double by nature and is sent as the double it
|
||||
* parsed to, so nothing is lost between typing and sending — and `{"n":1e308}`
|
||||
* stays acceptable, which `parseJsonObjectDraft`'s own tests pin. Negative zero
|
||||
* is the exception, and is checked in every spelling: there the loss happens on
|
||||
* the way *out*, not on the way in.
|
||||
*/
|
||||
export function hasImpreciseIntegerLiteral(text: string): boolean {
|
||||
export function findUnsendableNumberLiteral(text: string): string | null {
|
||||
for (const literal of jsonNumberLiterals(text)) {
|
||||
// Negative zero, in any spelling — `-0`, `-0.0`, `-0e1`. It is represented
|
||||
// exactly, so the digit comparison below has nothing to say about it, but
|
||||
// `JSON.stringify` writes it as `0`: the sign is lost on the way out.
|
||||
if (Object.is(Number(literal), -0)) return literal;
|
||||
if (!/^-?\d+$/.test(literal)) continue;
|
||||
// BigInt on both sides, so the comparison is exact: `Number(literal)` is an
|
||||
// integer-valued double, and converting it back shows what survived.
|
||||
if (BigInt(literal) !== BigInt(Number(literal))) return true;
|
||||
if (BigInt(literal) !== BigInt(Number(literal))) return literal;
|
||||
}
|
||||
return false;
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Matches one JSON number token, anchored where the scan currently stands. */
|
||||
|
|
@ -179,14 +185,17 @@ export function duplicateKeyError(key: string): string {
|
|||
}
|
||||
|
||||
/**
|
||||
* The message every draft parser uses for {@link hasImpreciseIntegerLiteral}.
|
||||
* The message every draft parser uses for
|
||||
* {@link findUnsendableNumberLiteral}.
|
||||
*
|
||||
* Phrased as "cannot be represented exactly" rather than naming the safe-integer
|
||||
* range, because the range is not the rule: a whole number past ±(2^53 − 1) that
|
||||
* *is* exactly representable — 2^54, say — round-trips and is accepted.
|
||||
* Names the literal rather than the rule, because the rule is two rules: a
|
||||
* whole number can lose digits on the way in, and a negative zero loses its
|
||||
* sign on the way out. Naming what is wrong with *this* document is more use
|
||||
* than either explanation, and it is how the duplicate-key message reads too.
|
||||
*/
|
||||
export const IMPRECISE_INTEGER_ERROR =
|
||||
"This whole number cannot be represented exactly — it is rounded when parsed, so a different value would be sent";
|
||||
export function unsendableNumberError(literal: string): string {
|
||||
return `\`${literal}\` would not arrive as written — it changes when serialized, so a different value would be sent`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a JSON-object editor's draft text.
|
||||
|
|
@ -228,8 +237,9 @@ export function parseJsonObjectDraft(text: string): JsonObjectDraft {
|
|||
"Numbers must be finite — a value like `1e400` overflows and would be sent as null",
|
||||
};
|
||||
}
|
||||
if (hasImpreciseIntegerLiteral(text)) {
|
||||
return { ok: false, error: IMPRECISE_INTEGER_ERROR };
|
||||
const unsendable = findUnsendableNumberLiteral(text);
|
||||
if (unsendable !== null) {
|
||||
return { ok: false, error: unsendableNumberError(unsendable) };
|
||||
}
|
||||
const duplicate = findDuplicateObjectKey(text);
|
||||
if (duplicate !== null) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue