diff --git a/.changeset/known-entity-prototype-chain.md b/.changeset/known-entity-prototype-chain.md new file mode 100644 index 0000000000..55f7e4a601 --- /dev/null +++ b/.changeset/known-entity-prototype-chain.md @@ -0,0 +1,27 @@ +--- +'@ifc-lite/parser': patch +'@ifc-lite/codegen': patch +--- + +Stop the generated schema registry answering for `Object.prototype` members. + +`SCHEMA_REGISTRY.entities` is a plain object literal, so `in` and `obj[key]` +both reach the prototype chain. `getEntityMetadata('constructor')` returned +the `Object` constructor. Two exported guards were wrong as a result: + +- `isInstantiable('constructor')` was `true`. Its own docblock says it exists + to stop authoring code writing an abstract class into an exported file. +- `normalizeIfcTypeName` returned the string `"Object"` for `constructor`, and + `undefined` for `__proto__` from a signature declaring `string`. + +`isKnownType('constructor')` was already `false` and is unchanged. It is worth +naming, because the guard that reads as looser was the one answering correctly, +and the guard documented as the strict authoring boundary was the one letting +it through. + +`isKnownEntity` had the same defect and now delegates to `getEntityMetadata` +rather than repeating the lookup. + +The same generator emits a second registry with the same defect, also fixed: +`getTypeId('constructor')` returned the `Object` constructor from a signature +declaring `number | undefined`. diff --git a/packages/codegen/generated/ifc4/schema-registry.ts b/packages/codegen/generated/ifc4/schema-registry.ts index 823997eae3..fab0267e8d 100644 --- a/packages/codegen/generated/ifc4/schema-registry.ts +++ b/packages/codegen/generated/ifc4/schema-registry.ts @@ -63136,6 +63136,7 @@ export const SCHEMA_REGISTRY: SchemaRegistry = { export function getEntityMetadata(typeName: string): EntityMetadata | undefined { // Normalize to IfcXxx format const normalized = normalizeTypeName(typeName); + if (!Object.prototype.hasOwnProperty.call(SCHEMA_REGISTRY.entities, normalized)) return undefined; return SCHEMA_REGISTRY.entities[normalized]; } @@ -63159,8 +63160,7 @@ export function getInheritanceChainForEntity(typeName: string): string[] { * Check if a type is a known entity */ export function isKnownEntity(typeName: string): boolean { - const normalized = normalizeTypeName(typeName); - return normalized in SCHEMA_REGISTRY.entities; + return getEntityMetadata(typeName) !== undefined; } /** diff --git a/packages/codegen/generated/ifc4/type-ids.ts b/packages/codegen/generated/ifc4/type-ids.ts index cc6eeda339..638224183b 100644 --- a/packages/codegen/generated/ifc4/type-ids.ts +++ b/packages/codegen/generated/ifc4/type-ids.ts @@ -1586,6 +1586,7 @@ export const TYPE_NAMES: Record = { export function getTypeId(name: string): number | undefined { // Normalize to IfcXxx format const normalized = normalizeTypeName(name); + if (!Object.prototype.hasOwnProperty.call(TYPE_IDS, normalized)) return undefined; return (TYPE_IDS as Record)[normalized]; } diff --git a/packages/codegen/generated/ifc4x3/schema-registry.ts b/packages/codegen/generated/ifc4x3/schema-registry.ts index 3d98f71108..583a762f8a 100644 --- a/packages/codegen/generated/ifc4x3/schema-registry.ts +++ b/packages/codegen/generated/ifc4x3/schema-registry.ts @@ -71439,6 +71439,7 @@ export const SCHEMA_REGISTRY: SchemaRegistry = { export function getEntityMetadata(typeName: string): EntityMetadata | undefined { // Normalize to IfcXxx format const normalized = normalizeTypeName(typeName); + if (!Object.prototype.hasOwnProperty.call(SCHEMA_REGISTRY.entities, normalized)) return undefined; return SCHEMA_REGISTRY.entities[normalized]; } @@ -71462,8 +71463,7 @@ export function getInheritanceChainForEntity(typeName: string): string[] { * Check if a type is a known entity */ export function isKnownEntity(typeName: string): boolean { - const normalized = normalizeTypeName(typeName); - return normalized in SCHEMA_REGISTRY.entities; + return getEntityMetadata(typeName) !== undefined; } /** diff --git a/packages/codegen/generated/ifc4x3/type-ids.ts b/packages/codegen/generated/ifc4x3/type-ids.ts index 9a0f9759cf..afc12bf1f3 100644 --- a/packages/codegen/generated/ifc4x3/type-ids.ts +++ b/packages/codegen/generated/ifc4x3/type-ids.ts @@ -1786,6 +1786,7 @@ export const TYPE_NAMES: Record = { export function getTypeId(name: string): number | undefined { // Normalize to IfcXxx format const normalized = normalizeTypeName(name); + if (!Object.prototype.hasOwnProperty.call(TYPE_IDS, normalized)) return undefined; return (TYPE_IDS as Record)[normalized]; } diff --git a/packages/codegen/src/type-ids-generator.ts b/packages/codegen/src/type-ids-generator.ts index a364cc59e4..4470f00992 100644 --- a/packages/codegen/src/type-ids-generator.ts +++ b/packages/codegen/src/type-ids-generator.ts @@ -66,6 +66,7 @@ export const TYPE_NAMES: Record = { export function getTypeId(name: string): number | undefined { // Normalize to IfcXxx format const normalized = normalizeTypeName(name); + if (!Object.prototype.hasOwnProperty.call(TYPE_IDS, normalized)) return undefined; return (TYPE_IDS as Record)[normalized]; } diff --git a/packages/codegen/src/typescript-generator.ts b/packages/codegen/src/typescript-generator.ts index ad8f21bda1..d126660f44 100644 --- a/packages/codegen/src/typescript-generator.ts +++ b/packages/codegen/src/typescript-generator.ts @@ -461,6 +461,15 @@ export const SCHEMA_REGISTRY: SchemaRegistry = { code += ` ${select.name}: [${escapedTypes.join(', ')}],\n`; } + // Everything from here to the end of this template literal is EMITTED TEXT, + // not code that runs here. Two consequences that have both bitten: + // + // - A backtick anywhere in it closes the literal and the rest becomes real + // TypeScript. That is a `tsc` failure on this package, and the only thing + // that catches it, so never write one in a comment below. + // - The registry is a plain object literal, so `in` and `obj[key]` both + // walk the prototype chain and answer for `constructor`, `toString` and + // `__proto__`. Every lookup below must be an own-property check (#3063). code += ` }, }; @@ -470,6 +479,7 @@ export const SCHEMA_REGISTRY: SchemaRegistry = { export function getEntityMetadata(typeName: string): EntityMetadata | undefined { // Normalize to IfcXxx format const normalized = normalizeTypeName(typeName); + if (!Object.prototype.hasOwnProperty.call(SCHEMA_REGISTRY.entities, normalized)) return undefined; return SCHEMA_REGISTRY.entities[normalized]; } @@ -493,8 +503,7 @@ export function getInheritanceChainForEntity(typeName: string): string[] { * Check if a type is a known entity */ export function isKnownEntity(typeName: string): boolean { - const normalized = normalizeTypeName(typeName); - return normalized in SCHEMA_REGISTRY.entities; + return getEntityMetadata(typeName) !== undefined; } /** diff --git a/packages/codegen/test/typescript-generator-mapping.test.ts b/packages/codegen/test/typescript-generator-mapping.test.ts index 2ec71f5783..bc3d28dbcc 100644 --- a/packages/codegen/test/typescript-generator-mapping.test.ts +++ b/packages/codegen/test/typescript-generator-mapping.test.ts @@ -300,6 +300,25 @@ describe('emitted SCHEMA_REGISTRY runtime helpers (executed, not substring-match expect(mod.isKnownEntity('IFCNOTATHING')).toBe(false); }); + // The registry is a plain object literal, so `in` and `obj[key]` both reach + // Object.prototype. Every lookup the generator emits has to be an + // own-property check, and getEntityMetadata is the one that matters: it + // feeds isInstantiable and normalizeIfcTypeName in @ifc-lite/parser, so + // before #3063 `isInstantiable('constructor')` was true and + // `normalizeIfcTypeName('constructor')` was the string "Object". + // + // `__proto__` is the case that is not like the others: it resolves to an + // object rather than a function, and its `.name` is undefined, which made + // normalizeIfcTypeName return undefined from a signature promising string. + it.each(['constructor', 'toString', 'hasOwnProperty', '__proto__'])( + 'does not treat the inherited property %s as an entity', + async (name) => { + const mod = await evalEmitted(code.schemaRegistry); + expect(mod.isKnownEntity(name)).toBe(false); + expect(mod.getEntityMetadata(name)).toBeUndefined(); + } + ); + it('carries isAbstract through to the runtime metadata, both values', async () => { const mod = await evalEmitted(code.schemaRegistry); expect(mod.getEntityMetadata('IfcRoot')?.isAbstract).toBe(true); diff --git a/packages/parser/src/generated/schema-registry.ts b/packages/parser/src/generated/schema-registry.ts index 823997eae3..fab0267e8d 100644 --- a/packages/parser/src/generated/schema-registry.ts +++ b/packages/parser/src/generated/schema-registry.ts @@ -63136,6 +63136,7 @@ export const SCHEMA_REGISTRY: SchemaRegistry = { export function getEntityMetadata(typeName: string): EntityMetadata | undefined { // Normalize to IfcXxx format const normalized = normalizeTypeName(typeName); + if (!Object.prototype.hasOwnProperty.call(SCHEMA_REGISTRY.entities, normalized)) return undefined; return SCHEMA_REGISTRY.entities[normalized]; } @@ -63159,8 +63160,7 @@ export function getInheritanceChainForEntity(typeName: string): string[] { * Check if a type is a known entity */ export function isKnownEntity(typeName: string): boolean { - const normalized = normalizeTypeName(typeName); - return normalized in SCHEMA_REGISTRY.entities; + return getEntityMetadata(typeName) !== undefined; } /** diff --git a/packages/parser/src/generated/type-ids.ts b/packages/parser/src/generated/type-ids.ts index cc6eeda339..638224183b 100644 --- a/packages/parser/src/generated/type-ids.ts +++ b/packages/parser/src/generated/type-ids.ts @@ -1586,6 +1586,7 @@ export const TYPE_NAMES: Record = { export function getTypeId(name: string): number | undefined { // Normalize to IfcXxx format const normalized = normalizeTypeName(name); + if (!Object.prototype.hasOwnProperty.call(TYPE_IDS, normalized)) return undefined; return (TYPE_IDS as Record)[normalized]; } diff --git a/packages/parser/src/ifc-schema.prototype-guard.test.ts b/packages/parser/src/ifc-schema.prototype-guard.test.ts new file mode 100644 index 0000000000..8a91d0039d --- /dev/null +++ b/packages/parser/src/ifc-schema.prototype-guard.test.ts @@ -0,0 +1,85 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +import { describe, it, expect } from 'vitest'; +import { isKnownType, isInstantiable, normalizeIfcTypeName } from './ifc-schema.js'; +import { getTypeId } from './generated/type-ids.js'; + +/** + * #3063. The generated SCHEMA_REGISTRY is a plain object literal, so `in` and + * `obj[key]` both reach Object.prototype. + * + * This file pins the DAMAGE, not the mechanism. The mechanism is pinned in + * packages/codegen/test/typescript-generator-mapping.test.ts, against the text + * the generator emits, which is the only copy that cannot drift. What that + * cannot see is which of this package's exported guards the defect reached, + * and those are the ones callers actually hold: + * + * isKnownType('constructor') false (was already false) + * isInstantiable('constructor') TRUE <- the authoring guard + * normalizeIfcTypeName('constructor') "Object" + * normalizeIfcTypeName('__proto__') undefined, from a `: string` signature + * + * isInstantiable is the one that matters. Its own docblock says it exists so + * authoring code cannot write an abstract class into an exported file, and it + * was the weaker of the two: it said yes to `constructor` while isKnownType, + * the guard that reads as looser, correctly said no. + */ +describe('schema guards reject inherited Object.prototype names', () => { + // `__proto__` is deliberately in this list and is not like the others: it + // resolves to an object rather than a function, so its `.name` is undefined + // rather than a string. It is the case that turned a wrong answer into a + // type lie. + const inherited = ['constructor', 'toString', 'hasOwnProperty', '__proto__']; + + it.each(inherited)('isInstantiable(%s) is false', (name) => { + expect(isInstantiable(name)).toBe(false); + }); + + it.each(inherited)('isKnownType(%s) is false', (name) => { + expect(isKnownType(name)).toBe(false); + }); + + it.each(inherited)('normalizeIfcTypeName(%s) returns the name unchanged', (name) => { + // Unknown names are preserved as-is, because a vendor extension is not an + // error. The failure being pinned is returning something ELSE: "Object" + // for `constructor`, or undefined for `__proto__`. + const result = normalizeIfcTypeName(name); + expect(typeof result).toBe('string'); + expect(result).toBe(name); + }); + + // The same generator emits a second registry, and it had the same defect. + // The comment added to typescript-generator.ts says every lookup below it + // must be an own-property check, which was true of that file and read as + // covering the generator, so this is the sibling that claim would have hidden. + it.each(inherited)('getTypeId(%s) is undefined, not a function', (name) => { + const id = getTypeId(name); + // `number | undefined` is the declared return. Before the fix this handed + // back the Object constructor for `constructor` and Object.prototype for + // `__proto__`, so asserting `undefined` alone would pass for the wrong + // reason if the signature were ever loosened. Assert the type too. + expect(id).toBeUndefined(); + expect(typeof id).not.toBe('function'); + }); + + it('still resolves a real type id', () => { + expect(typeof getTypeId('IfcWall')).toBe('number'); + }); + + // Without these the suite is satisfied by making every guard return false, + // which would be a worse bug than the one being fixed. + it('still answers for real entities', () => { + expect(isKnownType('IfcWall')).toBe(true); + expect(isInstantiable('IfcWall')).toBe(true); + expect(normalizeIfcTypeName('IFCWALL')).toBe('IfcWall'); + }); + + it('still reports an abstract class as known but not instantiable', () => { + // The distinction isInstantiable exists to draw. If the fix had broken it, + // the inherited-name cases above would still pass. + expect(isKnownType('IfcRoot')).toBe(true); + expect(isInstantiable('IfcRoot')).toBe(false); + }); +});