Appropriate error when a multipart field exceeds the size limit.

Fixes #159 .
jaydenseric · Oct 7, 2019 · 1795c7f · 1795c7f
1 parent 1f51499
commit 1795c7f
Show file tree

Hide file tree

Showing 5 changed files with 174 additions and 74 deletions.
diff --git a/changelog.md b/changelog.md
@@ -4,6 +4,7 @@
 
 ### Minor
 
+- `processRequest` now throws an appropriate error when a multipart field value exceeds the configured size limit, fixing [#159](https://github.com/jaydenseric/graphql-upload/issues/159).
 - Added a new `processRequest` option to the `graphqlUploadExpress` and `graphqlUploadKoa` middleware, for improved testing without mocks or spies which are difficult to achieve with ESM.
 
 ### Patch

diff --git a/src/processRequest.mjs b/src/processRequest.mjs
@@ -142,109 +142,120 @@ export const processRequest = (
           if (upload.file) upload.file.capacitor.destroy()
     }
 
-    parser.on('field', (fieldName, value) => {
-      if (exitError) return
-
-      switch (fieldName) {
-        case 'operations':
-          try {
-            operations = JSON.parse(value)
-          } catch (error) {
-            return exit(
-              createError(
-                400,
-                `Invalid JSON in the ‘operations’ multipart field (${SPEC_URL}).`
-              )
+    parser.on(
+      'field',
+      (fieldName, value, fieldNameTruncated, valueTruncated) => {
+        if (exitError) return
+
+        if (valueTruncated)
+          return exit(
+            createError(
+              413,
+              `The ‘${fieldName}’ multipart field value exceeds the ${maxFieldSize} byte size limit.`
             )
-          }
+          )
 
-          if (!isEnumerableObject(operations) && !Array.isArray(operations))
-            return exit(
-              createError(
-                400,
-                `Invalid type for the ‘operations’ multipart field (${SPEC_URL}).`
+        switch (fieldName) {
+          case 'operations':
+            try {
+              operations = JSON.parse(value)
+            } catch (error) {
+              return exit(
+                createError(
+                  400,
+                  `Invalid JSON in the ‘operations’ multipart field (${SPEC_URL}).`
+                )
               )
-            )
-
-          operationsPath = objectPath(operations)
+            }
 
-          break
-        case 'map': {
-          if (!operations)
-            return exit(
-              createError(
-                400,
-                `Misordered multipart fields; ‘map’ should follow ‘operations’ (${SPEC_URL}).`
+            if (!isEnumerableObject(operations) && !Array.isArray(operations))
+              return exit(
+                createError(
+                  400,
+                  `Invalid type for the ‘operations’ multipart field (${SPEC_URL}).`
+                )
               )
-            )
 
-          let parsedMap
-          try {
-            parsedMap = JSON.parse(value)
-          } catch (error) {
-            return exit(
-              createError(
-                400,
-                `Invalid JSON in the ‘map’ multipart field (${SPEC_URL}).`
-              )
-            )
-          }
+            operationsPath = objectPath(operations)
 
-          if (!isEnumerableObject(parsedMap))
-            return exit(
-              createError(
-                400,
-                `Invalid type for the ‘map’ multipart field (${SPEC_URL}).`
+            break
+          case 'map': {
+            if (!operations)
+              return exit(
+                createError(
+                  400,
+                  `Misordered multipart fields; ‘map’ should follow ‘operations’ (${SPEC_URL}).`
+                )
               )
-            )
-
-          const mapEntries = Object.entries(parsedMap)
 
-          // Check max files is not exceeded, even though the number of files to
-          // parse might not match th(e map provided by the client.
-          if (mapEntries.length > maxFiles)
-            return exit(
-              createError(413, `${maxFiles} max file uploads exceeded.`)
-            )
+            let parsedMap
+            try {
+              parsedMap = JSON.parse(value)
+            } catch (error) {
+              return exit(
+                createError(
+                  400,
+                  `Invalid JSON in the ‘map’ multipart field (${SPEC_URL}).`
+                )
+              )
+            }
 
-          map = new Map()
-          for (const [fieldName, paths] of mapEntries) {
-            if (!Array.isArray(paths))
+            if (!isEnumerableObject(parsedMap))
               return exit(
                 createError(
                   400,
-                  `Invalid type for the ‘map’ multipart field entry key ‘${fieldName}’ array (${SPEC_URL}).`
+                  `Invalid type for the ‘map’ multipart field (${SPEC_URL}).`
                 )
               )
 
-            map.set(fieldName, new Upload())
+            const mapEntries = Object.entries(parsedMap)
+
+            // Check max files is not exceeded, even though the number of files to
+            // parse might not match th(e map provided by the client.
+            if (mapEntries.length > maxFiles)
+              return exit(
+                createError(413, `${maxFiles} max file uploads exceeded.`)
+              )
 
-            for (const [index, path] of paths.entries()) {
-              if (typeof path !== 'string')
+            map = new Map()
+            for (const [fieldName, paths] of mapEntries) {
+              if (!Array.isArray(paths))
                 return exit(
                   createError(
                     400,
-                    `Invalid type for the ‘map’ multipart field entry key ‘${fieldName}’ array index ‘${index}’ value (${SPEC_URL}).`
+                    `Invalid type for the ‘map’ multipart field entry key ‘${fieldName}’ array (${SPEC_URL}).`
                   )
                 )
 
-              try {
-                operationsPath.set(path, map.get(fieldName).promise)
-              } catch (error) {
-                return exit(
-                  createError(
-                    400,
-                    `Invalid object path for the ‘map’ multipart field entry key ‘${fieldName}’ array index ‘${index}’ value ‘${path}’ (${SPEC_URL}).`
+              map.set(fieldName, new Upload())
+
+              for (const [index, path] of paths.entries()) {
+                if (typeof path !== 'string')
+                  return exit(
+                    createError(
+                      400,
+                      `Invalid type for the ‘map’ multipart field entry key ‘${fieldName}’ array index ‘${index}’ value (${SPEC_URL}).`
+                    )
                   )
-                )
+
+                try {
+                  operationsPath.set(path, map.get(fieldName).promise)
+                } catch (error) {
+                  return exit(
+                    createError(
+                      400,
+                      `Invalid object path for the ‘map’ multipart field entry key ‘${fieldName}’ array index ‘${index}’ value ‘${path}’ (${SPEC_URL}).`
+                    )
+                  )
+                }
               }
             }
-          }
 
-          resolve(operations)
+            resolve(operations)
+          }
         }
       }
-    })
+    )
 
     parser.on('file', (fieldName, stream, filename, encoding, mimetype) => {
       if (exitError) {

diff --git a/src/processRequest.test.mjs b/src/processRequest.test.mjs
@@ -1466,6 +1466,54 @@ t.test('Exceed max file size.', async t => {
   })
 })
 
+t.test('Exceed max field size.', async t => {
+  const maxFieldSize = 1
+  const sendRequest = async (t, port) => {
+    const body = new FormData()
+
+    body.append('operations', '{ "variables": { "file": null } }')
+    body.append('map', '{ 1: ["variables.file"] }')
+    body.append('1', 'a', { filename: 'a.txt' })
+
+    const { status } = await fetch(`http://localhost:${port}`, {
+      method: 'POST',
+      body
+    })
+
+    t.equal(status, 413, 'Response status.')
+  }
+
+  await t.test('Koa middleware.', async t => {
+    t.plan(2)
+
+    const app = new Koa()
+      .on('error', error =>
+        t.matchSnapshot(snapshotError(error), 'Middleware throws.')
+      )
+      .use(graphqlUploadKoa({ maxFieldSize }))
+
+    const port = await startServer(t, app)
+
+    await sendRequest(t, port)
+  })
+
+  await t.test('Express middleware.', async t => {
+    t.plan(2)
+
+    const app = express()
+      .use(graphqlUploadExpress({ maxFieldSize }))
+      .use((error, request, response, next) => {
+        if (response.headersSent) return next(error)
+        t.matchSnapshot(snapshotError(error), 'Middleware throws.')
+        response.send()
+      })
+
+    const port = await startServer(t, app)
+
+    await sendRequest(t, port)
+  })
+})
+
 t.test('Misorder ‘map’ before ‘operations’.', async t => {
   const sendRequest = async (t, port) => {
     const body = new FormData()

diff --git a/tap-snapshots/lib-processRequest.test.js-TAP.test.js b/tap-snapshots/lib-processRequest.test.js-TAP.test.js
@@ -183,6 +183,26 @@ exports[`lib/processRequest.test.js TAP Deprecated file upload ‘stream’ prop
 }
 `
 
+exports[`lib/processRequest.test.js TAP Exceed max field size. Express middleware. > Middleware throws. 1`] = `
+{
+  "name": "PayloadTooLargeError",
+  "message": "The ‘operations’ multipart field value exceeds the 1 byte size limit.",
+  "status": 413,
+  "statusCode": 413,
+  "expose": true
+}
+`
+
+exports[`lib/processRequest.test.js TAP Exceed max field size. Koa middleware. > Middleware throws. 1`] = `
+{
+  "name": "PayloadTooLargeError",
+  "message": "The ‘operations’ multipart field value exceeds the 1 byte size limit.",
+  "status": 413,
+  "statusCode": 413,
+  "expose": true
+}
+`
+
 exports[`lib/processRequest.test.js TAP Exceed max file size. Express middleware. Upload A. > Stream error. 1`] = `
 {
   "name": "PayloadTooLargeError",

diff --git a/tap-snapshots/lib-processRequest.test.mjs-TAP.test.js b/tap-snapshots/lib-processRequest.test.mjs-TAP.test.js
@@ -183,6 +183,26 @@ exports[`lib/processRequest.test.mjs TAP Deprecated file upload ‘stream’ pro
 }
 `
 
+exports[`lib/processRequest.test.mjs TAP Exceed max field size. Express middleware. > Middleware throws. 1`] = `
+{
+  "name": "PayloadTooLargeError",
+  "message": "The ‘operations’ multipart field value exceeds the 1 byte size limit.",
+  "status": 413,
+  "statusCode": 413,
+  "expose": true
+}
+`
+
+exports[`lib/processRequest.test.mjs TAP Exceed max field size. Koa middleware. > Middleware throws. 1`] = `
+{
+  "name": "PayloadTooLargeError",
+  "message": "The ‘operations’ multipart field value exceeds the 1 byte size limit.",
+  "status": 413,
+  "statusCode": 413,
+  "expose": true
+}
+`
+
 exports[`lib/processRequest.test.mjs TAP Exceed max file size. Express middleware. Upload A. > Stream error. 1`] = `
 {
   "name": "PayloadTooLargeError",