diff --git a/.changeset/picklocalized-own-props-3907.md b/.changeset/picklocalized-own-props-3907.md new file mode 100644 index 000000000..3a5385847 --- /dev/null +++ b/.changeset/picklocalized-own-props-3907.md @@ -0,0 +1,11 @@ +--- +'@object-ui/i18n': patch +--- + +`pickLocalized` reads own properties only, and takes only string values, on every limb + +The resolver read four of its six limbs — the exact tag, the base language, `default` and `en` — with a bare bracket access. Bare access walks the prototype chain, so a locale that happened to name an `Object.prototype` member resolved to that member and the function stringified it into the label: `pickLocalized({ en: 'Pricing' }, 'constructor')` returned `function Object() { [native code] }`, and the same held for `toString`, `valueOf`, `hasOwnProperty`, `isPrototypeOf`, `propertyIsEnumerable` and `toLocaleString`. Those same four limbs also skipped the `typeof === 'string'` filter the regional and last-resort limbs already applied, so a non-string value short-circuited the chain and rendered as `[object Object]`. + +Both guards now apply uniformly. A guarded limb **misses** rather than aborting the resolution, so an unusable entry falls through to the next limb exactly as an absent one does — `pickLocalized({ en: 'Pricing' }, 'constructor')` is now `'Pricing'` (the `en` limb), and only a map with no usable entry at all resolves to `''`. An empty-string value is still a hit, because `''` is a label the author wrote. + +No real language tag can observe this: no BCP-47 tag is an `Object.prototype` member, and the inline locale map is declared `z.record(, z.string())`, so every in-contract input resolves byte-identically to before. What it changes is agreement with the backend twin `resolveI18nLabel` (objectstack#6765), which shipped with exactly these two narrowings recorded as deliberate departures from this function because on a server the locale can arrive in an `Accept-Language` header. That recorded rule divergence is now zero; the only remaining difference is how each side spells a miss (`''` here for a text node, `undefined` there for a producer's fallback chain), which is pinned as an identity in the cross-resolver parity table. diff --git a/packages/i18n/src/__tests__/pickLocalized.test.ts b/packages/i18n/src/__tests__/pickLocalized.test.ts index b6d1d0524..a4e0c8973 100644 --- a/packages/i18n/src/__tests__/pickLocalized.test.ts +++ b/packages/i18n/src/__tests__/pickLocalized.test.ts @@ -28,3 +28,188 @@ describe('pickLocalized', () => { expect(pickLocalized({ en: 'E' }, undefined)).toBe('E'); }); }); + +/** + * objectui#3907 — every limb reads OWN properties only, and accepts only + * `string` values. + * + * Both halves used to hold on limbs 3 and 6 alone, so the other four limbs + * (exact tag, base language, `default`, `en`) could resolve against + * `Object.prototype` and could short-circuit the chain with a non-string. They + * are the two departures the backend twin `resolveI18nLabel` (objectstack#6765, + * `packages/spec/src/ui/i18n-label-resolver.ts`) documented as deliberate + * narrowings of this function; landing them here collapses that recorded + * divergence to zero. + * + * ## The shape of the fix is "the limb misses", NOT "the call returns `''`" + * + * A locale naming an `Object.prototype` member stops being a HIT on that limb — + * it does not abort the resolution. The `??` chain therefore continues to the + * next limb exactly as it would for any other unknown tag, so + * `pickLocalized({ en: 'Pricing' }, 'constructor')` is `'Pricing'` (limb 5), not + * `''`. `''` is only correct when NO limb hits, which needs a map with no usable + * entry at all. Both are pinned below, because a test that demanded `''` for the + * first case would be pinning a fallthrough bug rather than this fix. + */ +describe('pickLocalized — own properties only (objectui#3907)', () => { + /** Every `Object.prototype` member the card enumerates. */ + const PROTOTYPE_MEMBERS = [ + 'constructor', + 'toString', + 'valueOf', + 'hasOwnProperty', + 'isPrototypeOf', + 'propertyIsEnumerable', + 'toLocaleString', + ] as const; + + it.each(PROTOTYPE_MEMBERS)( + 'a locale named %s misses every limb instead of resolving up the prototype chain', + (member) => { + // Before the fix this returned the member's source text, e.g. + // 'function Object() { [native code] }', as the rendered label. + expect(pickLocalized({ en: 'Pricing' }, member)).toBe('Pricing'); + }, + ); + + it.each(PROTOTYPE_MEMBERS)( + 'a locale named %s never leaks a function body into the label', + (member) => { + // Stated once over the whole set rather than per member: an + // implementation that resolved a DIFFERENT prototype member would satisfy + // no assertion above but would also fail none of them. + expect(pickLocalized({ en: 'Pricing' }, member)).not.toContain('native code'); + expect(pickLocalized({ ja: '価格' }, member)).not.toContain('native code'); + }, + ); + + it('the prototype limb is a miss, so the chain continues to the next limb', () => { + // Limb 5 (`en`) is what answers above. With no `en`, the last-resort limb + // answers; with nothing usable at all, and only then, the answer is `''`. + expect(pickLocalized({ ja: '価格' }, 'constructor')).toBe('価格'); + expect(pickLocalized({ default: 'D' }, 'toString')).toBe('D'); + expect(pickLocalized({}, 'constructor')).toBe(''); + }); + + it('own keys that happen to be named like prototype members still resolve', () => { + // The guard is `hasOwnProperty`, not a denylist of names: an author who + // really wrote such a key gets it back. No BCP-47 tag looks like this, but + // a denylist-shaped fix would answer differently here and that difference + // is worth pinning. + expect(pickLocalized({ constructor: 'Ctor', en: 'Pricing' }, 'constructor')).toBe('Ctor'); + expect(pickLocalized({ toString: 'Str', en: 'Pricing' }, 'toString')).toBe('Str'); + }); + + it('a map with a null prototype resolves identically to a plain object', () => { + const nullProto = Object.assign(Object.create(null), { en: 'Pricing' }) as Record; + expect(pickLocalized(nullProto, 'constructor')).toBe('Pricing'); + expect(pickLocalized(nullProto, 'en')).toBe('Pricing'); + }); +}); + +describe('pickLocalized — only `string` values are eligible, on every limb (objectui#3907)', () => { + // Off-spec input: the framework's `InlineLocaleMapSchema` is + // `z.record(, z.string())`, so nothing in contract can hold these. The + // renderer must still not paint '[object Object]' onto a screen — a + // non-string value is treated as absent and the limb misses. + + it('limb 1 (exact tag): a non-string value falls through instead of stringifying', () => { + expect(pickLocalized({ 'zh-CN': { nested: 'x' }, en: 'Pricing' }, 'zh-CN')).toBe('Pricing'); + }); + + it('limb 2 (base language): a non-string value falls through', () => { + expect(pickLocalized({ zh: { nested: 'x' }, en: 'Pricing' }, 'zh-CN')).toBe('Pricing'); + }); + + it('limb 4 (`default`): a non-string value falls through', () => { + expect(pickLocalized({ default: { nested: 'x' }, ja: '価格' }, 'fr')).toBe('価格'); + }); + + it('limb 5 (`en`): a non-string value falls through', () => { + expect(pickLocalized({ en: { nested: 'x' }, ja: '価格' }, 'fr')).toBe('価格'); + }); + + it('never renders the stringified object on any limb', () => { + const offSpec = [ + [{ 'zh-CN': { n: 1 }, en: 'Pricing' }, 'zh-CN'], + [{ zh: { n: 1 }, en: 'Pricing' }, 'zh-CN'], + [{ default: { n: 1 }, ja: '価格' }, 'fr'], + [{ en: { n: 1 }, ja: '価格' }, 'fr'], + [{ 'zh-CN': [1, 2], en: 'Pricing' }, 'zh-CN'], + [{ en: { n: 1 } }, 'fr'], + ] as const; + for (const [map, locale] of offSpec) { + expect(pickLocalized(map, locale)).not.toBe('[object Object]'); + } + }); + + it('an off-spec value on every limb at once is a miss, not a stringified object', () => { + expect(pickLocalized({ en: { n: 1 }, ja: { n: 2 } }, 'fr')).toBe(''); + }); + + it('an empty-string value is still a hit — `""` is a label the author wrote', () => { + // The boundary the string filter must not cross: `''` is a string, so the + // limb hits and the chain stops. Pinned because a truthiness-based filter + // would pass every test above and break exactly this. + expect(pickLocalized({ en: '', 'zh-CN': '定价' }, 'en')).toBe(''); + expect(pickLocalized({ en: '' }, 'fr')).toBe(''); + expect(pickLocalized({ 'zh-CN': '' }, 'zh')).toBe(''); + }); + + it('numeric and boolean VALUES are not labels (they were stringified before)', () => { + // Distinct from a scalar passed as the whole `value`, which is still + // stringified by design — asserted in the controls below. + expect(pickLocalized({ en: 42, ja: '価格' }, 'fr')).toBe('価格'); + expect(pickLocalized({ en: true, ja: '価格' }, 'fr')).toBe('価格'); + expect(pickLocalized({ en: null, ja: '価格' }, 'fr')).toBe('価格'); + }); +}); + +describe('pickLocalized — controls: every real language tag is byte-identical (objectui#3907)', () => { + /** `[map, locale, the string this resolved to BEFORE the #3907 guards]` */ + const UNCHANGED: ReadonlyArray, string | undefined, string]> = [ + // Limb 1 — exact tag. + [{ en: 'Pricing', 'zh-CN': '定价' }, 'zh-CN', '定价'], + [{ en: 'Pricing', 'zh-CN': '定价' }, 'en', 'Pricing'], + [{ en: 'Pricing', 'zh-CN': '定价' }, ' zh-CN ', '定价'], + // Limb 2 — base language. + [{ en: 'Pricing', zh: '定价' }, 'zh-CN', '定价'], + [{ en: 'Pricing', zh: '定价' }, 'zh-Hans-CN', '定价'], + [{ zh: '基础', 'zh-CN': '区域' }, 'zh', '基础'], + // Limb 3 — a regional key sharing the base. + [{ en: 'Pricing', 'zh-CN': '定价' }, 'zh', '定价'], + [{ en: 'Pricing', 'zh-CN': '定价' }, 'zh-TW', '定价'], + [{ 'pt-BR': 'Preços' }, 'pt-PT', 'Preços'], + [{ 'zh-TW': '區域', 'zh-CN': '区域' }, 'zh', '區域'], + // Limb 4 — `default` outranks `en`. + [{ default: 'D', en: 'E' }, 'fr', 'D'], + // Limb 5 — `en`. + [{ en: 'E', ja: 'J' }, 'fr', 'E'], + [{ en: 'E', 'zh-CN': 'Z' }, undefined, 'E'], + [{ en: 'E', 'zh-CN': 'Z' }, '', 'E'], + // Limb 6 — last resort, in key order. + [{ ja: 'J' }, 'fr', 'J'], + [{ ja: 'J', ko: 'K' }, 'fr', 'J'], + // The language subtag's case still matters; the region's still does not. + [{ 'zh-CN': '定价' }, 'zh-cn', '定价'], + [{ 'zh-CN': '定价', en: 'Pricing' }, 'ZH-CN', 'Pricing'], + // Misses. + [{}, 'en', ''], + [{}, undefined, ''], + ]; + + it.each(UNCHANGED)('resolves %j at locale %j to %j, exactly as before', (map, locale, expected) => { + expect(pickLocalized(map, locale)).toBe(expected); + }); + + it('the non-map forms are untouched', () => { + expect(pickLocalized('Pricing', 'zh-CN')).toBe('Pricing'); + expect(pickLocalized('', 'zh-CN')).toBe(''); + expect(pickLocalized(null, 'zh')).toBe(''); + expect(pickLocalized(undefined, 'zh')).toBe(''); + // A scalar passed as the whole value is still stringified — that is the + // documented pass-through, and it is NOT what the value filter governs. + expect(pickLocalized(42, 'en')).toBe('42'); + expect(pickLocalized(true, 'en')).toBe('true'); + }); +}); diff --git a/packages/i18n/src/pickLocalized.ts b/packages/i18n/src/pickLocalized.ts index 9e5d92ae2..30fcb0a86 100644 --- a/packages/i18n/src/pickLocalized.ts +++ b/packages/i18n/src/pickLocalized.ts @@ -11,6 +11,27 @@ * * Pure — pair it with `useObjectTranslation().language` (or any current-locale * source) at the call site. + * + * Every limb reads **own properties only** and accepts **only `string` values** + * (objectui#3907). Both guards used to hold on the regional and last-resort + * limbs alone, which left the other four able to resolve a locale against + * `Object.prototype` — `pickLocalized({ en: 'Pricing' }, 'constructor')` + * rendered the constructor's source text as the label — and able to + * short-circuit the chain with a non-string value, rendering `[object Object]`. + * Neither is reachable by an input the contract admits (no BCP-47 tag is an + * `Object.prototype` member, and `InlineLocaleMapSchema` is + * `z.record(, z.string())`), so applying them uniformly changes nothing + * for any real language tag. It makes this function agree limb for limb with + * the backend twin `resolveI18nLabel` (`@objectstack/spec`, objectstack#6765), + * whose locale can arrive from an untrusted `Accept-Language` header and which + * shipped with exactly these two narrowings recorded as deliberate departures. + * The only remaining difference between the two is how each spells a miss — + * `''` here for a text node, `undefined` there for a producer's `?? name` chain + * — pinned in `plugin-list/src/__tests__/i18nLabel-resolver-parity.test.ts`. + * + * A guard makes its limb **miss**; it never aborts the resolution. An unusable + * entry falls through to the next limb exactly as an absent one does, so + * `pickLocalized({ en: 'Pricing' }, 'constructor')` is `'Pricing'`, not `''`. */ export function pickLocalized(value: unknown, language: string | undefined | null): string { if (value == null) return ''; @@ -18,19 +39,30 @@ export function pickLocalized(value: unknown, language: string | undefined | nul if (typeof value === 'number' || typeof value === 'boolean') return String(value); if (typeof value === 'object') { const o = value as Record; + /** + * One entry, or `undefined` when this limb does not hit — which is what + * lets the limbs keep chaining through `??` in the same order as before. + */ + const read = (key: string): string | undefined => { + if (!Object.prototype.hasOwnProperty.call(o, key)) return undefined; + const entry = o[key]; + return typeof entry === 'string' ? entry : undefined; + }; const lang = (language || 'en').trim(); const base = lang.split('-')[0]; // Runtime language is often a bare base code ('zh') while metadata authors // write full BCP-47 tags ('zh-CN') — upgrade to any key sharing the base. - const regional = Object.keys(o).find((k) => k.split('-')[0] === base && typeof o[k] === 'string'); + // `Object.keys` is already own-and-enumerable, so `read` here is uniformity + // rather than a second guard. + const regional = Object.keys(o).find((k) => k.split('-')[0] === base && read(k) !== undefined); const pick = - o[lang] ?? - o[base] ?? - (regional !== undefined ? o[regional] : undefined) ?? - o.default ?? - o.en ?? - Object.values(o).find((v) => typeof v === 'string'); - return pick == null ? '' : String(pick); + read(lang) ?? + read(base) ?? + (regional !== undefined ? read(regional) : undefined) ?? + read('default') ?? + read('en') ?? + Object.values(o).find((v): v is string => typeof v === 'string'); + return pick == null ? '' : pick; } return String(value); } diff --git a/packages/plugin-list/src/__tests__/i18nLabel-resolver-parity.test.ts b/packages/plugin-list/src/__tests__/i18nLabel-resolver-parity.test.ts index e0990622b..53d338c46 100644 --- a/packages/plugin-list/src/__tests__/i18nLabel-resolver-parity.test.ts +++ b/packages/plugin-list/src/__tests__/i18nLabel-resolver-parity.test.ts @@ -43,11 +43,29 @@ * that changes on one side and not the other fails here rather than in a * screenshot. * - * The ONE deliberate difference is normalized explicitly rather than hidden: + * The ONE remaining difference is normalized explicitly rather than hidden: * the spec's resolver reports a miss as `undefined` (so a caller's `?? name` * fallback chain can proceed), `pickLocalized` reports it as `''` (so a text * node renders nothing). `specForRender` states that conversion in one place; * if the two ever disagree about anything else, the table row fails. + * + * ## The two RULE divergences are gone as of objectui#3907 + * + * They are recorded here because this file is where the claim "one difference" + * has to stay true. `resolveI18nLabel` shipped (objectstack#6765) with two + * documented departures from `pickLocalized`, both narrowings: it read **own + * properties only**, and it applied the `typeof === 'string'` filter on **every** + * limb rather than only on limbs 3 and 6. Neither was reachable by an input + * `I18nLabelSchema` accepts, so both were filed as an observation + * (objectui#3907) instead of a defect. + * + * objectui#3907 landed the same two guards in `pickLocalized`, so the rule + * divergence is now **zero** and the surviving difference is purely the + * miss SPELLING above — a return-shape choice each side's callers need, not a + * disagreement about which entry to pick. The convergence is asserted, not + * asserted-in-prose: `CONVERGED` below carries the exact vectors that used to + * tell the two implementations apart, and every one of them is now answered + * identically. If either side regresses, those rows fail here. */ import { describe, it, expect } from 'vitest'; @@ -103,6 +121,56 @@ const TABLE: ReadonlyArray = [ [{}, 'en', ''], ]; +/** + * The vectors that used to separate the two implementations (objectui#3907). + * + * Every row here is OUT of the declared domain — no BCP-47 tag is an + * `Object.prototype` member, and `InlineLocaleMapSchema` is + * `z.record(, z.string())` so no in-contract map holds a non-string value. + * That is exactly why they are worth pinning separately from `TABLE`: they are + * the only inputs that could ever have told the two ends apart, so they are the + * only rows whose agreement is evidence rather than restatement. + * + * Note the expected answers are NOT `''`. A prototype-named locale, or an + * off-spec value, makes that LIMB miss — it does not abort the resolution. The + * chain continues to the next limb, so these resolve by the ordinary rule. + */ +const CONVERGED: ReadonlyArray = [ + // Own properties only — the locale names an `Object.prototype` member, so the + // limb misses and `en` (limb 5) answers. Before #3907 the objectui side + // returned the member's source text, e.g. 'function Object() { [native code] }'. + [{ en: 'Sales' }, 'constructor', 'Sales'], + [{ en: 'Sales' }, 'toString', 'Sales'], + [{ en: 'Sales' }, 'valueOf', 'Sales'], + [{ en: 'Sales' }, 'hasOwnProperty', 'Sales'], + [{ en: 'Sales' }, 'isPrototypeOf', 'Sales'], + [{ en: 'Sales' }, 'propertyIsEnumerable', 'Sales'], + [{ en: 'Sales' }, 'toLocaleString', 'Sales'], + // …and with no `en`, the last-resort limb answers instead. Only a map with + // nothing usable resolves to the miss. + [{ ja: '営業' }, 'constructor', '営業'], + [{}, 'constructor', ''], + // An own key really named like a prototype member is still the author's key: + // the guard is `hasOwnProperty`, not a denylist of names. + [{ constructor: 'Ctor', en: 'Sales' }, 'constructor', 'Ctor'], + + // `string` values only, on every limb — an off-spec value is treated as + // absent. Before #3907 the objectui side short-circuited on limbs 1/2/4/5 and + // rendered '[object Object]'. + [{ 'zh-CN': { nested: 'x' }, en: 'Sales' }, 'zh-CN', 'Sales'], // limb 1 + [{ zh: { nested: 'x' }, en: 'Sales' }, 'zh-CN', 'Sales'], // limb 2 + [{ default: { nested: 'x' }, ja: '営業' }, 'fr', '営業'], // limb 4 + [{ en: { nested: 'x' }, ja: '営業' }, 'fr', '営業'], // limb 5 + [{ en: 42, ja: '営業' }, 'fr', '営業'], + [{ en: null, ja: '営業' }, 'fr', '営業'], + // Nothing usable anywhere is a miss on both sides. + [{ en: { nested: 'x' } }, 'fr', ''], + // The boundary the value filter must not cross: `''` is a string, so the limb + // HITS and the chain stops. A truthiness filter would pass every row above + // and break this one. + [{ en: '', 'zh-CN': '销售' }, 'en', ''], +]; + describe('inline per-locale label resolution agrees across both resolvers (#4163)', () => { it.each(TABLE)( 'resolves %j at locale %j to %j identically', @@ -123,13 +191,41 @@ describe('inline per-locale label resolution agrees across both resolvers (#4163 expect(pickLocalized(undefined, 'en')).toBe(''); }); + it.each(CONVERGED)( + 'agrees on %j at locale %j (the objectui#3907 convergence)', + (label, locale, expected) => { + expect(pickLocalized(label, locale)).toBe(expected); + expect(specForRender(label, locale)).toBe(expected); + }, + ); + + it('has no rule divergence left — only the miss spelling differs (objectui#3907)', () => { + // The whole claim of this file in one pass, over BOTH tables. Before #3907 + // the `CONVERGED` rows disagreed; the header's "one difference" is only + // true while this stays empty. + const disagreements = [...TABLE, ...CONVERGED] + .filter(([label, locale]) => pickLocalized(label, locale) !== specForRender(label, locale)) + .map(([label, locale]) => `${JSON.stringify(label)} @ ${String(locale)}`); + expect(disagreements).toEqual([]); + }); + it('neither resolver ever hands a text node the stringified object', () => { // The harm #4163 exists for, stated once over the whole table rather than // per site — a resolver that started returning the map would satisfy no // assertion above but would also fail none of them for an untabled input. - for (const [label, locale] of TABLE) { + for (const [label, locale] of [...TABLE, ...CONVERGED]) { expect(pickLocalized(label, locale)).not.toBe('[object Object]'); expect(specForRender(label, locale)).not.toBe('[object Object]'); } }); + + it('neither resolver ever hands a text node a function body', () => { + // The other half of the same harm, and the one objectui#3907 fixed: a + // locale that names an `Object.prototype` member used to render that + // member's SOURCE TEXT as the label on the objectui side. + for (const [label, locale] of [...TABLE, ...CONVERGED]) { + expect(pickLocalized(label, locale)).not.toContain('native code'); + expect(specForRender(label, locale)).not.toContain('native code'); + } + }); });