diff --git a/clients/web/src/test/core/rootUnion.test.ts b/clients/web/src/test/core/rootUnion.test.ts index 384703ee..05e838cb 100644 --- a/clients/web/src/test/core/rootUnion.test.ts +++ b/clients/web/src/test/core/rootUnion.test.ts @@ -147,6 +147,24 @@ describe("resolveRootUnion", () => { expect(base.properties).toBeUndefined(); }); + it("leaves an allOf that adds names a restrictive additionalProperties forbids", () => { + // The root rejects `x` as an additional property; folding the member in + // would move it beside the keyword, where it reads as allowed. + const { base } = resolveRootUnion({ + type: "object", + additionalProperties: false, + allOf: [ + { + type: "object", + properties: { x: { type: "string" } }, + required: ["x"], + }, + ], + }); + expect(base.allOf).toHaveLength(1); + expect(base.properties).toBeUndefined(); + }); + it("merges allOf branches unconditionally", () => { const { base, branches } = resolveRootUnion({ type: "object", @@ -754,6 +772,31 @@ describe("resolveRootUnion", () => { expect(selectBranchIndex(objectPinned, { tag: { a: 2 } })).toBe(1); }); + it("does not read an inherited property as a supplied constant", () => { + // `constructor` is a legal field name; reading the inherited one would + // rule out the very branch that pins it. + const pinnedOnInherited = resolveRootUnion({ + type: "object", + anyOf: [ + { + type: "object", + properties: Object.fromEntries([ + ["constructor", { const: "a" }], + ["x", { type: "string" }], + ]), + }, + { + type: "object", + properties: Object.fromEntries([ + ["constructor", { const: "b" }], + ["y", { type: "string" }], + ]), + }, + ] as unknown[], + }).branches; + expect(selectBranchIndex(pinnedOnInherited, { x: "supplied" })).toBe(0); + }); + it("reports none when the values identify nothing", () => { expect(selectBranchIndex(branches, {})).toBeNull(); expect(selectBranchIndex(branches, { kind: "other" })).toBeNull(); diff --git a/core/json/rootUnion.ts b/core/json/rootUnion.ts index fbb8d92e..07b5786e 100644 --- a/core/json/rootUnion.ts +++ b/core/json/rootUnion.ts @@ -367,6 +367,46 @@ function hasDiscriminator( }); } +/** + * Whether a schema's `additionalProperties` rejects names its own `properties` + * does not list. + * + * A schema that constrains nothing is the equivalent of `true`, and that is not + * only the empty object: `{ title: "Extra value" }` is annotation and no more. + * Treating either as restrictive would decline a legal permissive schema. + */ +function restrictsAdditional(schema: RootUnionSchema): boolean { + const additional = schema.additionalProperties; + return ( + additional === false || + (typeof additional === "object" && + additional !== null && + Object.keys(additional).some( + (keyword) => !ANNOTATION_KEYWORDS.has(keyword), + )) + ); +} + +/** + * Whether folding a member into a base would make names the base forbids look + * allowed. + * + * `additionalProperties` constrains whatever its **sibling** `properties` does + * not name, so under a restrictive one the base rejects every field the member + * adds. Merging moves those fields beside the keyword, where they read as + * permitted — and a form built from that submits what the schema forbids. + */ +function addsForbiddenNames( + base: RootUnionSchema, + member: RootUnionSchema, +): boolean { + if (!restrictsAdditional(base)) return false; + const baseNames = propertiesOf(base) ?? {}; + return Object.keys(propertiesOf(member) ?? {}).some( + (name) => !Object.hasOwn(baseNames, name), + ); +} + /** * Every property name the schema's composition members declare, whether or not * the composition could be flattened. @@ -526,7 +566,8 @@ export function resolveRootUnion( if ( branch === null || !isFlattenable(member) || - conflictsWithBase(merged, branch) + conflictsWithBase(merged, branch) || + addsForbiddenNames(merged, branch) ) { return { base: schema as ResolvedSchema, branches: [] }; } @@ -562,25 +603,9 @@ export function resolveRootUnion( // branches add — the original schema admits none of them. Flattening moves // those fields *beside* the keyword, where they would read as allowed, so a // form built from it would submit what the schema forbids. - const additional = base.additionalProperties; - const restrictsAdditional = - additional === false || - // A schema that constrains nothing is the equivalent of `true`, and that is - // not only the empty object: `{ title: "Extra value" }` is annotation and - // no more. Declining on key count alone would recreate the empty form for a - // legal permissive schema, so what counts is whether an assertion is there. - (typeof additional === "object" && - additional !== null && - Object.keys(additional).some( - (keyword) => !ANNOTATION_KEYWORDS.has(keyword), - )); - const baseNames = propertiesOf(base) ?? {}; if ( - restrictsAdditional && branches.some((branch) => - Object.keys(propertiesOf(branch as RootUnionSchema) ?? {}).some( - (name) => !Object.hasOwn(baseNames, name), - ), + addsForbiddenNames(base, branch as RootUnionSchema), ) ) { return { base, branches: [] }; @@ -646,7 +671,11 @@ export function selectBranchIndex( // constant it did not supply is one this identification exists to *seed* — // requiring it would mean a deep link naming `kind` alone matched no branch // whenever the branches also pin, say, a `version`. - const supplied = pinned.filter(([name]) => values[name] !== undefined); + // `hasOwn`: a pinned field legally named `constructor` would otherwise read + // the inherited one as a supplied value and rule its own branch out. + const supplied = pinned.filter( + ([name]) => Object.hasOwn(values, name) && values[name] !== undefined, + ); if ( supplied.length > 0 && // Structural, not reference: a `const` may be an object or an array, and