Fix handling of invalid and empty filter queries

.changeset/three-dolls-ring.md

@@ -0,0 +1,5 @@
+---
+'@directus/api': patch
+---
+
+Fixed handling of invalid and empty filter queries

api/src/utils/apply-query.ts

@@ -480,7 +480,11 @@ export function applyFilter(
 
  const { relation, relationType } = getRelationInfo(relations, collection, pathRoot);
 
- const { operator: filterOperator, value: filterValue } = getOperation(key, value);
+ const operation = getOperation(key, value);
+
+ if (!operation) continue;
+
+ const { operator: filterOperator, value: filterValue } = operation;
 
  if (
  filterPath.length > 1 ||
@@ -922,9 +926,9 @@ export function applyAggregate(
 
 function getFilterPath(key: string, value: Record<string, any>) {
  const path = [key];
- const childKey = Object.keys(value)[0]!;
+ const childKey = Object.keys(value)[0];
 
- if (typeof childKey === 'string' && childKey.startsWith('_') === true && !['_none', '_some'].includes(childKey)) {
+ if (!childKey || (childKey.startsWith('_') === true && !['_none', '_some'].includes(childKey))) {
  return path;
  }
 
@@ -935,14 +939,20 @@ function getFilterPath(key: string, value: Record<string, any>) {
  return path;
 }
 
-function getOperation(key: string, value: Record<string, any>): { operator: string; value: any } {
+function getOperation(key: string, value: Record<string, any>): { operator: string; value: any } | null {
  if (key.startsWith('_') && !['_and', '_or', '_none', '_some'].includes(key)) {
- return { operator: key as string, value };
- } else if (isPlainObject(value) === false) {
+ return { operator: key, value };
+ } else if (!isPlainObject(value)) {
  return { operator: '_eq', value };
  }
 
- return getOperation(Object.keys(value)[0]!, Object.values(value)[0]);
+ const childKey = Object.keys(value)[0];
+
+ if (childKey) {
+ return getOperation(childKey, Object.values(value)[0]);
+ }
+
+ return null;
 }
 
 function isNumericField(field: FieldOverview): field is FieldOverview & { type: NumericType } {

api/src/utils/sanitize-query.test.ts

@@ -1,4 +1,5 @@
 import { useEnv } from '@directus/env';
+import { parseFilter, parseJSON } from '@directus/utils';
 import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest';
 import { sanitizeQuery } from './sanitize-query.js';
 
@@ -7,10 +8,11 @@ import { sanitizeQuery } from './sanitize-query.js';
 vi.mock('@directus/env', () => ({ useEnv: vi.fn().mockReturnValue({}) }));
 
 vi.mock('@directus/utils', async () => {
- const actual = (await vi.importActual('@directus/utils')) as any;
+ const actual = await vi.importActual<typeof import('@directus/utils')>('@directus/utils');
 
  return {
  ...actual,
+ parseJSON: vi.fn().mockImplementation(actual.parseJSON),
  parseFilter: vi.fn().mockImplementation((value) => value),
  };
 });
@@ -200,21 +202,41 @@ describe('sort', () => {
 });
 
 describe('filter', () => {
- test('should accept valid value', () => {
+ test('should accept valid filter', () => {
  const filter = { field_a: { _eq: 'test' } };
 
  const sanitizedQuery = sanitizeQuery({ filter });
 
  expect(sanitizedQuery.filter).toEqual({ field_a: { _eq: 'test' } });
  });
 
+ test('should throw error on invalid filter', () => {
+ const filter = { field_a: null };
+
+ vi.mocked(parseFilter).mockImplementationOnce(() => {
+ throw new Error();
+ });
+
+ expect(() => sanitizeQuery({ filter })).toThrowError('Invalid query. Invalid filter object.');
+ });
+
  test('should parse as json when it is a string', () => {
  const filter = '{ "field_a": { "_eq": "test" } }';
 
  const sanitizedQuery = sanitizeQuery({ filter });
 
  expect(sanitizedQuery.filter).toEqual({ field_a: { _eq: 'test' } });
  });
+
+ test('should throw error on invalid json', () => {
+ const filter = '{ "field_a": }';
+
+ vi.mocked(parseJSON).mockImplementationOnce(() => {
+ throw new Error();
+ });
+
+ expect(() => sanitizeQuery({ filter })).toThrowError('Invalid query. Invalid JSON for filter object.');
+ });
 });
 
 describe('offset', () => {

api/src/utils/sanitize-query.ts

@@ -1,5 +1,6 @@
 import { useEnv } from '@directus/env';
-import type { Accountability, Aggregate, Filter, Query } from '@directus/types';
+import { InvalidQueryError } from '@directus/errors';
+import type { Accountability, Aggregate, Query } from '@directus/types';
 import { parseFilter, parseJSON } from '@directus/utils';
 import { flatten, get, isPlainObject, merge, set } from 'lodash-es';
 import { useLogger } from '../logger.js';
@@ -137,19 +138,21 @@ function sanitizeAggregate(rawAggregate: any): Aggregate {
 }
 
 function sanitizeFilter(rawFilter: any, accountability: Accountability | null) {
- const logger = useLogger();
-
- let filters: Filter | null = rawFilter;
+ let filters = rawFilter;
 
- if (typeof rawFilter === 'string') {
+ if (typeof filters === 'string') {
  try {
- filters = parseJSON(rawFilter);
+ filters = parseJSON(filters);
  } catch {
- logger.warn('Invalid value passed for filter query parameter.');
+ throw new InvalidQueryError({ reason: 'Invalid JSON for filter object' });
  }
  }
 
- return parseFilter(filters, accountability);
+ try {
+ return parseFilter(filters, accountability);
+ } catch {
+ throw new InvalidQueryError({ reason: 'Invalid filter object' });
+ }
 }
 
 function sanitizeLimit(rawLimit: any) {

api/src/utils/validate-query.ts

@@ -1,6 +1,6 @@
 import { useEnv } from '@directus/env';
 import { InvalidQueryError } from '@directus/errors';
-import type { Query } from '@directus/types';
+import type { Filter, Query } from '@directus/types';
 import Joi from 'joi';
 import { isPlainObject, uniq } from 'lodash-es';
 import { stringify } from 'wellknown';
@@ -52,9 +52,7 @@ export function validateQuery(query: Query): Query {
  return query;
 }
 
-function validateFilter(filter: Query['filter']) {
- if (!filter) throw new InvalidQueryError({ reason: 'Invalid filter object' });
-
+function validateFilter(filter: Filter) {
  for (const [key, nested] of Object.entries(filter)) {
  if (key === '_and' || key === '_or') {
  nested.forEach(validateFilter);

packages/utils/shared/parse-filter.test.ts

@@ -12,6 +12,22 @@ describe('#parseFilter', () => {
  vi.useRealTimers();
  });
 
+ it('should accept empty filter object', () => {
+ const filter = {};
+
+ const parsedFilter = parseFilter(filter, null);
+
+ expect(parsedFilter).toEqual({});
+ });
+
+ it('should accept empty object for key', () => {
+ const filter = { field_a: {} };
+
+ const parsedFilter = parseFilter(filter, null);
+
+ expect(parsedFilter).toEqual({ field_a: {} });
+ });
+
  it('returns the filter when passed accountability with only a role', () => {
  const mockFilter = { _and: [{ field: { _eq: 'field' } }] } as Filter;
  const mockAccountability = { role: 'admin' };