diff --git a/src/graph/FilterCreator.ts b/src/graph/FilterCreator.ts index 994f76652..eeab3f75a 100644 --- a/src/graph/FilterCreator.ts +++ b/src/graph/FilterCreator.ts @@ -1,8 +1,7 @@ import { Image } from "./Image"; import { FilterExpression, - FilterOperator, - FilterValue, + FilterKey, } from "./FilterExpression"; import { Func } from "../util/Func"; @@ -42,63 +41,72 @@ export class FilterCreator { * indicates whether the image passed the test or not. */ public createFilter(filter: FilterExpression): FilterFunction { - return new Function("node", "return " + this._compile(filter) + ";"); - } - - private _compile(filter: FilterExpression): string { if (filter == null || filter.length <= 1) { - return "true"; + return () => true; } - const operator: FilterOperator = filter[0]; - const operation: string = - operator === "==" ? this._compileComparisonOp("===", filter[1], filter[2], false) : - operator === "!=" ? this._compileComparisonOp("!==", filter[1], filter[2], false) : - operator === ">" || - operator === ">=" || - operator === "<" || - operator === "<=" ? this._compileComparisonOp(operator, filter[1], filter[2], true) : - operator === "in" ? - this._compileInOp(filter[1], filter.slice(2)) : - operator === "!in" ? - this._compileNegation( - this._compileInOp(filter[1], filter.slice(2))) : - operator === "all" ? this._compileLogicalOp(filter.slice(1), "&&") : - "true"; - - return "(" + operation + ")"; - } - - private _compare(a: T, b: T): number { - return a < b ? -1 : a > b ? 1 : 0; + switch (filter[0]) { + case "==": + return this._compileComparisonOp((a, b) => a === b, filter[1], filter[2], false); + case "!=": + return this._compileComparisonOp((a, b) => a !== b, filter[1], filter[2], false); + case ">": + return this._compileComparisonOp((a, b) => a > b, filter[1], filter[2], true); + case ">=": + return this._compileComparisonOp((a, b) => a >= b, filter[1], filter[2], true); + case "<": + return this._compileComparisonOp((a, b) => a < b, filter[1], filter[2], true); + case "<=": + return this._compileComparisonOp((a, b) => a <= b, filter[1], filter[2], true); + case "in": { + const [, property, ...values] = filter; + return this._compileInOp(property, values); + } + case "!in": { + const [, property, ...values] = filter; + return this._compileNegation(this._compileInOp(property, values)); + } + case "all": { + const [, ...filters] = filter; + return this._compileLogicalOpAnd(filters); + } + default: + // tslint:disable-next-line:no-unused-expression + filter satisfies never; // exhaustivity check + return () => true; + } } - private _compileComparisonOp(operator: string, property: string, value: T, checkType: boolean): string { - const left: string = this._compilePropertyReference(property); - const right: string = JSON.stringify(value); - - return (checkType ? "typeof " + left + "===typeof " + right + "&&" : "") + left + operator + right; + private _compileComparisonOp( + operator: (a: T | null, b: T | null) => boolean, + property: FilterKey, + right: T, + checkType: boolean, + ): FilterFunction { + const normalizedRight: T | null = this._normalizeValue(right); + return (x: Image): boolean => { + const left: T = x[property]; + return (checkType ? typeof left === typeof normalizedRight : true) && operator(left, normalizedRight); + }; } - private _compileInOp(property: string, values: T[]): string { - const compare: (a: T, b: T) => number = this._compare; - const left: string = JSON.stringify(values.sort(compare)); - const right: string = this._compilePropertyReference(property); - - return left + ".indexOf(" + right + ")!==-1"; + private _compileInOp(property: FilterKey, values: T[]): FilterFunction { + const normalizedValues: Set = new Set(values.map((value: T): T | null => this._normalizeValue(value))); + return (x: Image): boolean => normalizedValues.has(x[property]); } - private _compileLogicalOp(filters: FilterExpression[], operator: string): string { - const compile: (filter: FilterExpression) => string = this._compile.bind(this); - - return filters.map(compile).join(operator); + private _compileLogicalOpAnd( + filters: FilterExpression[], + ): FilterFunction { + const functions = filters.map(filter => this.createFilter(filter)); + return x => functions.every(f => f(x)); } - private _compileNegation(expression: string): string { - return "!(" + expression + ")"; + private _compileNegation(f: FilterFunction): FilterFunction { + return (x) => !f(x); } - private _compilePropertyReference(property: string): string { - return "node[" + JSON.stringify(property) + "]"; + private _normalizeValue(value: T): T | null { + return typeof value === "number" && !Number.isFinite(value) ? null : value; } } diff --git a/test/graph/FilterCreator.compatibility.test.ts b/test/graph/FilterCreator.compatibility.test.ts new file mode 100644 index 000000000..9d896673f --- /dev/null +++ b/test/graph/FilterCreator.compatibility.test.ts @@ -0,0 +1,101 @@ +import { Image } from "../../src/graph/Image"; +import { FilterExpression } from "../../src/graph/FilterExpression"; +import { FilterCreator, FilterFunction } from "../../src/graph/FilterCreator"; + +describe("FilterCreator security-policy compatibility", () => { + it("should create and evaluate every operator without dynamic code generation", () => { + const creator: FilterCreator = new FilterCreator(); + const image: Image = { capturedAt: 1, sequenceId: "sequence" }; + const expressions: FilterExpression[] = [ + ["==", "capturedAt", 1], + ["!=", "capturedAt", 0], + [">", "capturedAt", 0], + [">=", "capturedAt", 1], + ["<", "capturedAt", 2], + ["<=", "capturedAt", 1], + ["in", "sequenceId", "other", "sequence"], + ["!in", "sequenceId", "other"], + ["all", [">", "capturedAt", 0], ["==", "sequenceId", "sequence"]], + ]; + const results: boolean[] = []; + const functionSpy: jest.SpyInstance = jest.spyOn(global, "Function").mockImplementation(() => { + throw new EvalError("Dynamic code generation is disabled"); + }); + const evalSpy: jest.SpyInstance = jest.spyOn(global, "eval").mockImplementation(() => { + throw new EvalError("Dynamic code generation is disabled"); + }); + + try { + for (const expression of expressions) { + const filter: FilterFunction = creator.createFilter(expression); + results.push(filter(image)); + } + results.push(creator.createFilter(null)(image)); + results.push(creator.createFilter(undefined)(image)); + results.push(creator.createFilter(["all"])(image)); + } finally { + evalSpy.mockRestore(); + functionSpy.mockRestore(); + } + + expect(results).toEqual(expressions.map(() => true).concat([true, true, true])); + }); + + it("should short-circuit all when an earlier expression is false", () => { + const creator: FilterCreator = new FilterCreator(); + const image: Image = { + capturedAt: 0, + get sequenceId(): string { + throw new Error("The later expression must not be evaluated"); + }, + }; + const filter: FilterFunction = creator.createFilter([ + "all", [">", "capturedAt", 0], ["==", "sequenceId", "sequence"], + ]); + + expect(filter(image)).toBe(false); + }); + + it("should preserve string escaping and strict type comparisons", () => { + const creator: FilterCreator = new FilterCreator(); + const value: string = "quoted\"\\\nvalue"; + const image: Image = { sequenceId: value, capturedAt: 1 }; + + expect(creator.createFilter(["==", "sequenceId", value])(image)).toBe(true); + expect(creator.createFilter(["in", "sequenceId", "other", value])(image)).toBe(true); + expect(creator.createFilter(["==", "capturedAt", "1"])(image)).toBe(false); + expect(creator.createFilter([">=", "capturedAt", "1"])(image)).toBe(false); + }); +}); + +describe("FilterCreator non-finite numeric compatibility", () => { + for (const value of [NaN, Infinity, -Infinity]) { + it(`should preserve null comparisons for ${value}`, () => { + const creator: FilterCreator = new FilterCreator(); + const nullImage: Image = { capturedAt: null }; + const numericImage: Image = { capturedAt: 1 }; + const nonFiniteImage: Image = { capturedAt: value }; + + expect(creator.createFilter(["==", "capturedAt", value])(nullImage)).toBe(true); + expect(creator.createFilter(["!=", "capturedAt", value])(nullImage)).toBe(false); + expect(creator.createFilter([">=", "capturedAt", value])(nullImage)).toBe(true); + expect(creator.createFilter(["<=", "capturedAt", value])(nullImage)).toBe(true); + expect(creator.createFilter([">", "capturedAt", value])(numericImage)).toBe(false); + expect(creator.createFilter(["<", "capturedAt", value])(numericImage)).toBe(false); + expect(creator.createFilter(["==", "capturedAt", value])(nonFiniteImage)).toBe(false); + }); + + it(`should preserve null membership for ${value}`, () => { + const creator: FilterCreator = new FilterCreator(); + const nullImage: Image = { capturedAt: null }; + const numericImage: Image = { capturedAt: 1 }; + const nonFiniteImage: Image = { capturedAt: value }; + + expect(creator.createFilter(["in", "capturedAt", value])(nullImage)).toBe(true); + expect(creator.createFilter(["!in", "capturedAt", value])(nullImage)).toBe(false); + expect(creator.createFilter(["in", "capturedAt", value])(numericImage)).toBe(false); + expect(creator.createFilter(["in", "capturedAt", value])(nonFiniteImage)).toBe(false); + expect(creator.createFilter(["in", "capturedAt", value, 1])(numericImage)).toBe(true); + }); + } +});