fix(core): Fix potential stack overflow with filter comparison

doug-martin · Aug 14, 2020 · f498802 · f498802
1 parent 3582ed2
commit f498802
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 16 deletions.
diff --git a/packages/core/__tests__/helpers.spec.ts b/packages/core/__tests__/helpers.spec.ts
@@ -314,6 +314,14 @@ describe('applyFilter', () => {
     expect(applyFilter({ first: 'e', last: 'bar' }, filter)).toBe(true);
   });
 
+  it('should throw an error for an unknown operator', () => {
+    const filter: Filter<TestDTO> = {
+      // @ts-ignore
+      first: { foo: 'bar' },
+    };
+    expect(() => applyFilter({ first: 'baz', last: 'kaz' }, filter)).toThrow('unknown comparison "foo"');
+  });
+
   it('should handle and grouping', () => {
     const filter: Filter<TestDTO> = {
       and: [{ first: { eq: 'foo' } }, { last: { like: '%bar' } }],

diff --git a/packages/core/src/helpers/comparison.builder.ts b/packages/core/src/helpers/comparison.builder.ts
@@ -1,4 +1,9 @@
-import { CommonFieldComparisonBetweenType, FilterComparisonOperators, Filter } from '../interfaces';
+import {
+  CommonFieldComparisonBetweenType,
+  FilterComparisonOperators,
+  Filter,
+  FilterFieldComparison,
+} from '../interfaces';
 import { ComparisonField, FilterFn } from './types';
 
 type LikeComparisonOperators = 'like' | 'notLike' | 'iLike' | 'notILike';
@@ -36,7 +41,9 @@ const isBooleanComparisonOperators = (op: any): op is BooleanComparisonOperators
   return op === 'eq' || op === 'neq' || op === 'is' || op === 'isNot';
 };
 
-export const isComparison = <DTO>(maybeComparison: Filter<DTO>[keyof DTO]): boolean => {
+export const isComparison = <DTO, K extends keyof DTO>(
+  maybeComparison: FilterFieldComparison<DTO[K]> | Filter<DTO[K]>,
+): maybeComparison is FilterFieldComparison<DTO[K]> => {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   return Object.keys(maybeComparison as Record<string, any>).every((op) => {
     return (

diff --git a/packages/core/src/helpers/filter.builder.ts b/packages/core/src/helpers/filter.builder.ts
@@ -7,15 +7,16 @@ export class FilterBuilder {
     const { and, or } = filter;
     const filters: FilterFn<DTO>[] = [];
 
-    if (and) {
+    if (and && and.length) {
       filters.push(this.andFilterFn(...and.map((f) => this.build(f))));
     }
 
-    if (or) {
+    if (or && or.length) {
       filters.push(this.orFilterFn(...or.map((f) => this.build(f))));
     }
-
-    filters.push(this.filterFieldsOrNested(filter));
+    if (Object.keys(filter).length) {
+      filters.push(this.filterFieldsOrNested(filter));
+    }
     return this.andFilterFn(...filters);
   }
 
@@ -31,16 +32,7 @@ export class FilterBuilder {
     return this.andFilterFn(
       ...Object.keys(filter)
         .filter((k) => k !== 'and' && k !== 'or')
-        .map((fieldOrNested) => {
-          const value = this.getField(filter as FilterComparisons<DTO>, fieldOrNested as keyof DTO);
-
-          if (isComparison(filter[fieldOrNested as keyof DTO])) {
-            return this.withFilterComparison(fieldOrNested as keyof DTO, value);
-          }
-
-          const nestedFilterFn = this.build(value);
-          return (dto?: DTO) => nestedFilterFn(dto ? dto[fieldOrNested as keyof DTO] : null);
-        }),
+        .map((fieldOrNested) => this.withComparison(filter, fieldOrNested as keyof DTO)),
     );
   }
 
@@ -62,4 +54,16 @@ export class FilterBuilder {
       ),
     );
   }
+
+  private static withComparison<DTO>(filter: FilterComparisons<DTO>, fieldOrNested: keyof DTO): FilterFn<DTO> {
+    const value = this.getField(filter, fieldOrNested);
+    if (isComparison(value)) {
+      return this.withFilterComparison(fieldOrNested, value);
+    }
+    if (typeof value !== 'object') {
+      throw new Error(`unknown comparison ${JSON.stringify(fieldOrNested)}`);
+    }
+    const nestedFilterFn = this.build(value);
+    return (dto?: DTO) => nestedFilterFn(dto ? dto[fieldOrNested] : null);
+  }
 }