From 8fe0e650457a8c22cf4eef841cc67e2907e5252d Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Thu, 14 Aug 2025 11:19:18 +0200 Subject: [PATCH] refactor: rewrite sanitize to support nested objects (@fehmer) (#6875) --- frontend/__tests__/utils/misc.spec.ts | 131 +----- frontend/__tests__/utils/sanitize.spec.ts | 372 ++++++++++++++++++ .../src/ts/elements/account/result-filters.ts | 11 +- frontend/src/ts/utils/config.ts | 3 +- frontend/src/ts/utils/misc.ts | 75 ---- frontend/src/ts/utils/sanitize.ts | 97 +++++ 6 files changed, 475 insertions(+), 214 deletions(-) create mode 100644 frontend/__tests__/utils/sanitize.spec.ts create mode 100644 frontend/src/ts/utils/sanitize.ts diff --git a/frontend/__tests__/utils/misc.spec.ts b/frontend/__tests__/utils/misc.spec.ts index f9b554f01..25fc22d70 100644 --- a/frontend/__tests__/utils/misc.spec.ts +++ b/frontend/__tests__/utils/misc.spec.ts @@ -1,10 +1,4 @@ -import { z } from "zod"; -import { - deepClone, - getErrorMessage, - isObject, - sanitize, -} from "../../src/ts/utils/misc"; +import { deepClone, getErrorMessage, isObject } from "../../src/ts/utils/misc"; import { getLanguageDisplayString, removeLanguageSize, @@ -233,127 +227,4 @@ describe("misc.ts", () => { }); }); }); - describe("sanitize function", () => { - const schema = z - .object({ - name: z.string(), - age: z.number().positive(), - tags: z.array(z.string()), - enumArray: z.array(z.enum(["one", "two"])).min(2), - }) - .partial() - .strip(); - - it("should return the same object if it is valid", () => { - const obj = { name: "Alice", age: 30, tags: ["developer", "coder"] }; - expect(sanitize(schema, obj)).toEqual(obj); - }); - - it("should remove properties with invalid values", () => { - const obj = { name: "Alice", age: -5, tags: ["developer", "coder"] }; - expect(sanitize(schema, obj)).toEqual({ - name: "Alice", - tags: ["developer", "coder"], - age: undefined, - }); - }); - - it("should remove invalid array elements", () => { - const obj = { - name: "Alice", - age: 30, - tags: ["developer", 123, "coder"] as any, - }; - expect(sanitize(schema, obj)).toEqual({ - name: "Alice", - age: 30, - tags: ["developer", "coder"], - }); - }); - - it("should remove invalid array elements with min size", () => { - const schema = z - .object({ - name: z.string(), - tags: z.array(z.enum(["coder", "developer"])).min(2), - }) - .partial(); - const obj = { - name: "Alice", - tags: ["developer", "unknown"] as any, - }; - expect(sanitize(schema, obj)).toEqual({ - name: "Alice", - }); - }); - - it("should remove entire property if all array elements are invalid", () => { - const obj = { name: "Alice", age: 30, tags: [123, 456] as any }; - const sanitized = sanitize(schema, obj); - expect(sanitized).toEqual({ - name: "Alice", - age: 30, - }); - expect(sanitized).not.toHaveProperty("tags"); - }); - - it("should remove object properties if they are invalid", () => { - const obj = { name: 123 as any, age: 30, tags: ["developer", "coder"] }; - const sanitized = sanitize(schema, obj); - expect(sanitized).toEqual({ - age: 30, - tags: ["developer", "coder"], - }); - expect(sanitized).not.toHaveProperty("name"); - }); - - it("should remove nested objects if not valid", () => { - //GIVEN - const schema = z - .object({ - name: z.string(), - info: z.object({ age: z.number() }).partial(), - }) - .partial(); - - const obj = { - name: "Alice", - info: { age: "42" as any }, - }; - //WHEN / THEN - expect(sanitize(schema, obj)).toEqual({ - name: "Alice", - }); - }); - - it("should strip extra keys", () => { - const obj = { - name: "bob", - age: 30, - tags: ["developer", "coder"], - powerLevel: 9001, - } as any; - const stripped = sanitize(schema.strip(), obj); - expect(stripped).not.toHaveProperty("powerLevel"); - }); - it("should strip extra keys on error", () => { - const obj = { - name: "bob", - age: 30, - powerLevel: 9001, - } as any; - const stripped = sanitize(schema.strip(), obj); - expect(stripped).not.toHaveProperty("powerLevel"); - }); - it("should provide a readable error message", () => { - const obj = { - arrayOneTwo: ["one", "nonexistent"], - } as any; - expect(() => { - sanitize(schema.required().strip(), obj); - }).toThrowError( - "unable to sanitize: name: Required, age: Required, tags: Required, enumArray: Required" - ); - }); - }); }); diff --git a/frontend/__tests__/utils/sanitize.spec.ts b/frontend/__tests__/utils/sanitize.spec.ts new file mode 100644 index 000000000..fe2b426ea --- /dev/null +++ b/frontend/__tests__/utils/sanitize.spec.ts @@ -0,0 +1,372 @@ +import { describe, it, expect } from "vitest"; +import { z } from "zod"; +import { sanitize } from "../../src/ts/utils/sanitize"; + +describe("sanitize function", () => { + describe("arrays", () => { + const numberArraySchema = z.array(z.number()); + const numbersArrayMin2Schema = numberArraySchema.min(2); + + const testCases: { + input: number[]; + expected: { + numbers: number[] | boolean; + numbersMin: number[] | boolean; + }; + }[] = [ + { input: [], expected: { numbers: true, numbersMin: false } }, + { input: [1, 2], expected: { numbers: true, numbersMin: true } }, + { + input: [1, "2" as any], + expected: { numbers: [1], numbersMin: false }, + }, + { + input: ["one", "two"] as any, + expected: { numbers: [], numbersMin: false }, + }, + ]; + it.for(testCases)("number array with $input", ({ input, expected }) => { + const sanitized = expect( + expected.numbers === false + ? () => sanitize(numberArraySchema, input) + : sanitize(numberArraySchema, input) + ); + + if (expected.numbers === false) { + sanitized.toThrowError(); + } else if (expected.numbers === true) { + sanitized.toStrictEqual(input); + } else { + sanitized.toStrictEqual(expected.numbers); + } + }); + it.for(testCases)( + "number array.min(2) with $input", + ({ input, expected }) => { + const sanitized = expect( + expected.numbersMin === false + ? () => sanitize(numbersArrayMin2Schema, input) + : sanitize(numbersArrayMin2Schema, input) + ); + + if (expected.numbersMin === false) { + sanitized.toThrowError(); + } else if (expected.numbersMin === true) { + sanitized.toStrictEqual(input); + } else { + sanitized.toStrictEqual(expected.numbersMin); + } + } + ); + }); + describe("objects", () => { + const objectSchema = z.object({ + name: z.string(), + age: z.number().positive(), + tags: z.array(z.string()), + enumArray: z.array(z.enum(["one", "two"])).min(2), + }); + const objectSchemaFullPartial = objectSchema.partial().strip(); + const objectSchemaWithOptional = objectSchema.partial({ + tags: true, + enumArray: true, + }); + + const testCases: { + input: z.infer; + expected: { + mandatory: z.infer | boolean; + partial: z.infer | boolean; + optional: z.infer | boolean; + }; + }[] = [ + { + input: {}, + expected: { mandatory: false, partial: true, optional: false }, + }, + { + input: { + name: "Alice", + age: 23, + tags: ["one", "two"], + enumArray: ["one", "two"], + }, + expected: { mandatory: true, partial: true, optional: true }, + }, + { + input: { + name: "Alice", + age: 23, + }, + expected: { mandatory: false, partial: true, optional: true }, + }, + { + input: { + name: "Alice", + age: "sixty" as any, + }, + expected: { + mandatory: false, + partial: { name: "Alice" }, + optional: false, + }, + }, + { + input: { + name: "Alice", + age: 23, + tags: ["one", 2 as any], + enumArray: "one" as any, + }, + expected: { + mandatory: false, + partial: { name: "Alice", age: 23, tags: ["one"] }, + optional: { name: "Alice", age: 23, tags: ["one"] }, + }, + }, + { + input: { + name: "Alice", + age: 23, + tags: [1, 2] as any, + enumArray: [1, 2] as any, + }, + expected: { + mandatory: false, + partial: { name: "Alice", age: 23 }, + optional: { name: "Alice", age: 23 }, + }, + }, + { + input: { + name: "Alice", + age: 23, + extraArray: [], + extraObject: {}, + extraString: "", + } as any, + expected: { + mandatory: false, + partial: { name: "Alice", age: 23 }, + optional: { name: "Alice", age: 23 }, + }, + }, + ]; + + it.for(testCases)("object mandatory with $input", ({ input, expected }) => { + const sanitized = expect( + expected.mandatory === false + ? () => sanitize(objectSchema, input as any) + : sanitize(objectSchema, input as any) + ); + + if (expected.mandatory === false) { + sanitized.toThrowError(); + } else if (expected.mandatory === true) { + sanitized.toStrictEqual(input); + } else { + sanitized.toStrictEqual(expected.mandatory); + } + }); + it.for(testCases)( + "object full partial with $input", + ({ input, expected }) => { + const sanitized = expect( + expected.partial === false + ? () => sanitize(objectSchemaFullPartial, input as any) + : sanitize(objectSchemaFullPartial, input as any) + ); + + if (expected.partial === false) { + sanitized.toThrowError(); + } else if (expected.partial === true) { + sanitized.toStrictEqual(input); + } else { + sanitized.toStrictEqual(expected.partial); + } + } + ); + it.for(testCases)("object optional with $input", ({ input, expected }) => { + const sanitized = expect( + expected.optional === false + ? () => sanitize(objectSchemaWithOptional, input as any) + : sanitize(objectSchemaWithOptional, input as any) + ); + + if (expected.optional === false) { + sanitized.toThrowError(); + } else if (expected.optional === true) { + sanitized.toStrictEqual(input); + } else { + sanitized.toStrictEqual(expected.optional); + } + }); + }); + + describe("nested", () => { + const itemSchema = z.object({ + name: z.string(), + enumArray: z.array(z.enum(["one", "two"])).min(2), + }); + const nestedSchema = z.object({ + nested: z.array(itemSchema), + }); + + const nestedSchemaFullPartial = z + .object({ + nested: z.array(itemSchema.partial()), + }) + .partial(); + const nestedSchemaWithMin2Array = z.object({ + nested: z.array(itemSchema).min(2), + }); + + const testCases: { + input: z.infer; + expected: { + mandatory: z.infer | boolean; + partial: z.infer | boolean; + minArray: z.infer | boolean; + }; + }[] = [ + { + input: {} as any, + expected: { + mandatory: false, + partial: true, + minArray: false, + }, + }, + { + input: { + nested: [ + { name: "Alice", enumArray: ["one", "two"] }, + { name: "Bob", enumArray: ["one", "two"] }, + ], + }, + expected: { + mandatory: true, + partial: true, + minArray: true, + }, + }, + { + input: { + nested: [ + { name: "Alice", enumArray: ["one", "two"] }, + { name: "Bob" } as any, + ], + }, + expected: { + mandatory: { + nested: [{ name: "Alice", enumArray: ["one", "two"] }], + }, + partial: true, + minArray: false, + }, + }, + { + input: { + nested: [ + { enumArray: ["one", "two"] } as any, + { name: "Bob" } as any, + ], + }, + expected: { + mandatory: false, + partial: true, + minArray: false, + }, + }, + ]; + + it.for(testCases)("nested mandatory with $input", ({ input, expected }) => { + const sanitized = expect( + expected.mandatory === false + ? () => sanitize(nestedSchema, input as any) + : sanitize(nestedSchema, input as any) + ); + + if (expected.mandatory === false) { + sanitized.toThrowError(); + } else if (expected.mandatory === true) { + sanitized.toStrictEqual(input); + } else { + sanitized.toStrictEqual(expected.mandatory); + } + }); + it.for(testCases)("nested partial with $input", ({ input, expected }) => { + const sanitized = expect( + expected.partial === false + ? () => sanitize(nestedSchemaFullPartial, input as any) + : sanitize(nestedSchemaFullPartial, input as any) + ); + + if (expected.partial === false) { + sanitized.toThrowError(); + } else if (expected.partial === true) { + sanitized.toStrictEqual(input); + } else { + sanitized.toStrictEqual(expected.partial); + } + }); + it.for(testCases)( + "nested array min(2) with $input", + ({ input, expected }) => { + const sanitized = expect( + expected.minArray === false + ? () => sanitize(nestedSchemaWithMin2Array, input as any) + : sanitize(nestedSchemaWithMin2Array, input as any) + ); + + if (expected.minArray === false) { + sanitized.toThrowError(); + } else if (expected.minArray === true) { + sanitized.toStrictEqual(input); + } else { + sanitized.toStrictEqual(expected.minArray); + } + } + ); + }); + + const schema = z + .object({ + name: z.string(), + age: z.number().positive(), + tags: z.array(z.string()), + enumArray: z.array(z.enum(["one", "two"])).min(2), + }) + .partial() + .strip(); + + it("should strip extra keys", () => { + const obj = { + name: "bob", + age: 30, + tags: ["developer", "coder"], + powerLevel: 9001, + } as any; + const stripped = sanitize(schema.strip(), obj); + expect(stripped).not.toHaveProperty("powerLevel"); + }); + it("should strip extra keys on error", () => { + const obj = { + name: "bob", + age: 30, + powerLevel: 9001, + } as any; + const stripped = sanitize(schema.strip(), obj); + expect(stripped).not.toHaveProperty("powerLevel"); + }); + it("should provide a readable error message", () => { + const obj = { + arrayOneTwo: ["one", "nonexistent"], + } as any; + expect(() => { + sanitize(schema.required().strip(), obj); + }).toThrowError( + "unable to sanitize: name: Required, age: Required, tags: Required, enumArray: Required" + ); + }); +}); diff --git a/frontend/src/ts/elements/account/result-filters.ts b/frontend/src/ts/elements/account/result-filters.ts index dc075aadc..b0e3733b1 100644 --- a/frontend/src/ts/elements/account/result-filters.ts +++ b/frontend/src/ts/elements/account/result-filters.ts @@ -18,6 +18,7 @@ import defaultResultFilters from "../../constants/default-result-filters"; import { getAllFunboxes } from "@monkeytype/funbox"; import { Snapshot, SnapshotUserTag } from "../../constants/default-snapshot"; import { LanguageList } from "../../constants/languages"; +import { sanitize } from "../../utils/sanitize"; export function mergeWithDefaultFilters( filters: Partial @@ -56,10 +57,7 @@ const resultFiltersLS = new LocalStorageWithSchema({ return defaultResultFilters; } return mergeWithDefaultFilters( - Misc.sanitize( - ResultFiltersSchema.partial().strip(), - unknown as ResultFilters - ) + sanitize(ResultFiltersSchema.partial().strip(), unknown as ResultFilters) ); }, }); @@ -930,10 +928,7 @@ $(".group.presetFilterButtons .filterBtns").on( function verifyResultFiltersStructure(filterIn: ResultFilters): ResultFilters { const filter = mergeWithDefaultFilters( - Misc.sanitize( - ResultFiltersSchema.partial().strip(), - Misc.deepClone(filterIn) - ) + sanitize(ResultFiltersSchema.partial().strip(), Misc.deepClone(filterIn)) ); return filter; diff --git a/frontend/src/ts/utils/config.ts b/frontend/src/ts/utils/config.ts index 383611b23..064004b09 100644 --- a/frontend/src/ts/utils/config.ts +++ b/frontend/src/ts/utils/config.ts @@ -4,7 +4,8 @@ import { PartialConfig, FunboxName, } from "@monkeytype/schemas/configs"; -import { sanitize, typedKeys } from "./misc"; +import { typedKeys } from "./misc"; +import { sanitize } from "./sanitize"; import * as ConfigSchemas from "@monkeytype/schemas/configs"; import { getDefaultConfig } from "../constants/default-config"; /** diff --git a/frontend/src/ts/utils/misc.ts b/frontend/src/ts/utils/misc.ts index db833c52d..c42e9a7e8 100644 --- a/frontend/src/ts/utils/misc.ts +++ b/frontend/src/ts/utils/misc.ts @@ -4,7 +4,6 @@ import { lastElementFromArray } from "./arrays"; import { Config } from "@monkeytype/schemas/configs"; import { Mode, Mode2, PersonalBests } from "@monkeytype/schemas/shared"; import { Result } from "@monkeytype/schemas/results"; -import { z } from "zod"; export function whorf(speed: number, wordlen: number): number { return Math.min( @@ -710,80 +709,6 @@ export function debounceUntilResolved( }; } -/** - * Sanitize object. Remove invalid values based on the schema. - * @param schema zod schema - * @param obj object - * @returns sanitized object - */ -export function sanitize( - schema: T, - obj: z.infer -): z.infer { - const validate = schema.safeParse(obj); - - if (validate.success) { - //use the parsed data, not the obj. keys might been removed - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return validate.data; - } - - const errors: Map = new Map(); - for (const error of validate.error.errors) { - const element = error.path[0] as string; - let val = errors.get(element); - if (typeof error.path[1] === "number") { - val = [...(val ?? []), error.path[1]]; - } - errors.set(element, val); - } - - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - const cleanedObject = Object.fromEntries( - Object.entries(obj) - .map(([key, value]) => { - if (!errors.has(key)) { - return [key, value]; - } - - const error = errors.get(key); - - if ( - Array.isArray(value) && - error !== undefined && //error is not on the array itself - error.length < value.length //not all items in the array are invalid - ) { - //some items of the array are invalid - const cleanedArray = value.filter( - (_element, index) => !error.includes(index) - ); - const cleanedArrayValidation = schema.safeParse( - Object.fromEntries([[key, cleanedArray]]) - ); - if (cleanedArrayValidation.success) { - return [key, cleanedArray]; - } else { - return [key, undefined]; - } - } else { - return [key, undefined]; - } - }) - .filter((it) => it[1] !== undefined) - ) as z.infer; - - const cleanValidate = schema.safeParse(cleanedObject); - if (cleanValidate.success) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return cleanValidate.data; - } - - const errorsString = cleanValidate.error.errors - .map((e) => e.path.join(".") + ": " + e.message) - .join(", "); - throw new Error("unable to sanitize: " + errorsString); -} - export function triggerResize(): void { $(window).trigger("resize"); } diff --git a/frontend/src/ts/utils/sanitize.ts b/frontend/src/ts/utils/sanitize.ts new file mode 100644 index 000000000..1b959e668 --- /dev/null +++ b/frontend/src/ts/utils/sanitize.ts @@ -0,0 +1,97 @@ +import { z } from "zod"; +import { deepClone } from "./misc"; + +function removeProblems( + obj: T, + problems: (number | string)[] +): T | undefined { + if (Array.isArray(obj)) { + if (problems.length === obj.length) return undefined; + + return obj.filter((_, index) => !problems.includes(index)) as T; + } else { + const entries = Object.entries(obj); + if (problems.length === entries.length) return undefined; + + return Object.fromEntries( + entries.filter(([key]) => !problems.includes(key)) + ) as T; + } +} + +function getNestedValue(obj: [] | object, path: string[]): [] | object { + //@ts-expect-error can be array or object + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return path.slice(0, -1).reduce((acc, it: string) => acc[it], obj); +} + +/** + * Sanitize object. Remove invalid values based on the schema. + * @param schema zod schema + * @param obj object + * @returns sanitized object + */ +export function sanitize( + schema: T, + obj: z.infer +): z.infer { + const maxAttempts = 2; + let result; + let current = deepClone(obj); + + for (let attempt = 0; attempt <= maxAttempts; attempt++) { + result = schema.safeParse(current); + + if (result.success) { + //use the parsed data, not the obj. keys might been removed + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return result.data as z.infer; + } + if (attempt === maxAttempts) { + //exit loop and throw error + break; + } + const pathsWithProblems = result.error.errors.reduce((acc, { path }) => { + const parent = path.slice(0, -1).join("."); + const element = path.at(-1); + + if (element !== undefined) { + acc.set(parent, [...(acc.get(parent) ?? []), element]); + } + return acc; + }, new Map>()) as Map< + string, //parent path + string[] | number[] //childs with problems + >; + + for (const [pathString, problems] of pathsWithProblems.entries()) { + if (pathString === "") { + current = + removeProblems(current, problems) ?? + (Array.isArray(current) ? [] : {}); + } else { + const path = pathString.split("."); + const parent = getNestedValue(current, path); + const key = path.at(-1) as string; + + //@ts-expect-error can be object or array + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const cleaned = removeProblems(parent[key], problems); + + if (cleaned === undefined) { + //@ts-expect-error can be object or array + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete parent[key]; + } else { + //@ts-expect-error can be object or array + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + parent[key] = cleaned; + } + } + } + } + const errorsString = result?.error.errors + .map((e) => `${e.path.join(".")}: ${e.message}`) + .join(", "); + throw new Error("unable to sanitize: " + errorsString); +}