From 62b26f9c6872259142c23c103d2fc46d25a2dcc3 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Mon, 8 Dec 2025 13:31:37 +0100 Subject: [PATCH 1/2] fix: disallow getting unsafe properties using function `get` --- README.md | 2 +- src/compile.test.ts | 18 +++++++++++++ src/functions.ts | 6 ++--- src/is.test.ts | 52 ++++++++++++++++++++++++++++++++++++++ src/is.ts | 20 +++++++++++++++ test-suite/parse.test.json | 4 --- 6 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 src/is.test.ts diff --git a/README.md b/README.md index 4503d7f..fc2f2b0 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Try it out on the online playground: ## Features -- Small: just `4.1 kB` when minified and gzipped! The JSON query engine without parse/stringify is only `2.1 kB`. +- Small: just `4.2 kB` when minified and gzipped! The JSON query engine without parse/stringify is only `2.2 kB`. - Feature rich (50+ powerful functions and operators) - Easy to interoperate with thanks to the intermediate JSON format. - Expressive diff --git a/src/compile.test.ts b/src/compile.test.ts index 10d36d9..9f202af 100644 --- a/src/compile.test.ts +++ b/src/compile.test.ts @@ -60,6 +60,24 @@ for (const [category, testGroups] of Object.entries(testsByCategory)) { } describe('error handling', () => { + test('should throw an error when trying to get a property from something that is not a plain object', () => { + const obj = { value: 42 } + expect(() => compile(['get', 'constructor'])(obj)).toThrow('Unsafe property "constructor"') + expect(() => compile(['get', '__proto__'])(obj)).toThrow('Unsafe property "__proto__"') + + const createdObj = Object.create(obj) + expect(compile(['get', 'value'])(createdObj)).toEqual(42) + expect(() => compile(['get', 'constructor'])(createdObj)).toThrow( + 'Unsafe property "constructor"' + ) + expect(() => compile(['get', '__proto__'])(createdObj)).toThrow('Unsafe property "__proto__"') + + const arr = [40, 41, 42] + expect(() => compile(['get', 'constructor'])(arr)).toThrow('Unsafe property "constructor"') + expect(() => compile(['get', '__proto__'])(arr)).toThrow('Unsafe property "__proto__"') + expect(() => compile(['get', 'length'])(arr)).toThrow('Unsafe property "length"') + }) + test('should throw a helpful error when a pipe contains a compile time error', () => { let actualErr = undefined try { diff --git a/src/functions.ts b/src/functions.ts index 801d2db..8fd7ce1 100644 --- a/src/functions.ts +++ b/src/functions.ts @@ -1,5 +1,5 @@ import { compile } from './compile' -import { isArray, isEqual } from './is' +import { getSafeProperty, isArray, isEqual } from './is' import type { Entry, FunctionBuilder, @@ -72,14 +72,14 @@ export const functions: FunctionBuildersMap = { if (path.length === 1) { const prop = path[0] - return (data: unknown) => data?.[prop] ?? null + return (data: unknown) => getSafeProperty(data, prop) ?? null } return (data: unknown) => { let value = data for (const prop of path) { - value = value?.[prop] + value = getSafeProperty(value, prop) } return value ?? null diff --git a/src/is.test.ts b/src/is.test.ts new file mode 100644 index 0000000..a50c8f5 --- /dev/null +++ b/src/is.test.ts @@ -0,0 +1,52 @@ +import { expect, test } from 'vitest' +import { getSafeProperty, isSafeProperty } from './is' + +test('isSafeProperty', () => { + const obj = { value: 42 } + expect(isSafeProperty(obj, 'prop')).toEqual(true) + expect(isSafeProperty(obj, 'toString')).toEqual(false) + expect(isSafeProperty(obj, 'constructor')).toEqual(false) + expect(isSafeProperty(obj, '__proto__')).toEqual(false) + + const createdObj = Object.create(obj) + expect(isSafeProperty(createdObj, 'value')).toEqual(true) + expect(isSafeProperty(createdObj, 'toString')).toEqual(false) + expect(isSafeProperty(createdObj, 'constructor')).toEqual(false) + expect(isSafeProperty(createdObj, '__proto__')).toEqual(false) + + const arr = [40, 41, 42] + expect(isSafeProperty(arr, '1')).toEqual(true) + expect(isSafeProperty(arr, 1)).toEqual(true) + expect(isSafeProperty(arr, 'constructor')).toEqual(false) + expect(isSafeProperty(arr, '__proto__')).toEqual(false) + expect(isSafeProperty(arr, 'length')).toEqual(false) // TODO: we could consider this safe + + class TestClass { + value = 42 + } + const t = new TestClass() + expect(isSafeProperty(t, 'value')).toEqual(false) // We don't support classes, only plain JSON + expect(isSafeProperty(t, 'foo')).toEqual(false) + expect(isSafeProperty(t, 'constructor')).toEqual(false) + expect(isSafeProperty(t, '__proto__')).toEqual(false) + + expect(isSafeProperty(function foo() {}, 'name')).toEqual(false) +}) + +test('getSafeProperty', () => { + const obj = { value: 42 } + expect(getSafeProperty(obj, 'value')).toEqual(42) + expect(getSafeProperty(obj, 'foo')).toEqual(undefined) + expect(() => getSafeProperty(obj, 'constructor')).toThrow(/Unsafe property "constructor"/) + expect(() => getSafeProperty(obj, '__proto__')).toThrow(/Unsafe property "__proto__"/) + + const arr = [40, 41, 42] + expect(getSafeProperty(arr, '2')).toEqual(42) + expect(getSafeProperty(arr, 2)).toEqual(42) + expect(() => getSafeProperty(arr, 'constructor')).toThrow(/Unsafe property "constructor"/) + expect(() => getSafeProperty(arr, '__proto__')).toThrow(/Unsafe property "__proto__"/) + expect(() => getSafeProperty(arr, 'length')).toThrow(/Unsafe property "length"/) + + expect(getSafeProperty(null, 'foo')).toEqual(undefined) + expect(getSafeProperty(undefined, 'foo')).toEqual(undefined) +}) diff --git a/src/is.ts b/src/is.ts index 8f6a70f..ef922b7 100644 --- a/src/is.ts +++ b/src/is.ts @@ -3,6 +3,10 @@ export const isArray = (value: unknown): value is T[] => Array.isArray(value) export const isObject = (value: unknown): value is object => value !== null && typeof value === 'object' && !isArray(value) +function isPlainObject(value: unknown): value is Record { + return value !== null && typeof value === 'object' && value.constructor === Object +} + export const isString = (value: unknown): value is string => typeof value === 'string' // source: https://stackoverflow.com/a/77278013/1262753 @@ -19,3 +23,19 @@ export const isEqual = (a: T, b: T): boolean => { Object.entries(a).every(([k, v]) => isEqual(v, b[k as keyof T])) ) } + +export const isSafeProperty = (object: unknown, prop: string | number): boolean => { + return isPlainObject(object) + ? !(prop in Object.prototype) + : Array.isArray(object) + ? !(prop in Array.prototype) + : false +} + +export const getSafeProperty = (object: unknown, prop: string | number): unknown => { + if (object && !isSafeProperty(object, prop)) { + throw new TypeError(`Unsafe property "${prop}"`) + } + + return object?.[prop] +} diff --git a/test-suite/parse.test.json b/test-suite/parse.test.json index b61bca2..09d643d 100644 --- a/test-suite/parse.test.json +++ b/test-suite/parse.test.json @@ -65,10 +65,6 @@ { "input": ".data | groupBy(.city).myCity", "output": ["pipe", ["get", "data"], ["groupBy", ["get", "city"]], ["get", "myCity"]] - }, - { - "input": "\"hello\".length", - "output": ["pipe", "hello", ["get", "length"]] } ] }, From b7d54947a4b8e368c55dd3f2cbe255b43b888191 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Tue, 9 Dec 2025 16:25:08 +0100 Subject: [PATCH 2/2] fix: simplify and improve getSafeProperty --- src/compile.test.ts | 21 +++++++++------- src/is.test.ts | 46 +++++++----------------------------- src/is.ts | 30 +++++++++++------------ test-suite/compile.test.json | 8 +++++++ 4 files changed, 44 insertions(+), 61 deletions(-) diff --git a/src/compile.test.ts b/src/compile.test.ts index 9f202af..5e23372 100644 --- a/src/compile.test.ts +++ b/src/compile.test.ts @@ -62,20 +62,25 @@ for (const [category, testGroups] of Object.entries(testsByCategory)) { describe('error handling', () => { test('should throw an error when trying to get a property from something that is not a plain object', () => { const obj = { value: 42 } - expect(() => compile(['get', 'constructor'])(obj)).toThrow('Unsafe property "constructor"') - expect(() => compile(['get', '__proto__'])(obj)).toThrow('Unsafe property "__proto__"') + expect(() => compile(['get', 'constructor'])(obj)).toThrow('Unsupported property "constructor"') + expect(() => compile(['get', '__proto__'])(obj)).toThrow('Unsupported property "__proto__"') + // we could technically support this, but we only support plain objects for now to keep the implementation simple const createdObj = Object.create(obj) - expect(compile(['get', 'value'])(createdObj)).toEqual(42) + expect(() => compile(['get', 'value'])(createdObj)).toThrow('Unsupported property "value"') expect(() => compile(['get', 'constructor'])(createdObj)).toThrow( - 'Unsafe property "constructor"' + 'Unsupported property "constructor"' + ) + expect(() => compile(['get', '__proto__'])(createdObj)).toThrow( + 'Unsupported property "__proto__"' ) - expect(() => compile(['get', '__proto__'])(createdObj)).toThrow('Unsafe property "__proto__"') const arr = [40, 41, 42] - expect(() => compile(['get', 'constructor'])(arr)).toThrow('Unsafe property "constructor"') - expect(() => compile(['get', '__proto__'])(arr)).toThrow('Unsafe property "__proto__"') - expect(() => compile(['get', 'length'])(arr)).toThrow('Unsafe property "length"') + expect(() => compile(['get', 'constructor'])(arr)).toThrow('Unsupported property "constructor"') + expect(() => compile(['get', '__proto__'])(arr)).toThrow('Unsupported property "__proto__"') + expect(() => compile(['get', 'length'])(arr)).toThrow('Unsupported property "length"') + + expect(() => compile(['get', 'length'])('text')).toThrow('Unsupported property "length"') }) test('should throw a helpful error when a pipe contains a compile time error', () => { diff --git a/src/is.test.ts b/src/is.test.ts index a50c8f5..5dbe86b 100644 --- a/src/is.test.ts +++ b/src/is.test.ts @@ -1,51 +1,21 @@ import { expect, test } from 'vitest' -import { getSafeProperty, isSafeProperty } from './is' - -test('isSafeProperty', () => { - const obj = { value: 42 } - expect(isSafeProperty(obj, 'prop')).toEqual(true) - expect(isSafeProperty(obj, 'toString')).toEqual(false) - expect(isSafeProperty(obj, 'constructor')).toEqual(false) - expect(isSafeProperty(obj, '__proto__')).toEqual(false) - - const createdObj = Object.create(obj) - expect(isSafeProperty(createdObj, 'value')).toEqual(true) - expect(isSafeProperty(createdObj, 'toString')).toEqual(false) - expect(isSafeProperty(createdObj, 'constructor')).toEqual(false) - expect(isSafeProperty(createdObj, '__proto__')).toEqual(false) - - const arr = [40, 41, 42] - expect(isSafeProperty(arr, '1')).toEqual(true) - expect(isSafeProperty(arr, 1)).toEqual(true) - expect(isSafeProperty(arr, 'constructor')).toEqual(false) - expect(isSafeProperty(arr, '__proto__')).toEqual(false) - expect(isSafeProperty(arr, 'length')).toEqual(false) // TODO: we could consider this safe - - class TestClass { - value = 42 - } - const t = new TestClass() - expect(isSafeProperty(t, 'value')).toEqual(false) // We don't support classes, only plain JSON - expect(isSafeProperty(t, 'foo')).toEqual(false) - expect(isSafeProperty(t, 'constructor')).toEqual(false) - expect(isSafeProperty(t, '__proto__')).toEqual(false) - - expect(isSafeProperty(function foo() {}, 'name')).toEqual(false) -}) +import { getSafeProperty } from './is' test('getSafeProperty', () => { const obj = { value: 42 } expect(getSafeProperty(obj, 'value')).toEqual(42) expect(getSafeProperty(obj, 'foo')).toEqual(undefined) - expect(() => getSafeProperty(obj, 'constructor')).toThrow(/Unsafe property "constructor"/) - expect(() => getSafeProperty(obj, '__proto__')).toThrow(/Unsafe property "__proto__"/) + expect(() => getSafeProperty(obj, 'constructor')).toThrow(/Unsupported property "constructor"/) + expect(() => getSafeProperty(obj, '__proto__')).toThrow(/Unsupported property "__proto__"/) + expect(() => getSafeProperty(obj, 'valueOf')).toThrow(/Unsupported property "valueOf"/) const arr = [40, 41, 42] expect(getSafeProperty(arr, '2')).toEqual(42) expect(getSafeProperty(arr, 2)).toEqual(42) - expect(() => getSafeProperty(arr, 'constructor')).toThrow(/Unsafe property "constructor"/) - expect(() => getSafeProperty(arr, '__proto__')).toThrow(/Unsafe property "__proto__"/) - expect(() => getSafeProperty(arr, 'length')).toThrow(/Unsafe property "length"/) + expect(() => getSafeProperty(arr, 'constructor')).toThrow(/Unsupported property "constructor"/) + expect(() => getSafeProperty(arr, '__proto__')).toThrow(/Unsupported property "__proto__"/) + expect(() => getSafeProperty(arr, 'length')).toThrow(/Unsupported property "length"/) + expect(() => getSafeProperty(arr, 'map')).toThrow(/Unsupported property "map"/) expect(getSafeProperty(null, 'foo')).toEqual(undefined) expect(getSafeProperty(undefined, 'foo')).toEqual(undefined) diff --git a/src/is.ts b/src/is.ts index ef922b7..4ff3d31 100644 --- a/src/is.ts +++ b/src/is.ts @@ -3,10 +3,6 @@ export const isArray = (value: unknown): value is T[] => Array.isArray(value) export const isObject = (value: unknown): value is object => value !== null && typeof value === 'object' && !isArray(value) -function isPlainObject(value: unknown): value is Record { - return value !== null && typeof value === 'object' && value.constructor === Object -} - export const isString = (value: unknown): value is string => typeof value === 'string' // source: https://stackoverflow.com/a/77278013/1262753 @@ -24,18 +20,22 @@ export const isEqual = (a: T, b: T): boolean => { ) } -export const isSafeProperty = (object: unknown, prop: string | number): boolean => { - return isPlainObject(object) - ? !(prop in Object.prototype) - : Array.isArray(object) - ? !(prop in Array.prototype) - : false -} - export const getSafeProperty = (object: unknown, prop: string | number): unknown => { - if (object && !isSafeProperty(object, prop)) { - throw new TypeError(`Unsafe property "${prop}"`) + const value = object?.[prop] + if (value === undefined) { + return undefined + } + + // 1. do not allow getting props from the prototype (can be unsafe, like .constructor) + // 2. in case of an array, test if prop is an int + // 3. do not allow getting props from a string or number for example + if ( + !Object.hasOwn(object as object, prop) || + (Array.isArray(object) && !/^\d+$/.test(prop as string)) || + typeof object !== 'object' + ) { + throw new TypeError(`Unsupported property "${prop}"`) } - return object?.[prop] + return value } diff --git a/test-suite/compile.test.json b/test-suite/compile.test.json index f540af2..650dabe 100644 --- a/test-suite/compile.test.json +++ b/test-suite/compile.test.json @@ -223,6 +223,14 @@ } ] }, + { + "category": "get", + "description": "should get a path that has the same name as an unsupported property", + "tests": [ + { "input": { "length": 42 }, "query": ["get", "length"], "output": 42 }, + { "input": { "constructor": "yes" }, "query": ["get", "constructor"], "output": "yes" } + ] + }, { "category": "get", "description": "should get a nested value that has the same name as a function",