From 886bf87bf2237c7ff1e570837ee482de2e7f9781 Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Tue, 1 Nov 2022 16:26:04 +0100 Subject: [PATCH 1/5] Fix JSON validation logic --- .../superstruct-npm-0.16.5-ddced4ae86.patch | 45 +++++++++++++++++++ package.json | 4 +- src/__fixtures__/json.ts | 39 ++-------------- src/json.test.ts | 29 ++++-------- src/json.ts | 27 +++-------- yarn.lock | 10 ++++- 6 files changed, 74 insertions(+), 80 deletions(-) create mode 100644 .yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch diff --git a/.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch b/.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch new file mode 100644 index 00000000..98201d5b --- /dev/null +++ b/.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch @@ -0,0 +1,45 @@ +diff --git a/lib/index.cjs b/lib/index.cjs +index 2a8b0da12dd40c1abfbd6d0c4a83b2d4ad75de84..c0da9d79ae96c186507444966d0da560e70b7bb9 100644 +--- a/lib/index.cjs ++++ b/lib/index.cjs +@@ -74,6 +74,10 @@ function isPlainObject(x) { + */ + + function print(value) { ++ if (typeof value === 'symbol') { ++ return value.toString() ++ } ++ + return typeof value === 'string' ? JSON.stringify(value) : `${value}`; + } + /** +diff --git a/lib/index.mjs b/lib/index.mjs +index 83032e92ecb941d6d5e6966f9ee39cb1da792e5d..417876fe58398e9afee2ad90e3fde45ce02d0644 100644 +--- a/lib/index.mjs ++++ b/lib/index.mjs +@@ -70,6 +70,10 @@ function isPlainObject(x) { + */ + + function print(value) { ++ if (typeof value === 'symbol') { ++ return value.toString() ++ } ++ + return typeof value === 'string' ? JSON.stringify(value) : `${value}`; + } + /** +diff --git a/umd/superstruct.js b/umd/superstruct.js +index ace968357a3ea65107dd966890b342fda553266e..83f1bb4ab55a71adc9a6a0373a1d9aeba354fe8e 100644 +--- a/umd/superstruct.js ++++ b/umd/superstruct.js +@@ -76,6 +76,10 @@ + */ + + function print(value) { ++ if (typeof value === 'symbol') { ++ return value.toString() ++ } ++ + return typeof value === 'string' ? JSON.stringify(value) : `${value}`; + } + /** diff --git a/package.json b/package.json index 3989b198..b497f571 100644 --- a/package.json +++ b/package.json @@ -28,12 +28,12 @@ "test:watch": "jest --watch" }, "resolutions": { - "jest-worker@^28.1.3": "patch:jest-worker@npm%3A28.1.3#./.yarn/patches/jest-worker-npm-28.1.3-5d0ff9006c.patch" + "jest-worker@^28.1.3": "patch:jest-worker@npm%3A28.1.3#./.yarn/patches/jest-worker-npm-28.1.3-5d0ff9006c.patch", + "superstruct@^0.16.5": "patch:superstruct@npm%3A0.16.5#./.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch" }, "dependencies": { "@types/debug": "^4.1.7", "debug": "^4.3.4", - "fast-deep-equal": "^3.1.3", "superstruct": "^0.16.5" }, "devDependencies": { diff --git a/src/__fixtures__/json.ts b/src/__fixtures__/json.ts index aa1b4116..992d20a7 100644 --- a/src/__fixtures__/json.ts +++ b/src/__fixtures__/json.ts @@ -1112,22 +1112,11 @@ export const COMPLEX_OBJECT = { t: [true, true, true], f: [false, false, false], nulls: [null, null, null], - undef: undefined, - mixed: [ - null, - undefined, - null, - undefined, - null, - true, - null, - false, - null, - undefined, - ], + undef: null, + mixed: [null, null, null, null, null, true, null, false, null, null], inObject: { valueOne: null, - valueTwo: undefined, + valueTwo: null, t: true, f: false, }, @@ -1159,25 +1148,3 @@ export const ARRAY_OF_DIFFRENT_KINDS_OF_NUMBERS = [ -5e-11, 5e-9, 0.000000000001, -0.00000000009, 100000.00000008, -100.88888, 0.333, 1000000000000, ]; - -export const ARRAY_OF_MIXED_SPECIAL_OBJECTS = [ - null, - undefined, - null, - undefined, - undefined, - undefined, - null, - null, - null, - undefined, -]; - -export const OBJECT_MIXED_WITH_UNDEFINED_VALUES = { - a: undefined, - b: 'b', - c: undefined, - d: 'd', - e: undefined, - f: 'f', -}; diff --git a/src/json.test.ts b/src/json.test.ts index 98361ab9..677dfda9 100644 --- a/src/json.test.ts +++ b/src/json.test.ts @@ -1,7 +1,6 @@ import * as superstructModule from 'superstruct'; import { ARRAY_OF_DIFFRENT_KINDS_OF_NUMBERS, - ARRAY_OF_MIXED_SPECIAL_OBJECTS, COMPLEX_OBJECT, JSON_FIXTURES, JSON_RPC_ERROR_FIXTURES, @@ -12,7 +11,6 @@ import { JSON_RPC_RESPONSE_FIXTURES, JSON_RPC_SUCCESS_FIXTURES, NON_SERIALIZABLE_NESTED_OBJECT, - OBJECT_MIXED_WITH_UNDEFINED_VALUES, } from './__fixtures__'; import { assertIsJsonRpcFailure, @@ -165,7 +163,7 @@ describe('json', () => { ); it.each(JSON_RPC_SUCCESS_FIXTURES.invalid)( - 'returns false for an invalid JSON-RPC success', + 'returns false for an invalid JSON-RPC success (%p)', (success) => { expect(isJsonRpcSuccess(success)).toBe(false); }, @@ -522,24 +520,15 @@ describe('json', () => { ]); }); - it('should return true for serialization and 2 for a size when only one key with undefined value is provided', () => { + it('should return false for serialization and 0 for a size when only one key with undefined value is provided', () => { const valueToSerialize = { a: undefined, }; - expect(validateJsonAndGetSize(valueToSerialize)).toStrictEqual([true, 2]); - }); - - it('should return true for serialization and 25 for a size when some of the values are undefined', () => { - expect( - validateJsonAndGetSize(OBJECT_MIXED_WITH_UNDEFINED_VALUES), - ).toStrictEqual([true, 25]); - }); - - it('should return true for serialization and 17 for a size with mixed null and undefined in an array', () => { - expect( - validateJsonAndGetSize(ARRAY_OF_MIXED_SPECIAL_OBJECTS), - ).toStrictEqual([true, 51]); + expect(validateJsonAndGetSize(valueToSerialize)).toStrictEqual([ + false, + 0, + ]); }); it('should return true for serialization and 73 for a size, for an array of numbers', () => { @@ -548,10 +537,10 @@ describe('json', () => { ).toStrictEqual([true, 73]); }); - it('should return true for serialization and 1259 for a size of a complex nested object', () => { + it('should return true for serialization and 1288 for a size of a complex nested object', () => { expect(validateJsonAndGetSize(COMPLEX_OBJECT)).toStrictEqual([ true, - 1259, + 1288, ]); }); @@ -761,7 +750,7 @@ describe('json', () => { expect(validateJsonAndGetSize(false, true)).toStrictEqual([true, 0]); expect(validateJsonAndGetSize('str', true)).toStrictEqual([true, 0]); expect(validateJsonAndGetSize(123, true)).toStrictEqual([true, 0]); - expect(validateJsonAndGetSize(undefined, true)).toStrictEqual([true, 0]); + expect(validateJsonAndGetSize(undefined, true)).toStrictEqual([false, 0]); // Value: string escape ASCII const charToJson = { diff --git a/src/json.ts b/src/json.ts index 0f0e0e2f..622d40c4 100644 --- a/src/json.ts +++ b/src/json.ts @@ -1,19 +1,16 @@ -import deepEqual from 'fast-deep-equal'; import { array, assert, - boolean, + define, Infer, integer, is, - lazy, literal, nullable, number, object, omit, optional, - record, string, Struct, union, @@ -37,15 +34,10 @@ function isErrorWithMessage(error: unknown): error is { message: string } { return typeof error === 'object' && error !== null && 'message' in error; } -// Note: This struct references itself, so TypeScript cannot infer the type. -export const JsonStruct: Struct = union([ - literal(null), - boolean(), - number(), - string(), - lazy(() => array(JsonStruct)), - lazy(() => record(string(), JsonStruct)), -]); +export const JsonStruct = define('Json', (value) => { + const [isValid] = validateJsonAndGetSize(value); + return isValid; +}); /** * Any JSON-compatible value. @@ -65,11 +57,7 @@ export type Json = * @returns Whether the value is valid JSON. */ export function isValidJson(value: unknown): value is Json { - try { - return deepEqual(value, JSON.parse(JSON.stringify(value))); - } catch (_) { - return false; - } + return is(value, JsonStruct); } /** @@ -512,8 +500,7 @@ export function validateJsonAndGetSize( skipSizing: boolean, ): [isValid: boolean, plainTextSizeInBytes: number] { if (value === undefined) { - // Return zero for undefined, since these are omitted from JSON serialization - return [true, 0]; + return [false, 0]; } else if (value === null) { // Return already specified constant size for null (special object) return [true, skipSizing ? 0 : JsonSize.Null]; diff --git a/yarn.lock b/yarn.lock index a013a083..76b2ab1c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -871,7 +871,6 @@ __metadata: eslint-plugin-jsdoc: ^36.1.0 eslint-plugin-node: ^11.1.0 eslint-plugin-prettier: ^3.3.1 - fast-deep-equal: ^3.1.3 jest: ^28.1.0 json-bigint: ^1.0.0 prettier: ^2.2.1 @@ -5782,13 +5781,20 @@ __metadata: languageName: node linkType: hard -"superstruct@npm:^0.16.5": +"superstruct@npm:0.16.5": version: 0.16.5 resolution: "superstruct@npm:0.16.5" checksum: 9f843c38695b584a605ae9b028629de18a85bd0dca0e9449b4ab98bb7b9ac3d82599870acbab9fbd2ee454c6b187af7e61562e252dfadabd974191ab4ab2e3ce languageName: node linkType: hard +"superstruct@patch:superstruct@npm%3A0.16.5#./.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch::locator=%40metamask%2Futils%40workspace%3A.": + version: 0.16.5 + resolution: "superstruct@patch:superstruct@npm%3A0.16.5#./.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch::version=0.16.5&hash=01978c&locator=%40metamask%2Futils%40workspace%3A." + checksum: 4d47b10b792d7795e570ec39718713a54783f1e4ae6c93d47f14072c8651fab3145f94b974f62e33be26b8bf3da966e0e5d3ee8ea9e7f901c4b57248fbf08b63 + languageName: node + linkType: hard + "supports-color@npm:^5.3.0": version: 5.5.0 resolution: "supports-color@npm:5.5.0" From ddc773e514a4fc6075c11695a4c0417482eff33d Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Tue, 1 Nov 2022 16:29:47 +0100 Subject: [PATCH 2/5] Remove debug value --- src/json.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/json.test.ts b/src/json.test.ts index 677dfda9..38e26ddb 100644 --- a/src/json.test.ts +++ b/src/json.test.ts @@ -163,7 +163,7 @@ describe('json', () => { ); it.each(JSON_RPC_SUCCESS_FIXTURES.invalid)( - 'returns false for an invalid JSON-RPC success (%p)', + 'returns false for an invalid JSON-RPC success', (success) => { expect(isJsonRpcSuccess(success)).toBe(false); }, From 763897c1fbc1a7300e3ca1351c509576b68a04ef Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Tue, 1 Nov 2022 16:35:14 +0100 Subject: [PATCH 3/5] Skip sizing for validation --- src/json.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/json.ts b/src/json.ts index 622d40c4..50e81927 100644 --- a/src/json.ts +++ b/src/json.ts @@ -35,7 +35,7 @@ function isErrorWithMessage(error: unknown): error is { message: string } { } export const JsonStruct = define('Json', (value) => { - const [isValid] = validateJsonAndGetSize(value); + const [isValid] = validateJsonAndGetSize(value, true); return isValid; }); From be413e452417d3c433db5c6c236c7ae315c0203b Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Tue, 1 Nov 2022 16:48:51 +0100 Subject: [PATCH 4/5] Remove dead code and add array test --- src/json.test.ts | 6 ++++++ src/json.ts | 12 ------------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/json.test.ts b/src/json.test.ts index 38e26ddb..0337b541 100644 --- a/src/json.test.ts +++ b/src/json.test.ts @@ -600,6 +600,12 @@ describe('json', () => { ]); }); + it('returns true for serialization and 37 for a size when checking an array', () => { + expect( + validateJsonAndGetSize(['foo', 'bar', null, ['foo', 'bar', null]]), + ).toStrictEqual([true, 37]); + }); + it('should return true or false for validity depending on the test scenario from ECMA TC39 (test262)', () => { // This test will perform a series of validation assertions. // These tests are taken from ECMA TC39 (test262) test scenarios used diff --git a/src/json.ts b/src/json.ts index 50e81927..431a9b3f 100644 --- a/src/json.ts +++ b/src/json.ts @@ -587,18 +587,6 @@ export function validateJsonAndGetSize( return 0; } - // If the size is 0, the value is undefined and undefined in an array - // when serialized will be replaced with null - if (size === 0 && Array.isArray(value)) { - size = JsonSize.Null; - } - - // If the size is 0, that means the object is undefined and - // the rest of the object structure will be omitted - if (size === 0) { - return sum; - } - // Objects will have be serialized with "key": value, // therefore we include the key in the calculation here const keySize = Array.isArray(value) From ca1d3638bfc44c4b48e3738c6581815ac47f0fc7 Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Wed, 2 Nov 2022 08:35:42 +0100 Subject: [PATCH 5/5] Bump superstruct --- .../superstruct-npm-0.16.5-ddced4ae86.patch | 45 ------------------- package.json | 5 +-- yarn.lock | 17 +++---- 3 files changed, 7 insertions(+), 60 deletions(-) delete mode 100644 .yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch diff --git a/.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch b/.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch deleted file mode 100644 index 98201d5b..00000000 --- a/.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch +++ /dev/null @@ -1,45 +0,0 @@ -diff --git a/lib/index.cjs b/lib/index.cjs -index 2a8b0da12dd40c1abfbd6d0c4a83b2d4ad75de84..c0da9d79ae96c186507444966d0da560e70b7bb9 100644 ---- a/lib/index.cjs -+++ b/lib/index.cjs -@@ -74,6 +74,10 @@ function isPlainObject(x) { - */ - - function print(value) { -+ if (typeof value === 'symbol') { -+ return value.toString() -+ } -+ - return typeof value === 'string' ? JSON.stringify(value) : `${value}`; - } - /** -diff --git a/lib/index.mjs b/lib/index.mjs -index 83032e92ecb941d6d5e6966f9ee39cb1da792e5d..417876fe58398e9afee2ad90e3fde45ce02d0644 100644 ---- a/lib/index.mjs -+++ b/lib/index.mjs -@@ -70,6 +70,10 @@ function isPlainObject(x) { - */ - - function print(value) { -+ if (typeof value === 'symbol') { -+ return value.toString() -+ } -+ - return typeof value === 'string' ? JSON.stringify(value) : `${value}`; - } - /** -diff --git a/umd/superstruct.js b/umd/superstruct.js -index ace968357a3ea65107dd966890b342fda553266e..83f1bb4ab55a71adc9a6a0373a1d9aeba354fe8e 100644 ---- a/umd/superstruct.js -+++ b/umd/superstruct.js -@@ -76,6 +76,10 @@ - */ - - function print(value) { -+ if (typeof value === 'symbol') { -+ return value.toString() -+ } -+ - return typeof value === 'string' ? JSON.stringify(value) : `${value}`; - } - /** diff --git a/package.json b/package.json index b497f571..40005903 100644 --- a/package.json +++ b/package.json @@ -28,13 +28,12 @@ "test:watch": "jest --watch" }, "resolutions": { - "jest-worker@^28.1.3": "patch:jest-worker@npm%3A28.1.3#./.yarn/patches/jest-worker-npm-28.1.3-5d0ff9006c.patch", - "superstruct@^0.16.5": "patch:superstruct@npm%3A0.16.5#./.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch" + "jest-worker@^28.1.3": "patch:jest-worker@npm%3A28.1.3#./.yarn/patches/jest-worker-npm-28.1.3-5d0ff9006c.patch" }, "dependencies": { "@types/debug": "^4.1.7", "debug": "^4.3.4", - "superstruct": "^0.16.5" + "superstruct": "^0.16.7" }, "devDependencies": { "@lavamoat/allow-scripts": "^2.0.3", diff --git a/yarn.lock b/yarn.lock index 76b2ab1c..ac159c4b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -877,7 +877,7 @@ __metadata: prettier-plugin-packagejson: ^2.2.11 rimraf: ^3.0.2 stdio-mock: ^1.2.0 - superstruct: ^0.16.5 + superstruct: ^0.16.7 ts-jest: ^28.0.8 tsd: ^0.24.1 typedoc: ^0.23.10 @@ -5781,17 +5781,10 @@ __metadata: languageName: node linkType: hard -"superstruct@npm:0.16.5": - version: 0.16.5 - resolution: "superstruct@npm:0.16.5" - checksum: 9f843c38695b584a605ae9b028629de18a85bd0dca0e9449b4ab98bb7b9ac3d82599870acbab9fbd2ee454c6b187af7e61562e252dfadabd974191ab4ab2e3ce - languageName: node - linkType: hard - -"superstruct@patch:superstruct@npm%3A0.16.5#./.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch::locator=%40metamask%2Futils%40workspace%3A.": - version: 0.16.5 - resolution: "superstruct@patch:superstruct@npm%3A0.16.5#./.yarn/patches/superstruct-npm-0.16.5-ddced4ae86.patch::version=0.16.5&hash=01978c&locator=%40metamask%2Futils%40workspace%3A." - checksum: 4d47b10b792d7795e570ec39718713a54783f1e4ae6c93d47f14072c8651fab3145f94b974f62e33be26b8bf3da966e0e5d3ee8ea9e7f901c4b57248fbf08b63 +"superstruct@npm:^0.16.7": + version: 0.16.7 + resolution: "superstruct@npm:0.16.7" + checksum: c8c855ff6945da8a41048c6d236de7b1af5d4d9c31742b3ee54d65647c31597488620281f65e095d5efc9e2fbdaad529b8c8f2506c12569d428467c835a21477 languageName: node linkType: hard