From fe0e18c3da6ffba9735520a56474ac623eee4606 Mon Sep 17 00:00:00 2001 From: Mark Woods Date: Mon, 8 Jul 2024 17:04:21 +0100 Subject: [PATCH 1/2] Fix body parser to handle borked content type I recently upgraded an app using @architect/functions from ^3.14.1 to ^8.1.6 and ran into an error parsing the body from some client requests. It turned out that there was some client code sending an invalid content type header, formatting it the same as http accept, and including two comma-separated mime types, application/json and text/plain, which I know is just wrong, but the way the body parser handled this was to consider the request to be both json and plain text and then blow up when parsing the body as plain text, with the error below... { "errorType": "TypeError", "errorMessage": "The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received an instance of Object", "code": "ERR_INVALID_ARG_TYPE", "stack": [ "TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received an instance of Object", " at new from (node:buffer:319:9)", " at parseBody (/var/task/node_modules/@architect/functions/src/http/helpers/body-parser.js:44:22)", " at Runtime.lambda [as handler] (/var/task/node_modules/@architect/functions/src/http/index.js:33:22)" ] } This commit fixes that by changing the conditions to only parse the body once, even if the content type is malformed and includes multiple types. This seems to do the job ok, but it might better to actually validate the content type header and blow up with an informative error message. --- src/http/helpers/body-parser.js | 12 +++--------- test/unit/src/http/helpers/body-parser-test.js | 13 +++++++++++++ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/http/helpers/body-parser.js b/src/http/helpers/body-parser.js index 0b63084f..bdff12c0 100644 --- a/src/http/helpers/body-parser.js +++ b/src/http/helpers/body-parser.js @@ -38,18 +38,12 @@ module.exports = function parseBody (req) { catch { throw Error('Invalid request body encoding or invalid JSON') } - } - - if (isPlainText || isXml) { + } else if (isPlainText || isXml) { request.body = new Buffer.from(request.body, 'base64').toString() - } - - if (isFormURLEncoded) { + } else if (isFormURLEncoded) { let data = new Buffer.from(request.body, 'base64').toString() request.body = qs.parse(data) - } - - if (isMultiPartFormData || isOctetStream) { + } else if (isMultiPartFormData || isOctetStream) { request.body = request.body.base64 ? request.body : { base64: request.body } diff --git a/test/unit/src/http/helpers/body-parser-test.js b/test/unit/src/http/helpers/body-parser-test.js index acf36072..35c72644 100644 --- a/test/unit/src/http/helpers/body-parser-test.js +++ b/test/unit/src/http/helpers/body-parser-test.js @@ -20,6 +20,19 @@ let octetStream = { 'Content-Type': 'application/octet-stream' } let text = { 'Content-Type': 'text/plain' } let xmlText = { 'Content-Type': 'text/xml' } let xmlApp = { 'Content-Type': 'application/xml' } +let borked = { 'Content-Type': 'application/json, text/plain' } + +test('Borked requests', t => { + t.plan(1) + + let req = { + body: str(hi), + headers: borked, + isBase64Encoded: false, + } + t.equals(str(parseBody(req)), str(hi), `body matches ${str(req.body)}`) + +}) test('Architect v10+ requests', t => { t.plan(6) From a2fb8f1bbd6383629cc524a7b59fc497352152db Mon Sep 17 00:00:00 2001 From: filmaj Date: Fri, 28 Mar 2025 23:29:53 -0400 Subject: [PATCH 2/2] add extra comment, linting --- src/http/helpers/body-parser.js | 10 +++++++--- test/unit/src/http/helpers/body-parser-test.js | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/http/helpers/body-parser.js b/src/http/helpers/body-parser.js index bdff12c0..b374cc51 100644 --- a/src/http/helpers/body-parser.js +++ b/src/http/helpers/body-parser.js @@ -14,6 +14,7 @@ module.exports = function parseBody (req) { // Paranoid deep copy let request = JSON.parse(JSON.stringify(req)) let headers = request.headers + // Note: content-type header may have multiple, comma-separated values. matching w/ includes may match to multiple different types let contentType = type => headers?.['content-type']?.includes(type) || headers?.['Content-Type']?.includes(type) let isString = typeof request.body === 'string' @@ -38,12 +39,15 @@ module.exports = function parseBody (req) { catch { throw Error('Invalid request body encoding or invalid JSON') } - } else if (isPlainText || isXml) { + } + else if (isPlainText || isXml) { request.body = new Buffer.from(request.body, 'base64').toString() - } else if (isFormURLEncoded) { + } + else if (isFormURLEncoded) { let data = new Buffer.from(request.body, 'base64').toString() request.body = qs.parse(data) - } else if (isMultiPartFormData || isOctetStream) { + } + else if (isMultiPartFormData || isOctetStream) { request.body = request.body.base64 ? request.body : { base64: request.body } diff --git a/test/unit/src/http/helpers/body-parser-test.js b/test/unit/src/http/helpers/body-parser-test.js index 35c72644..9d36a7a1 100644 --- a/test/unit/src/http/helpers/body-parser-test.js +++ b/test/unit/src/http/helpers/body-parser-test.js @@ -20,14 +20,14 @@ let octetStream = { 'Content-Type': 'application/octet-stream' } let text = { 'Content-Type': 'text/plain' } let xmlText = { 'Content-Type': 'text/xml' } let xmlApp = { 'Content-Type': 'application/xml' } -let borked = { 'Content-Type': 'application/json, text/plain' } +let multipleTypes = { 'Content-Type': 'application/json, text/plain' } test('Borked requests', t => { t.plan(1) let req = { body: str(hi), - headers: borked, + headers: multipleTypes, isBase64Encoded: false, } t.equals(str(parseBody(req)), str(hi), `body matches ${str(req.body)}`)