diff --git a/.changeset/proud-pets-fix.md b/.changeset/proud-pets-fix.md new file mode 100644 index 0000000000..300978653b --- /dev/null +++ b/.changeset/proud-pets-fix.md @@ -0,0 +1,7 @@ +--- +'@tanstack/table-core': patch +--- + +fix prototype-named column ids (e.g. `hasOwnProperty`, `toString`) crashing filtering/sorting + +Row value caches (`_valuesCache`, `_uniqueValuesCache`, `_groupingValuesCache`) were plain objects probed with `cache.hasOwnProperty(columnId)`. A column whose id collided with an `Object.prototype` member shadowed the inherited method on write, so the next probe invoked a cached value as a function and threw `TypeError`. Caches are now created with `Object.create(null)` and read via `Object.prototype.hasOwnProperty.call`, and `_getAllCellsByColumnId` is likewise null-prototyped so prototype-named ids resolve correctly. diff --git a/packages/table-core/src/core/row.ts b/packages/table-core/src/core/row.ts index ec09633087..19a72bc4c5 100644 --- a/packages/table-core/src/core/row.ts +++ b/packages/table-core/src/core/row.ts @@ -107,10 +107,10 @@ export const createRow = ( original, depth, parentId, - _valuesCache: {}, - _uniqueValuesCache: {}, + _valuesCache: Object.create(null), + _uniqueValuesCache: Object.create(null), getValue: (columnId) => { - if (row._valuesCache.hasOwnProperty(columnId)) { + if (Object.prototype.hasOwnProperty.call(row._valuesCache, columnId)) { return row._valuesCache[columnId] } @@ -128,7 +128,9 @@ export const createRow = ( return row._valuesCache[columnId] as any }, getUniqueValues: (columnId) => { - if (row._uniqueValuesCache.hasOwnProperty(columnId)) { + if ( + Object.prototype.hasOwnProperty.call(row._uniqueValuesCache, columnId) + ) { return row._uniqueValuesCache[columnId] } @@ -185,7 +187,7 @@ export const createRow = ( acc[cell.column.id] = cell return acc }, - {} as Record>, + Object.create(null) as Record>, ) }, getMemoOptions(table.options, 'debugRows', 'getAllCellsByColumnId'), diff --git a/packages/table-core/src/features/ColumnGrouping.ts b/packages/table-core/src/features/ColumnGrouping.ts index 58b9fa082b..641f692971 100644 --- a/packages/table-core/src/features/ColumnGrouping.ts +++ b/packages/table-core/src/features/ColumnGrouping.ts @@ -363,7 +363,12 @@ export const ColumnGrouping: TableFeature = { ): void => { row.getIsGrouped = () => !!row.groupingColumnId row.getGroupingValue = (columnId) => { - if (row._groupingValuesCache.hasOwnProperty(columnId)) { + if ( + Object.prototype.hasOwnProperty.call( + row._groupingValuesCache, + columnId, + ) + ) { return row._groupingValuesCache[columnId] } @@ -379,7 +384,7 @@ export const ColumnGrouping: TableFeature = { return row._groupingValuesCache[columnId] } - row._groupingValuesCache = {} + row._groupingValuesCache = Object.create(null) }, createCell: ( diff --git a/packages/table-core/src/utils/getGroupedRowModel.ts b/packages/table-core/src/utils/getGroupedRowModel.ts index 33267bcaa1..688a3fdaaf 100644 --- a/packages/table-core/src/utils/getGroupedRowModel.ts +++ b/packages/table-core/src/utils/getGroupedRowModel.ts @@ -94,7 +94,12 @@ export function getGroupedRowModel(): ( getValue: (columnId: string) => { // Don't aggregate columns that are in the grouping if (existingGrouping.includes(columnId)) { - if (row._valuesCache.hasOwnProperty(columnId)) { + if ( + Object.prototype.hasOwnProperty.call( + row._valuesCache, + columnId, + ) + ) { return row._valuesCache[columnId] } @@ -106,7 +111,12 @@ export function getGroupedRowModel(): ( return row._valuesCache[columnId] } - if (row._groupingValuesCache.hasOwnProperty(columnId)) { + if ( + Object.prototype.hasOwnProperty.call( + row._groupingValuesCache, + columnId, + ) + ) { return row._groupingValuesCache[columnId] } diff --git a/packages/table-core/tests/prototypeColumnId.test.ts b/packages/table-core/tests/prototypeColumnId.test.ts new file mode 100644 index 0000000000..85b5150ab3 --- /dev/null +++ b/packages/table-core/tests/prototypeColumnId.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from 'vitest' +import { ColumnDef, getCoreRowModel } from '../src' +import { createTable } from '../src/core/table' + +type Row = Record + +// Column ids that collide with Object.prototype members used to poison the +// per-row value caches (see Row.getValue / _valuesCache). These must be +// treated as plain data keys, not as inherited methods. +const PROTOTYPE_IDS = [ + 'hasOwnProperty', + 'toString', + 'constructor', + 'valueOf', + '__proto__', + 'isPrototypeOf', +] + +function makeTable(data: Row[]) { + const columns: ColumnDef[] = PROTOTYPE_IDS.map((id) => ({ + id, + accessorFn: (row) => row[id], + })) + + return createTable({ + onStateChange() {}, + renderFallbackValue: '', + data, + state: {}, + columns, + getCoreRowModel: getCoreRowModel(), + }) +} + +describe('prototype-named column ids', () => { + it('does not throw and returns the correct value from getValue', () => { + const data: Row[] = [ + Object.fromEntries(PROTOTYPE_IDS.map((id) => [id, `${id}-value`])), + ] + const table = makeTable(data) + const row = table.getCoreRowModel().rows[0]! + + for (const id of PROTOTYPE_IDS) { + // First read populates the cache, the second reads it back. Neither + // should throw, and both should yield the user-supplied value. + expect(() => row.getValue(id)).not.toThrow() + expect(row.getValue(id)).toBe(`${id}-value`) + expect(row.getValue(id)).toBe(`${id}-value`) + } + }) + + it('does not let a cached hasOwnProperty value break later getValue calls', () => { + const data: Row[] = [ + Object.fromEntries(PROTOTYPE_IDS.map((id) => [id, `${id}-value`])), + ] + const table = makeTable(data) + const row = table.getCoreRowModel().rows[0]! + + // Reading the hasOwnProperty column used to overwrite the inherited method + // on the cache, breaking every subsequent getValue on the row. + expect(row.getValue('hasOwnProperty')).toBe('hasOwnProperty-value') + expect(() => row.getValue('toString')).not.toThrow() + expect(row.getValue('toString')).toBe('toString-value') + }) + + it('returns unique values for prototype-named column ids', () => { + const data: Row[] = [ + Object.fromEntries(PROTOTYPE_IDS.map((id) => [id, `${id}-value`])), + ] + const table = makeTable(data) + const row = table.getCoreRowModel().rows[0]! + + for (const id of PROTOTYPE_IDS) { + expect(() => row.getUniqueValues(id)).not.toThrow() + expect(row.getUniqueValues(id)).toEqual([`${id}-value`]) + } + }) +})