fix(medusa): Query parser issues with large array (#4213)

* fix(medusa): Query parser issues with large array

* Create little-weeks-exercise.md

* add integration tests

.changeset/little-weeks-exercise.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/medusa": patch
+---
+
+fix(medusa): Query parser issues with large array

integration-tests/api/__tests__/admin/variant.js

@@ -8,7 +8,12 @@ const { simpleProductFactory } = require("../../factories")
 const adminSeeder = require("../../helpers/admin-seeder")
 const adminVariantsSeeder = require("../../helpers/admin-variants-seeder")
 const productSeeder = require("../../helpers/product-seeder")
-const storeProductSeeder = require("../../helpers/store-product-seeder")
+
+const adminHeaders = {
+  headers: {
+    Authorization: "Bearer test_token",
+  },
+}
 
 jest.setTimeout(30000)
 
@@ -44,11 +49,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .get("/admin/variants/", {
+        .get("/admin/variants/", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -74,11 +75,7 @@ describe("/admin/products", () => {
     it("lists all product variants matching a specific sku", async () => {
       const api = useApi()
       const response = await api
-        .get("/admin/variants?q=sku2", {
+        .get("/admin/variants?q=sku2", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -97,11 +94,7 @@ describe("/admin/products", () => {
     it("lists all product variants matching a specific variant title", async () => {
       const api = useApi()
       const response = await api
-        .get("/admin/variants?q=rank (1)", {
+        .get("/admin/variants?q=rank (1)", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -122,11 +115,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .get("/admin/variants?q=Test product1", {
+        .get("/admin/variants?q=Test product1", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -170,11 +159,7 @@ describe("/admin/products", () => {
 
       const response = await api.get(
         "/admin/variants?id=test-variant&currency_code=usd",
-        {
+        adminHeaders
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        }
       )
 
       expect(response.data).toMatchSnapshot({
@@ -203,11 +188,7 @@ describe("/admin/products", () => {
 
       const response = await api.get(
         "/admin/variants?id=test-variant&region_id=reg-europe",
-        {
+        adminHeaders
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        }
       )
 
       expect(response.data).toMatchSnapshot({
@@ -236,11 +217,7 @@ describe("/admin/products", () => {
 
       const response = await api.get(
         "/admin/variants?id=test-variant&region_id=reg-europe&customer_id=test-customer",
-        {
+        adminHeaders
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        }
       )
 
       expect(response.data).toMatchSnapshot({
@@ -263,5 +240,48 @@ describe("/admin/products", () => {
         ],
       })
     })
+
+    it("returns a list of variants matching the given ids", async () => {
+      const api = useApi()
+
+      const productData = {
+        id: "test-product_filtering_by_variant_id",
+        title: "Test product filtering by variant id",
+        handle: "test-product_filtering_by_variant_id",
+        options: [
+          {
+            id: "test-product_filtering_by_variant_id-option",
+            title: "Size",
+          },
+        ],
+        variants: [],
+      }
+
+      for (let i = 0; i < 25; i++) {
+        productData.variants.push({
+          product_id: productData.id,
+          sku: `test-product_filtering_by_variant_id-${i}`,
+          title: `test-product_filtering_by_variant_id-${i}`,
+        })
+      }
+
+      const product = await simpleProductFactory(dbConnection, productData)
+
+      const variantIds = product.variants.map((v) => v.id)
+      const qs = "id[]=" + variantIds.join("&id[]=")
+
+      const response = await api
+        .get("/admin/variants?limit=30&" + qs, adminHeaders)
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+      expect(response.data.variants.length).toEqual(variantIds.length)
+
+      for (const variant of response.data.variants) {
+        expect(variantIds).toContain(variant.id)
+      }
+    })
   })
 })

packages/medusa/package.json

@@ -83,6 +83,7 @@
     "passport-http-bearer": "^1.0.1",
     "passport-jwt": "^4.0.1",
     "passport-local": "^1.0.0",
+    "qs": "^6.11.2",
     "randomatic": "^3.1.1",
     "redis": "^3.0.2",
     "reflect-metadata": "^0.1.13",

packages/medusa/src/loaders/api.ts

@@ -1,4 +1,5 @@
 import { Express } from "express"
+import qs from "qs"
 import bodyParser from "body-parser"
 import routes from "../api"
 import { AwilixContainer } from "awilix"
@@ -11,6 +12,18 @@ type Options = {
 }
 
 export default async ({ app, container, configModule }: Options) => {
+  // This is a workaround for the issue described here: https://github.com/expressjs/express/issues/3454
+  // We parse the url and get the qs to be parsed and override the query prop from the request
+  app.use(function (req, res, next) {
+    const parsedUrl = req.url.split("?")
+    parsedUrl.shift()
+    const queryParamsStr = parsedUrl.join("?")
+    if (queryParamsStr) {
+      req.query = qs.parse(queryParamsStr, { arrayLimit: Infinity })
+    }
+    next()
+  })
+
   app.use(bodyParser.json())
   app.use("/", routes(container, configModule.projectConfig))
 

yarn.lock

@@ -6229,6 +6229,7 @@ __metadata:
     passport-http-bearer: ^1.0.1
     passport-jwt: ^4.0.1
     passport-local: ^1.0.0
+    qs: ^6.11.2
     randomatic: ^3.1.1
     redis: ^3.0.2
     reflect-metadata: ^0.1.13
@@ -35506,6 +35507,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"qs@npm:^6.11.2":
+  version: 6.11.2
+  resolution: "qs@npm:6.11.2"
+  dependencies:
+    side-channel: ^1.0.4
+  checksum: 4f95d4ff18ed480befcafa3390022817ffd3087fc65f146cceb40fc5edb9fa96cb31f648cae2fa96ca23818f0798bd63ad4ca369a0e22702fcd41379b3ab6571
+  languageName: node
+  linkType: hard
+
 "qs@npm:~6.5.2":
   version: 6.5.3
   resolution: "qs@npm:6.5.3"