qs.parse returns invalid key with brackets when using nested array query (#1751)

* add test to fix nested query

* add fix to account for limitations of qs.parse

* Add assertion for key wrapped in square brackets

* update qs library that includes strictDepth=true

* Revert "add test to fix nested query"

This reverts commit a3ea344.

* remove previous fix that manually changes the key. Just use qs configuration

* add qs-test module

* fix lint

* update ts-expect-error for missing strictDepth type

* fix spacing

* Upgrade @types/qs to include strictDepth

Author: tintinthong

packages/host/package.json

@@ -68,7 +68,7 @@
     "@types/matrix-js-sdk": "^11.0.1",
     "@types/ms": "^0.7.34",
     "@types/pluralize": "^0.0.30",
-    "@types/qs": "^6.9.14",
+    "@types/qs": "^6.9.17",
     "@types/qunit": "^2.11.3",
     "@types/rsvp": "^4.0.9",
     "@types/uuid": "^9.0.8",
@@ -159,7 +159,7 @@
     "pluralize": "^8.0.0",
     "prettier": "^3.0.3",
     "prettier-plugin-ember-template-tag": "^1.1.0",
-    "qs": "^6.12.3",
+    "qs": "^6.13.0",
     "qunit": "^2.20.0",
     "qunit-dom": "^2.0.0",
     "safe-stable-stringify": "^2.4.3",

packages/host/tests/unit/qs-test.ts

@@ -0,0 +1,54 @@
+import qs from 'qs';
+import { module, test } from 'qunit';
+
+import { parseQuery, Query } from '@cardstack/runtime-common/query';
+
+module('Unit | qs | parse', function () {
+  test('parseQuery errors out if the query is too deep', async function (assert) {
+    assert.throws(
+      () => parseQuery('a[b][c][d][e][f][g][h][i][j][k][l]=m'),
+      /RangeError: Input depth exceeded depth option of 10 and strictDepth is true/,
+    );
+  });
+  test('invertibility: applying stringify and parse on object will return the same object', async function (assert) {
+    let testRealmURL = 'https://example.com/';
+    let query: Query = {
+      filter: {
+        on: {
+          module: `${testRealmURL}book`,
+          name: 'Book',
+        },
+        every: [
+          {
+            eq: {
+              'author.firstName': 'Cardy',
+            },
+          },
+          {
+            any: [
+              {
+                eq: {
+                  'author.lastName': 'Jones',
+                },
+              },
+              {
+                eq: {
+                  'author.lastName': 'Stackington Jr. III',
+                },
+              },
+            ],
+          },
+        ],
+      },
+      sort: [
+        {
+          by: 'author.lastName',
+          on: { module: `${testRealmURL}book`, name: 'Book' },
+        },
+      ],
+    };
+    let queryString = qs.stringify(query);
+    let parsedQuery: any = parseQuery(queryString);
+    assert.deepEqual(parsedQuery, query);
+  });
+});

packages/realm-server/package.json

@@ -22,7 +22,7 @@
     "@types/mime-types": "^2.1.1",
     "@types/node": "^18.18.5",
     "@types/pg": "^8.11.5",
-    "@types/qs": "^6.9.14",
+    "@types/qs": "^6.9.17",
     "@types/qunit": "^2.11.3",
     "@types/sane": "^2.0.1",
     "@types/supertest": "^2.0.12",
@@ -51,7 +51,7 @@
     "pg": "^8.11.5",
     "prettier": "^2.8.4",
     "prettier-plugin-ember-template-tag": "^1.1.0",
-    "qs": "^6.12.3",
+    "qs": "^6.13.0",
     "qunit": "^2.20.0",
     "sane": "^5.0.1",
     "sql-parser-cst": "^0.28.0",

packages/runtime-common/package.json

@@ -25,7 +25,7 @@
     "@types/jsonwebtoken": "^9.0.5",
     "@types/lodash": "^4.14.182",
     "@types/pluralize": "^0.0.30",
-    "@types/qs": "^6.9.14",
+    "@types/qs": "^6.9.17",
     "@types/uuid": "^9.0.8",
     "babel-import-util": "^1.2.2",
     "babel-plugin-ember-template-compilation": "^2.2.1",
@@ -45,7 +45,7 @@
     "loglevel": "^1.8.1",
     "marked": "^12.0.1",
     "pluralize": "^8.0.0",
-    "qs": "^6.12.3",
+    "qs": "^6.13.0",
     "qunit": "^2.20.0",
     "recast": "^0.23.4",
     "safe-stable-stringify": "^2.4.3",

packages/runtime-common/query.ts

@@ -307,9 +307,9 @@ function assertEveryFilter(
       `${pointer.join('/') || '/'}: every must be an array of Filters`,
     );
   } else {
-    filter.every.every((value: any, index: number) =>
-      assertFilter(value, pointer.concat(`[${index}]`)),
-    );
+    filter.every.forEach((value: any, index: number) => {
+      assertFilter(value, pointer.concat(`[${index}]`));
+    });
   }
 }
 
