remove eval()

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 <FilterFunction>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 = <FilterOperator>filter[0];
-        const operation: string =
-            operator === "==" ? this._compileComparisonOp("===", <string>filter[1], filter[2], false) :
-                operator === "!=" ? this._compileComparisonOp("!==", <string>filter[1], filter[2], false) :
-                    operator === ">" ||
-                        operator === ">=" ||
-                        operator === "<" ||
-                        operator === "<=" ? this._compileComparisonOp(operator, <string>filter[1], filter[2], true) :
-                        operator === "in" ?
-                            this._compileInOp<FilterValue>(<string>filter[1], <FilterValue[]>filter.slice(2)) :
-                            operator === "!in" ?
-                                this._compileNegation(
-                                    this._compileInOp<FilterValue>(<string>filter[1], <FilterValue[]>filter.slice(2))) :
-                                operator === "all" ? this._compileLogicalOp(<FilterExpression[]>filter.slice(1), "&&") :
-                                    "true";
-
-        return "(" + operation + ")";
-    }
-
-    private _compare<T>(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<T>(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<T>(
+        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 = <T>x[property];
+            return (checkType ? typeof left === typeof normalizedRight : true) && operator(left, normalizedRight);
+        };
     }
 
-    private _compileInOp<T>(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<T>(property: FilterKey, values: T[]): FilterFunction {
+        const normalizedValues: Set<T | null> = new Set(values.map((value: T): T | null => this._normalizeValue(value)));
+        return (x: Image): boolean => normalizedValues.has(<T>x[property]);
     }
 
-    private _compileLogicalOp(filters: FilterExpression[], operator: string): string {
-        const compile: (filter: FilterExpression) => string = this._compile.bind(this);
-
-        return filters.map<string>(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<T>(value: T): T | null {
+        return typeof value === "number" && !Number.isFinite(value) ? null : value;
     }
 }

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 = <Image><unknown>{ 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 = <Image><unknown>{
+            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 = <Image><unknown>{ 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 = <Image><unknown>{ capturedAt: null };
+            const numericImage: Image = <Image><unknown>{ capturedAt: 1 };
+            const nonFiniteImage: Image = <Image><unknown>{ 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 = <Image><unknown>{ capturedAt: null };
+            const numericImage: Image = <Image><unknown>{ capturedAt: 1 };
+            const nonFiniteImage: Image = <Image><unknown>{ 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);
+        });
+    }
+});