From 97e6e6044a53c48a0feb350a68b4fb9a594600b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 13:34:43 +0000 Subject: [PATCH] fix(app-shell): build the `filter[...]` memo key with URLSearchParams `ObjectView` built its memo key by joining `key=value` pairs from `searchParams.entries()` and then re-parsed that string. `entries()` yields DECODED values, so the join put back, unescaped, the two characters that are structural in a query string: `&` truncated the value at its first occurrence (`Smith & Sons` reached the reader as `Smith `, leaving a stray empty-valued param) and `+` arrived as a space (`A+B` as `A B`). The destination list rendered, scoped by a silently wrong value. The selection now appends onto a `URLSearchParams` (`selectFilterParams`, an exported helper so the shape is assertable without mounting the view) which the memo reads DIRECTLY; `toString()` percent-encodes and only keys the memo, so the serialize-then-re-parse round trip is gone rather than patched. The key still absorbs unrelated `uf_*` params, which is the only reason it exists. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011QreXiyMEqKLN4U5daMPVa --- ...7-objectview-filter-params-key-encoding.md | 26 +++ ...tView.filterParamsKeyEncoding-9287.test.ts | 172 ++++++++++++++++++ packages/app-shell/src/views/ObjectView.tsx | 43 ++++- 3 files changed, 236 insertions(+), 5 deletions(-) create mode 100644 .changeset/9287-objectview-filter-params-key-encoding.md create mode 100644 packages/app-shell/src/views/ObjectView.filterParamsKeyEncoding-9287.test.ts diff --git a/.changeset/9287-objectview-filter-params-key-encoding.md b/.changeset/9287-objectview-filter-params-key-encoding.md new file mode 100644 index 0000000000..fb076892cb --- /dev/null +++ b/.changeset/9287-objectview-filter-params-key-encoding.md @@ -0,0 +1,26 @@ +--- +'@object-ui/app-shell': patch +--- + +`ObjectView`'s `filter[...]` memo key is built with `URLSearchParams` instead of by +hand, so a filter value containing `&` or `+` reaches the reader intact +(objectui#9287). + +The key was `entries().map(([k, v]) => k + '=' + v).join('&')`, then re-parsed with +`new URLSearchParams(key)`. `entries()` yields DECODED values, and the join put the +two characters that are structural in a query string back unescaped: `&` truncated +the value at its first occurrence (`Smith & Sons` reached the reader as `Smith `, +leaving a stray empty-valued param) and `+` arrived as a space (`A+B` as `A B`). +Neither produced an absent condition — the destination list rendered, scoped by a +silently wrong value, with nothing to say the value had been cut. + +Reachable from ordinary in-app navigation: any related-list "View All" (or nav +`filters` link) whose parent key value contains an ampersand or a plus. A company +name holding `&` is ordinary data, not hostile input. + +The selection now appends onto a `URLSearchParams` (`selectFilterParams`) which the +memo reads directly; `toString()` percent-encodes and only keys the memo, so the +serialize-then-re-parse round trip that lost the character is gone rather than +patched. The key still absorbs unrelated `uf_*` params, which is the only reason it +exists. Plain equality is unchanged — `filter[account_name]=Acme` reads identically +before and after. diff --git a/packages/app-shell/src/views/ObjectView.filterParamsKeyEncoding-9287.test.ts b/packages/app-shell/src/views/ObjectView.filterParamsKeyEncoding-9287.test.ts new file mode 100644 index 0000000000..12e1b6993a --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.filterParamsKeyEncoding-9287.test.ts @@ -0,0 +1,172 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9287 — `ObjectView` re-serialized its `filter[...]` memo key BY HAND, + * so a value containing `&` or `+` reached the reader mangled. + * + * The key was built as `entries().map(([k, v]) => k + '=' + v).join('&')` and + * then re-parsed with `new URLSearchParams(key)`. `entries()` yields DECODED + * values, and the join put back, unescaped, the two characters that are + * STRUCTURAL in a query string: + * + * - `&` TRUNCATED the value at its first occurrence — `Smith & Sons` reached + * the reader as `Smith `, plus a stray empty-valued param; + * - `+` came back as a space — `A+B` as `A B`. + * + * Neither is an absent condition. The list renders, scoped by + * `account_name = 'Smith '`, and nothing anywhere says the value was cut. + * + * ## SUITE DIRECTION — stated before running + * + * RED on the unmodified base tree: the `Smith & Sons` and `A+B` rows of the + * card's table (all three assertions each), plus both named-defect tests. The + * base tree has no `selectFilterParams` to import at all, so on the literal + * base bytes the red arrives as a resolution failure; the behavioural + * before/after over the live construction is in the PR body, and the ablation + * that reds this file against the repaired tree restores the hand-join INSIDE + * `selectFilterParams` rather than copying it here. + * + * GREEN on both sides, by design: the LIT CONTROL (`Acme`) and the other five + * clean rows of the card's table. They are recorded because they are the + * evidence that ordinary values were not broken while the two failing ones + * were repaired — not because they pin this change. + * + * ## Every control here can fire + * + * - The LIT CONTROL is `Acme`: it reads identically on both sides of the + * mangling, so neither the two failures nor the six clean rows are unmeasured + * cells. It fires if this repair over-reaches — a key that double-encoded, or + * one that stopped reading plain values, breaks it while leaving the two + * defect rows green. + * - `a b` and `100%` are in the table for the same reason: a repair that + * percent-encoded the ALREADY-decoded value twice reads `a%20b` back, and a + * repair that encoded the key string as a whole reads `100%` back as a broken + * escape. Both are green here and must stay green. + * - The memo-identity block carries a non-vacuity control: a key that collapsed + * to a constant would satisfy "unchanged by an unrelated `uf_*` write" while + * asserting nothing, so a DIFFERENT filter value must produce a different key + * in the same run. + * + * ⛔ This file does not touch objectui#9196's plain-equality control — that pin + * lives in `ObjectView.urlFilterSuffix-9196.test.ts` and is what proves the + * equality arm did not move. `Acme` here is this card's own lit control. + */ + +import { describe, it, expect } from 'vitest'; +import { selectFilterParams } from './ObjectView'; +import { parseUrlEqualityFilterTriples, serializeDrillFilterParams } from './drillUrlFilters'; + +/** The card's own example: a related-list "View All" scoped by a parent name. */ +const FIELD = 'account_name'; + +/** + * One trip down the real chain, from the URL a browser carries to the value the + * reader receives. + * + * The URL is built by `serializeDrillFilterParams` — this family's declared + * encoder, and the same `URLSearchParams` + `toString()` shape the two live + * producers of this route use (`NavigationRenderer`'s `filters` links and + * `RelatedList`'s parent scope). An unrelated `uf_*` param rides along in every + * case, because absorbing those is the only reason the memo key exists. + */ +function tripFor(value: string) { + const emitted = serializeDrillFilterParams({ [FIELD]: value }).toString(); + // What react-router parses out of `?...` and hands the view. + const searchParams = new URLSearchParams(`${emitted}&uf_status=open`); + const filterParams = selectFilterParams(searchParams); + const key = filterParams.toString(); + return { + key, + entries: Array.from(filterParams.entries()), + /** What the component reads: the params object, directly. */ + read: parseUrlEqualityFilterTriples(filterParams), + /** The same params re-derived from the memo key — the round trip. */ + reread: parseUrlEqualityFilterTriples(new URLSearchParams(key)), + }; +} + +/** + * The card's table, verbatim. `discriminating` marks the rows measured RED + * before this repair; the rest were clean on both sides and are carried so the + * failures are not a two-cell reading. + */ +const ROWS: Array<{ value: string; discriminating: boolean }> = [ + { value: 'Acme', discriminating: false }, // LIT CONTROL + { value: 'Smith & Sons', discriminating: true }, + { value: 'A+B', discriminating: true }, + { value: 'a=b', discriminating: false }, + { value: '100%', discriminating: false }, + { value: 'a b', discriminating: false }, + { value: 'Ünïcøde', discriminating: false }, + { value: 'a#b', discriminating: false }, +]; + +describe("objectui#9287 — the card's table: the value in the URL is the value the reader receives", () => { + it.each(ROWS)('$value (discriminating: $discriminating)', ({ value }) => { + const { read, reread, entries } = tripFor(value); + // What the component does: read the params object directly. + expect(read).toEqual([[FIELD, '=', value]]); + // And the memo key is a faithful identity for exactly those params. + expect(reread).toEqual([[FIELD, '=', value]]); + // One condition in, one param out — no stray empty-valued leftover. + expect(entries).toEqual([[`filter[${FIELD}]`, value]]); + }); +}); + +describe('objectui#9287 — the two failures, named', () => { + it('an ampersand does not truncate the value at its first occurrence', () => { + const { read, entries } = tripFor('Smith & Sons'); + expect(read).toEqual([[FIELD, '=', 'Smith & Sons']]); + // The base tree emitted `Smith ` AND a second, empty-valued param (` Sons`), + // so the truncation is stated both ways: the value that arrives, and the + // debris the split left behind. + expect(read).not.toContainEqual([FIELD, '=', 'Smith ']); + expect(entries).toHaveLength(1); + }); + + it('a plus stays a plus instead of arriving as a space', () => { + const { read } = tripFor('A+B'); + expect(read).toEqual([[FIELD, '=', 'A+B']]); + expect(read).not.toContainEqual([FIELD, '=', 'A B']); + }); + + it('LIT CONTROL — a plain value reads the same on both sides of the mangling', () => { + // Green before this repair and green after. It is the evidence that the two + // rows above are a repair and not a rewrite of what the reader receives. + expect(tripFor('Acme').read).toEqual([[FIELD, '=', 'Acme']]); + }); +}); + +describe('objectui#9287 — the key still does the one job it exists for', () => { + const keyFor = (search: string) => selectFilterParams(new URLSearchParams(search)).toString(); + + it('an unrelated `uf_*` write does not change the key', () => { + const a = keyFor('filter[stage]=won&uf_status=open'); + const b = keyFor('filter[stage]=won&uf_status=closed&uf_owner=me'); + expect(b).toBe(a); + }); + + it('non-vacuity control — a different filter value DOES change the key', () => { + // Without this, a key that collapsed to a constant (or to '') would pass the + // assertion above while identifying nothing. + expect(keyFor('filter[stage]=lost&uf_status=open')).not.toBe( + keyFor('filter[stage]=won&uf_status=open'), + ); + }); + + it('carries every `filter[...]` param and nothing else', () => { + const params = selectFilterParams( + new URLSearchParams('filter[stage]=won&uf_status=open&recordId=abc&filter[region]=emea'), + ); + expect(Array.from(params.entries())).toEqual([ + ['filter[stage]', 'won'], + ['filter[region]', 'emea'], + ]); + }); +}); diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 9fe318c0aa..28c951c927 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -1029,6 +1029,31 @@ export function buildPersistedViewBody( return viewKind === undefined ? { ...patch } : { ...patch, viewKind }; } +/** + * The `filter[...]` params of a URL, selected out of the full search params as + * their own `URLSearchParams`. Extracted for the same reason `buildViewTabs` + * above is: so the shape is assertable without mounting the view. + * + * Built by APPENDING onto a `URLSearchParams` rather than joining `key=value` + * pairs into a string by hand (objectui#9287). `searchParams.entries()` yields + * DECODED values, so a hand-joined key re-introduced, unescaped, the two + * characters that are structural in a query string: `&` TRUNCATED the value at + * its first occurrence (`Smith & Sons` reached the reader as `Smith `, plus a + * stray empty-valued param) and `+` came back as a space (`A+B` as `A B`). + * Neither produces an absent condition — the list renders, scoped by a silently + * WRONG value, and nothing anywhere says the value was cut. + * + * `toString()` percent-encodes, so the serialized form still round-trips and + * still ignores the unrelated `uf_*` params the memo key exists to absorb. + */ +export function selectFilterParams(searchParams: URLSearchParams): URLSearchParams { + const filterParams = new URLSearchParams(); + searchParams.forEach((value, key) => { + if (key.startsWith('filter[')) filterParams.append(key, value); + }); + return filterParams; +} + export function ObjectView({ dataSource, objects, onEdit, externalRefreshKey }: any) { const { objectName } = useParams(); const { t } = useObjectTranslation(); @@ -2102,12 +2127,20 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an // Dep on the serialized `filter[...]` entries only — `uf_*` user-filter // params also live in the URL and must not invalidate this memo (a new // array identity here rebuilds the whole list schema and refetches). - const filterParamsKey = Array.from(searchParams.entries()) - .filter(([k]) => k.startsWith('filter[')) - .map(([k, v]) => `${k}=${v}`) - .join('&'); + const filterParams = selectFilterParams(searchParams); + const filterParamsKey = filterParams.toString(); const urlFilters = useMemo( - () => parseUrlEqualityFilterTriples(new URLSearchParams(filterParamsKey)), + // The params object is read DIRECTLY — nothing is serialized here and + // parsed back, so there is no round trip left for a character to be + // lost in (objectui#9287). `filterParamsKey` is the memo's identity + // only. + () => parseUrlEqualityFilterTriples(filterParams), + // Keyed on the SERIALIZED params, not on the object's identity: + // `filterParams` is rebuilt every render, so listing it would + // invalidate this memo on every unrelated `uf_*` write — the churn + // this key exists to prevent. Equal keys imply equal contents, so the + // captured object is never stale. + // eslint-disable-next-line react-hooks/exhaustive-deps -- see above [filterParamsKey], );