@@ -348,9 +348,10 @@ function assertEqFilter(
   if (typeof filter.eq !== 'object' || filter.eq == null) {
     throw new Error(`${pointer.join('/') || '/'}: eq must be an object`);
   }
-  Object.entries(filter.eq).every(([key, value]) =>
-    assertJSONValue(value, pointer.concat(key)),
-  );
+  Object.entries(filter.eq).forEach(([key, value]) => {
+    assertKey(key, pointer);
+    assertJSONValue(value, pointer.concat(key));
+  });
 }
 
 function assertContainsFilter(
@@ -371,9 +372,10 @@ function assertContainsFilter(
   if (typeof filter.contains !== 'object' || filter.contains == null) {
     throw new Error(`${pointer.join('/') || '/'}: contains must be an object`);
   }
-  Object.entries(filter.contains).every(([key, value]) =>
-    assertJSONValue(value, pointer.concat(key)),
-  );
+  Object.entries(filter.contains).forEach(([key, value]) => {
+    assertKey(key, pointer);
+    assertJSONValue(value, pointer.concat(key));
+  });
 }
 
 function assertRangeFilter(
@@ -419,3 +421,15 @@ function assertRangeFilter(
     });
   });
 }
+
+export function assertKey(key: string, pointer: string[]) {
+  if (key.startsWith('[') && key.endsWith(']')) {
+    throw new Error(
+      `${pointer.join('/')}: field names cannot be wrapped in brackets: ${key}`,
+    );
+  }
+}
+
+export const parseQuery = (queryString: string) => {
+  return qs.parse(queryString, { depth: 10, strictDepth: true });
+};

packages/runtime-common/realm.ts

@@ -59,7 +59,7 @@ import {
   SupportedMimeType,
   lookupRouteTable,
 } from './router';
-import { assertQuery } from './query';
+import { assertQuery, parseQuery } from './query';
 import type { Readable } from 'stream';
 import { type CardDef } from 'https://cardstack.com/base/card-api';
 import type * as CardAPI from 'https://cardstack.com/base/card-api';
@@ -76,7 +76,6 @@ import { fetcher } from './fetcher';
 import { RealmIndexQueryEngine } from './realm-index-query-engine';
 import { RealmIndexUpdater } from './realm-index-updater';
 
-import qs from 'qs';
 import {
   MatrixBackendAuthentication,
   Utils,
@@ -1548,7 +1547,7 @@ export class Realm {
       request.headers.get('X-Boxel-Building-Index'),
     );
 
-    let cardsQuery = qs.parse(new URL(request.url).search.slice(1));
+    let cardsQuery = parseQuery(new URL(request.url).search.slice(1));
     assertQuery(cardsQuery);
 
     let doc = await this.#realmIndexQueryEngine.search(cardsQuery, {
@@ -1572,7 +1571,8 @@ export class Realm {
       request.headers.get('X-Boxel-Building-Index'),
     );
 
-    let parsedQueryString = qs.parse(new URL(request.url).search.slice(1));
+    let href = new URL(request.url).search.slice(1);
+    let parsedQueryString = parseQuery(href);
     let htmlFormat = parsedQueryString.prerenderedHtmlFormat as string;
     let cardUrls = parsedQueryString.cardUrls as string[];