From 5a68cf25000991ecde372c3376f1a36d4b0f21fc Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 17 Oct 2024 12:08:29 +0200 Subject: [PATCH 1/6] perf: Faster JSON validation --- src/json.test.ts | 20 ++++++++++++++++++++ src/json.ts | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/json.test.ts b/src/json.test.ts index 2b92d895..ccd823e4 100644 --- a/src/json.test.ts +++ b/src/json.test.ts @@ -418,6 +418,26 @@ describe('json', () => { assertIsJsonRpcRequest(JSON_RPC_REQUEST_FIXTURES.invalid[0]), ).toThrow('Invalid JSON-RPC request: oops'); }); + + it('is fast for large inputs', () => { + const request = { + jsonrpc: '2.0', + id: 'foo', + method: 'wallet_invokeSnap', + params: { + snapId: 'npm:@metamask/manage-state-example-snap', + request: { + method: 'setState', + params: { items: new Array(9999999).fill(2) }, + }, + }, + }; + const now = performance.now(); + expect(() => + assertIsJsonRpcRequest(request), + ).not.toThrow(); + console.log('Asserting took', performance.now() - now); + }); }); describe('isJsonRpcSuccess', () => { diff --git a/src/json.ts b/src/json.ts index 988114ee..f01a44c0 100644 --- a/src/json.ts +++ b/src/json.ts @@ -164,6 +164,48 @@ const finiteNumber = () => return is(value, number()) && Number.isFinite(value); }); +/** + * Validate an unknown input to be valid JSON. + * + * Useful for constructing JSON structs. + * + * @param json - An unknown value. + * @returns True if the value is valid JSON, otherwise false. + */ +function validateJson(json: unknown): boolean { + if (json === null || typeof json === 'boolean' || typeof json === 'string') { + return true; + } + + if (typeof json === 'number' && Number.isFinite(json)) { + return true; + } + + if (typeof json === 'object') { + if (Array.isArray(json)) { + return json.every((value) => validateJson(value)); + } + + const entries = Object.entries(json); + return entries.every( + ([key, value]) => typeof key === 'string' && validateJson(value), + ); + } + + return false; +} + +/** + * A struct to check if the given value is a valid JSON-serializable value. + * + * A faster alternative to {@link UnsafeJsonStruct}. + * + * Note that this struct is unsafe. For safe validation, use {@link JsonStruct}. + */ +export const UnsafeFastJsonStruct: Struct = define('JSON', (json) => + validateJson(json), +); + /** * A struct to check if the given value is a valid JSON-serializable value. * @@ -188,8 +230,8 @@ export const UnsafeJsonStruct: Struct = union([ * This struct sanitizes the value before validating it, so that it is safe to * use with untrusted input. */ -export const JsonStruct = coerce(UnsafeJsonStruct, any(), (value) => { - assertStruct(value, UnsafeJsonStruct); +export const JsonStruct = coerce(UnsafeFastJsonStruct, any(), (value) => { + assertStruct(value, UnsafeFastJsonStruct); return JSON.parse( JSON.stringify(value, (propKey, propValue) => { // Strip __proto__ and constructor properties to prevent prototype pollution. From 9b35af54c44c1a0911df5faf76f0750746b3afca Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 21 Oct 2024 12:29:53 +0200 Subject: [PATCH 2/6] Improve performance further and fix tests --- src/index.test.ts | 1 + src/json.test.ts | 8 ++------ src/json.ts | 24 +++++++++++++++++++----- src/node.test.ts | 1 + 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index 9e36ce70..08a0d657 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -41,6 +41,7 @@ describe('index', () => { "KnownCaipNamespace", "PendingJsonRpcResponseStruct", "StrictHexStruct", + "UnsafeFastJsonStruct", "UnsafeJsonStruct", "VersionRangeStruct", "VersionStruct", diff --git a/src/json.test.ts b/src/json.test.ts index ccd823e4..c4e620a5 100644 --- a/src/json.test.ts +++ b/src/json.test.ts @@ -236,7 +236,7 @@ describe('json', () => { const [error] = validate(undefined, JsonStruct); assert(error !== undefined); expect(error.message).toBe( - 'Expected the value to satisfy a union of `literal | boolean | finite number | string | array | record`, but received: undefined', + 'Expected a value of type `JSON`, but received: `undefined`', ); }); }); @@ -432,11 +432,7 @@ describe('json', () => { }, }, }; - const now = performance.now(); - expect(() => - assertIsJsonRpcRequest(request), - ).not.toThrow(); - console.log('Asserting took', performance.now() - now); + expect(() => assertIsJsonRpcRequest(request)).not.toThrow(); }); }); diff --git a/src/json.ts b/src/json.ts index f01a44c0..f8b75eb6 100644 --- a/src/json.ts +++ b/src/json.ts @@ -182,14 +182,28 @@ function validateJson(json: unknown): boolean { } if (typeof json === 'object') { + let every = true; if (Array.isArray(json)) { - return json.every((value) => validateJson(value)); + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let i = 0; i < json.length; i++) { + if (!validateJson(json[i])) { + every = false; + break; + } + } + return every; } const entries = Object.entries(json); - return entries.every( - ([key, value]) => typeof key === 'string' && validateJson(value), - ); + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let i = 0; i < entries.length; i++) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + if (typeof entries[i]![0] !== 'string' || !validateJson(entries[i]![1])) { + every = false; + break; + } + } + return every; } return false; @@ -197,7 +211,7 @@ function validateJson(json: unknown): boolean { /** * A struct to check if the given value is a valid JSON-serializable value. - * + * * A faster alternative to {@link UnsafeJsonStruct}. * * Note that this struct is unsafe. For safe validation, use {@link JsonStruct}. diff --git a/src/node.test.ts b/src/node.test.ts index 5f7d5ed4..b0cada9b 100644 --- a/src/node.test.ts +++ b/src/node.test.ts @@ -41,6 +41,7 @@ describe('node', () => { "KnownCaipNamespace", "PendingJsonRpcResponseStruct", "StrictHexStruct", + "UnsafeFastJsonStruct", "UnsafeJsonStruct", "VersionRangeStruct", "VersionStruct", From 43a2f026f826e0c72af3d181dac6e539853f9faa Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 21 Oct 2024 12:34:51 +0200 Subject: [PATCH 3/6] Replace old struct --- src/index.test.ts | 1 - src/json.ts | 35 +++-------------------------------- src/node.test.ts | 1 - 3 files changed, 3 insertions(+), 34 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index 08a0d657..9e36ce70 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -41,7 +41,6 @@ describe('index', () => { "KnownCaipNamespace", "PendingJsonRpcResponseStruct", "StrictHexStruct", - "UnsafeFastJsonStruct", "UnsafeJsonStruct", "VersionRangeStruct", "VersionStruct", diff --git a/src/json.ts b/src/json.ts index f8b75eb6..560d1925 100644 --- a/src/json.ts +++ b/src/json.ts @@ -153,17 +153,6 @@ export function exactOptional( }); } -/** - * A struct to check if the given value is finite number. Superstruct's - * `number()` struct does not check if the value is finite. - * - * @returns A struct to check if the given value is finite number. - */ -const finiteNumber = () => - define('finite number', (value) => { - return is(value, number()) && Number.isFinite(value); - }); - /** * Validate an unknown input to be valid JSON. * @@ -216,36 +205,18 @@ function validateJson(json: unknown): boolean { * * Note that this struct is unsafe. For safe validation, use {@link JsonStruct}. */ -export const UnsafeFastJsonStruct: Struct = define('JSON', (json) => +export const UnsafeJsonStruct: Struct = define('JSON', (json) => validateJson(json), ); -/** - * A struct to check if the given value is a valid JSON-serializable value. - * - * Note that this struct is unsafe. For safe validation, use {@link JsonStruct}. - */ -// We cannot infer the type of the struct, because it is recursive. -export const UnsafeJsonStruct: Struct = union([ - literal(null), - boolean(), - finiteNumber(), - string(), - array(lazy(() => UnsafeJsonStruct)), - record( - string(), - lazy(() => UnsafeJsonStruct), - ), -]); - /** * A struct to check if the given value is a valid JSON-serializable value. * * This struct sanitizes the value before validating it, so that it is safe to * use with untrusted input. */ -export const JsonStruct = coerce(UnsafeFastJsonStruct, any(), (value) => { - assertStruct(value, UnsafeFastJsonStruct); +export const JsonStruct = coerce(UnsafeJsonStruct, any(), (value) => { + assertStruct(value, UnsafeJsonStruct); return JSON.parse( JSON.stringify(value, (propKey, propValue) => { // Strip __proto__ and constructor properties to prevent prototype pollution. diff --git a/src/node.test.ts b/src/node.test.ts index b0cada9b..5f7d5ed4 100644 --- a/src/node.test.ts +++ b/src/node.test.ts @@ -41,7 +41,6 @@ describe('node', () => { "KnownCaipNamespace", "PendingJsonRpcResponseStruct", "StrictHexStruct", - "UnsafeFastJsonStruct", "UnsafeJsonStruct", "VersionRangeStruct", "VersionStruct", From 27f781a82d13b6bf8bf86c171a4cec6a9b3a1359 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 21 Oct 2024 12:36:53 +0200 Subject: [PATCH 4/6] Fix lint --- src/json.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/json.ts b/src/json.ts index 560d1925..03f55e75 100644 --- a/src/json.ts +++ b/src/json.ts @@ -1,13 +1,11 @@ import { any, array, - boolean, coerce, create, define, integer, is, - lazy, literal, nullable, number, @@ -201,8 +199,6 @@ function validateJson(json: unknown): boolean { /** * A struct to check if the given value is a valid JSON-serializable value. * - * A faster alternative to {@link UnsafeJsonStruct}. - * * Note that this struct is unsafe. For safe validation, use {@link JsonStruct}. */ export const UnsafeJsonStruct: Struct = define('JSON', (json) => From 2afc97c4a8bd241f51c06525804b0eb029534523 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 21 Oct 2024 12:53:07 +0200 Subject: [PATCH 5/6] Add comments --- src/json.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/json.ts b/src/json.ts index 03f55e75..4281c08f 100644 --- a/src/json.ts +++ b/src/json.ts @@ -171,6 +171,8 @@ function validateJson(json: unknown): boolean { if (typeof json === 'object') { let every = true; if (Array.isArray(json)) { + // Ignoring linting error since for-of is significantly slower than a normal for-loop + // and performance is important in this specific function. // eslint-disable-next-line @typescript-eslint/prefer-for-of for (let i = 0; i < json.length; i++) { if (!validateJson(json[i])) { @@ -182,6 +184,8 @@ function validateJson(json: unknown): boolean { } const entries = Object.entries(json); + // Ignoring linting errors since for-of is significantly slower than a normal for-loop + // and performance is important in this specific function. // eslint-disable-next-line @typescript-eslint/prefer-for-of for (let i = 0; i < entries.length; i++) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion From 6ee6dc326a64da3d231db287497e6e27ae5ee6a1 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 21 Oct 2024 12:54:34 +0200 Subject: [PATCH 6/6] Fix lint --- src/json.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/json.ts b/src/json.ts index 4281c08f..159ce5e3 100644 --- a/src/json.ts +++ b/src/json.ts @@ -171,7 +171,7 @@ function validateJson(json: unknown): boolean { if (typeof json === 'object') { let every = true; if (Array.isArray(json)) { - // Ignoring linting error since for-of is significantly slower than a normal for-loop + // Ignoring linting error since for-of is significantly slower than a normal for-loop // and performance is important in this specific function. // eslint-disable-next-line @typescript-eslint/prefer-for-of for (let i = 0; i < json.length; i++) { @@ -184,7 +184,7 @@ function validateJson(json: unknown): boolean { } const entries = Object.entries(json); - // Ignoring linting errors since for-of is significantly slower than a normal for-loop + // Ignoring linting errors since for-of is significantly slower than a normal for-loop // and performance is important in this specific function. // eslint-disable-next-line @typescript-eslint/prefer-for-of for (let i = 0; i < entries.length; i++) {