From de26ff8b360d59e011857cf02eda358fee3bfb2b Mon Sep 17 00:00:00 2001 From: Alex Joyce Date: Fri, 17 May 2019 17:01:20 +1000 Subject: [PATCH] fix: content type request bug in http serializer (#335) * fix: tests not waiting for callbacks * fix: content-type skip using request headers not response headers * version bump and handling Cannot read property headers of undefined --- lerna.json | 2 +- package-lock.json | 2 +- package.json | 2 +- packages/cache/package.json | 2 +- packages/core/package-lock.json | 2 +- packages/core/package.json | 2 +- .../package.json | 2 +- packages/error-logger/package-lock.json | 2 +- packages/error-logger/package.json | 2 +- packages/function-shield/package-lock.json | 2 +- packages/function-shield/package.json | 2 +- .../package-lock.json | 2 +- .../http-content-negotiation/package.json | 2 +- packages/http-cors/package.json | 2 +- packages/http-error-handler/package-lock.json | 2 +- packages/http-error-handler/package.json | 2 +- packages/http-event-normalizer/package.json | 2 +- packages/http-header-normalizer/package.json | 2 +- .../http-json-body-parser/package-lock.json | 2 +- packages/http-json-body-parser/package.json | 2 +- .../http-partial-response/package-lock.json | 2 +- packages/http-partial-response/package.json | 2 +- .../__tests__/index.js | 75 +++++++++++++++++-- packages/http-response-serializer/index.js | 10 ++- .../package-lock.json | 2 +- .../http-response-serializer/package.json | 2 +- packages/http-security-header/package.json | 2 +- .../package-lock.json | 2 +- .../http-urlencode-body-parser/package.json | 2 +- .../input-output-logger/package-lock.json | 2 +- packages/input-output-logger/package.json | 2 +- packages/s3-key-normalizer/package.json | 2 +- packages/secrets-manager/package-lock.json | 2 +- packages/secrets-manager/package.json | 2 +- packages/ssm/package-lock.json | 2 +- packages/ssm/package.json | 2 +- packages/validator/package-lock.json | 2 +- packages/validator/package.json | 2 +- packages/warmup/package.json | 2 +- 39 files changed, 111 insertions(+), 48 deletions(-) diff --git a/lerna.json b/lerna.json index 682affc5a..bd6abddf4 100644 --- a/lerna.json +++ b/lerna.json @@ -3,5 +3,5 @@ "packages": [ "packages/*" ], - "version": "1.0.0-alpha.31" + "version": "1.0.0-alpha.32" } diff --git a/package-lock.json b/package-lock.json index 9b5f9a8fa..1f7681dc0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "middy-monorepo", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 7650e68e9..16260ec6e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "middy-monorepo", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "🛵 The stylish Node.js middleware engine for AWS Lambda", "engines": { "node": ">=6.10" diff --git a/packages/cache/package.json b/packages/cache/package.json index 251ddb572..34787d08a 100644 --- a/packages/cache/package.json +++ b/packages/cache/package.json @@ -1,6 +1,6 @@ { "name": "@middy/cache", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Cache middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/core/package-lock.json b/packages/core/package-lock.json index 75ede9de9..b6f81e48e 100644 --- a/packages/core/package-lock.json +++ b/packages/core/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/core", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/core/package.json b/packages/core/package.json index c7a1a0798..19a2a5393 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@middy/core", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "🛵 The stylish Node.js middleware engine for AWS Lambda (core package)", "engines": { "node": ">=6.10" diff --git a/packages/do-not-wait-for-empty-event-loop/package.json b/packages/do-not-wait-for-empty-event-loop/package.json index 983f482fb..8aee6d5b7 100644 --- a/packages/do-not-wait-for-empty-event-loop/package.json +++ b/packages/do-not-wait-for-empty-event-loop/package.json @@ -1,6 +1,6 @@ { "name": "@middy/do-not-wait-for-empty-event-loop", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Middleware for the middy framework that allows to easily disable the wait for empty event loop in a Lambda function", "engines": { "node": ">=6.10" diff --git a/packages/error-logger/package-lock.json b/packages/error-logger/package-lock.json index ac8868f5a..298534b9a 100644 --- a/packages/error-logger/package-lock.json +++ b/packages/error-logger/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/error-logger", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/error-logger/package.json b/packages/error-logger/package.json index 3b73210b7..1c2e18adb 100644 --- a/packages/error-logger/package.json +++ b/packages/error-logger/package.json @@ -1,6 +1,6 @@ { "name": "@middy/error-logger", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Input and output logger middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/function-shield/package-lock.json b/packages/function-shield/package-lock.json index df78b3785..456bd42e6 100644 --- a/packages/function-shield/package-lock.json +++ b/packages/function-shield/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/function-shield", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/function-shield/package.json b/packages/function-shield/package.json index 500a0b250..1b4e57a4d 100644 --- a/packages/function-shield/package.json +++ b/packages/function-shield/package.json @@ -1,6 +1,6 @@ { "name": "@middy/function-shield", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Hardens AWS Lambda execution environment", "engines": { "node": ">=6.10" diff --git a/packages/http-content-negotiation/package-lock.json b/packages/http-content-negotiation/package-lock.json index ef5f4ed03..b65b01a15 100644 --- a/packages/http-content-negotiation/package-lock.json +++ b/packages/http-content-negotiation/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/http-content-negotiation", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/http-content-negotiation/package.json b/packages/http-content-negotiation/package.json index 813aae990..782d24242 100644 --- a/packages/http-content-negotiation/package.json +++ b/packages/http-content-negotiation/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-content-negotiation", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Http content negotiation middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/http-cors/package.json b/packages/http-cors/package.json index 246c8abfe..219167bfe 100644 --- a/packages/http-cors/package.json +++ b/packages/http-cors/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-cors", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "CORS (Cross-Origin Resource Sharing) middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/http-error-handler/package-lock.json b/packages/http-error-handler/package-lock.json index 409fc9409..3ccf47348 100644 --- a/packages/http-error-handler/package-lock.json +++ b/packages/http-error-handler/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/http-error-handler", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/http-error-handler/package.json b/packages/http-error-handler/package.json index e4e5fe8e8..c2779ae1f 100644 --- a/packages/http-error-handler/package.json +++ b/packages/http-error-handler/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-error-handler", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Http error handler middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/http-event-normalizer/package.json b/packages/http-event-normalizer/package.json index f14afd7af..eb3c33bff 100644 --- a/packages/http-event-normalizer/package.json +++ b/packages/http-event-normalizer/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-event-normalizer", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Http event normalizer middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/http-header-normalizer/package.json b/packages/http-header-normalizer/package.json index 71a06a6ff..12d69a9b5 100644 --- a/packages/http-header-normalizer/package.json +++ b/packages/http-header-normalizer/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-header-normalizer", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Http header normalizer middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/http-json-body-parser/package-lock.json b/packages/http-json-body-parser/package-lock.json index 41efda169..f67a2388e 100644 --- a/packages/http-json-body-parser/package-lock.json +++ b/packages/http-json-body-parser/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/http-json-body-parser", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/http-json-body-parser/package.json b/packages/http-json-body-parser/package.json index 65f1df196..292781d8e 100644 --- a/packages/http-json-body-parser/package.json +++ b/packages/http-json-body-parser/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-json-body-parser", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Http JSON body parser middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/http-partial-response/package-lock.json b/packages/http-partial-response/package-lock.json index 4d4104bd6..82b12e1d5 100644 --- a/packages/http-partial-response/package-lock.json +++ b/packages/http-partial-response/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/http-partial-response", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/http-partial-response/package.json b/packages/http-partial-response/package.json index a9b3a6027..19b90caea 100644 --- a/packages/http-partial-response/package.json +++ b/packages/http-partial-response/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-partial-response", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Http partial response middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/http-response-serializer/__tests__/index.js b/packages/http-response-serializer/__tests__/index.js index 402335427..068cb4a4a 100644 --- a/packages/http-response-serializer/__tests__/index.js +++ b/packages/http-response-serializer/__tests__/index.js @@ -34,7 +34,7 @@ describe('📦 Middleware Http Response Serializer', () => { ['CONTENT-TYPE'] ])( '%s skips response serialization', - (key) => { + (key, done) => { const handlerResponse = Object.assign({}, createHttpResponse(), { headers: { [key]: 'text/plain' @@ -46,6 +46,7 @@ describe('📦 Middleware Http Response Serializer', () => { handler({}, {}, (_, response) => { expect(response).toEqual(handlerResponse) + done() }) }) }) @@ -57,7 +58,7 @@ describe('📦 Middleware Http Response Serializer', () => { ['text/plain, text/x-c', 'Hello World'] ])( '%s returns %s', - (accept, result) => { + (accept, result, done) => { const handler = middy((event, context, cb) => cb(null, createHttpResponse())) handler.use(httpResponseSerializer(standardConfiguration)) @@ -70,11 +71,12 @@ describe('📦 Middleware Http Response Serializer', () => { handler(event, {}, (_, response) => { expect(response.body).toEqual(result) + done() }) }) }) - test('It should use `event.requiredContentType` instead of accept headers', () => { + test('It should use `event.requiredContentType` instead of accept headers', done => { const handler = middy((event, context, cb) => { event.requiredContentType = 'text/plain' @@ -97,10 +99,11 @@ describe('📦 Middleware Http Response Serializer', () => { }, body: 'Hello World' }) + done() }) }) - test('It should use the default when no accept preferences are given', () => { + test('It should use the default when no accept preferences are given', done => { const handler = middy((event, context, cb) => cb(null, createHttpResponse()) ) @@ -115,10 +118,11 @@ describe('📦 Middleware Http Response Serializer', () => { }, body: '{"message":"Hello World"}' }) + done() }) }) - test('It should use `event.preferredContentType` instead of the default', () => { + test('It should use `event.preferredContentType` instead of the default', done => { const handler = middy((event, context, cb) => { event.preferredContentType = 'text/plain' @@ -135,10 +139,11 @@ describe('📦 Middleware Http Response Serializer', () => { }, body: 'Hello World' }) + done() }) }) - test('It should pass-through when no preference or default is found', () => { + test('It should pass-through when no preference or default is found', done => { const handler = middy((event, context, cb) => cb(null, createHttpResponse()) ) @@ -152,10 +157,37 @@ describe('📦 Middleware Http Response Serializer', () => { statusCode: 200, body: 'Hello World' }) + done() }) }) - test('It should replace the response object when the serializer returns an object', () => { + test('It should not pass-through when request content-type is set', done => { + const handler = middy((event, context, cb) => + cb(null, createHttpResponse()) + ) + + handler.use(httpResponseSerializer(standardConfiguration)) + + const event = { + headers: { + 'Content-Type': 'application/xml' + } + } + + handler(event, {}, (err, response) => { + if (err) throw err + expect(response).toEqual({ + statusCode: 200, + headers: { + 'Content-Type': standardConfiguration.default + }, + body: '{"message":"Hello World"}' + }) + done() + }) + }) + + test('It should replace the response object when the serializer returns an object', done => { const handler = middy((event, context, cb) => cb(null, createHttpResponse()) ) @@ -182,10 +214,11 @@ describe('📦 Middleware Http Response Serializer', () => { body: 'SGVsbG8gV29ybGQ=', isBase64Encoded: true }) + done() }) }) - test('It should work with `http-error-handler` middleware', () => { + test('It should work with `http-error-handler` middleware', done => { const handler = middy((event, context, cb) => { throw new createError.UnprocessableEntity() }) @@ -202,6 +235,32 @@ describe('📦 Middleware Http Response Serializer', () => { 'Content-Type': 'application/json' } }) + done() + }) + }) + + test('It should not crash if the response is undefined', done => { + const handler = middy((event, context, cb) => + cb(null, undefined) + ) + + handler.use(httpResponseSerializer(standardConfiguration)) + + const event = { + headers: { + 'Content-Type': 'application/xml' + } + } + + handler(event, {}, (err, response) => { + if (err) throw err + expect(response).toEqual({ + headers: { + 'Content-Type': standardConfiguration.default + }, + body: '{}' + }) + done() }) }) }) diff --git a/packages/http-response-serializer/index.js b/packages/http-response-serializer/index.js index 6d530a78e..67c4a1b36 100644 --- a/packages/http-response-serializer/index.js +++ b/packages/http-response-serializer/index.js @@ -10,10 +10,11 @@ const getNormalisedHeaders = (source) => Object const middleware = (opts, handler, next) => { // normalise headers for internal use only - const headers = getNormalisedHeaders(handler.event.headers || {}) + const requestHeaders = getNormalisedHeaders((handler.event && handler.event.headers) || {}) + const responseHeaders = getNormalisedHeaders((handler.response && handler.response.headers) || {}) // skip serialization when content-type is already set - if (headers['content-type']) { + if (responseHeaders['content-type']) { return next() } @@ -24,7 +25,7 @@ const middleware = (opts, handler, next) => { types = [].concat(handler.event.requiredContentType) } else { types = [].concat( - (headers['accept'] && Accept.mediaTypes(headers['accept'])) || [], + (requestHeaders['accept'] && Accept.mediaTypes(requestHeaders['accept'])) || [], handler.event.preferredContentType || [], opts.default || [] ) @@ -41,6 +42,9 @@ const middleware = (opts, handler, next) => { if (!test) { return false } + // if the response is null or undefined, normalizes it back to an object + handler.response = handler.response || {} + // set header handler.response.headers = Object.assign({}, handler.response.headers, { 'Content-Type': type }) diff --git a/packages/http-response-serializer/package-lock.json b/packages/http-response-serializer/package-lock.json index 138cdb4e7..f3c66ecf1 100644 --- a/packages/http-response-serializer/package-lock.json +++ b/packages/http-response-serializer/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/http-response-serializer", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/http-response-serializer/package.json b/packages/http-response-serializer/package.json index 64603d338..8ccfd374e 100644 --- a/packages/http-response-serializer/package.json +++ b/packages/http-response-serializer/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-response-serializer", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Http response serializer middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/http-security-header/package.json b/packages/http-security-header/package.json index dad0610cb..8b3e784ea 100644 --- a/packages/http-security-header/package.json +++ b/packages/http-security-header/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-security-header", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Applies best practice security headers to responses. It's a simplified port of HelmetJS", "engines": { "node": ">=6.10" diff --git a/packages/http-urlencode-body-parser/package-lock.json b/packages/http-urlencode-body-parser/package-lock.json index 3640e4d56..fccb83825 100644 --- a/packages/http-urlencode-body-parser/package-lock.json +++ b/packages/http-urlencode-body-parser/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/http-urlencode-body-parser", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/http-urlencode-body-parser/package.json b/packages/http-urlencode-body-parser/package.json index b241b51fb..e6ce238ae 100644 --- a/packages/http-urlencode-body-parser/package.json +++ b/packages/http-urlencode-body-parser/package.json @@ -1,6 +1,6 @@ { "name": "@middy/http-urlencode-body-parser", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Urlencode body parser middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/input-output-logger/package-lock.json b/packages/input-output-logger/package-lock.json index c121078fb..5950febee 100644 --- a/packages/input-output-logger/package-lock.json +++ b/packages/input-output-logger/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/input-output-logger", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/input-output-logger/package.json b/packages/input-output-logger/package.json index fad195de4..1c43f0fcc 100644 --- a/packages/input-output-logger/package.json +++ b/packages/input-output-logger/package.json @@ -1,6 +1,6 @@ { "name": "@middy/input-output-logger", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Input and output logger middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/s3-key-normalizer/package.json b/packages/s3-key-normalizer/package.json index 511e9c459..cb73eb98c 100644 --- a/packages/s3-key-normalizer/package.json +++ b/packages/s3-key-normalizer/package.json @@ -1,6 +1,6 @@ { "name": "@middy/s3-key-normalizer", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "S3 key normalizer middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/secrets-manager/package-lock.json b/packages/secrets-manager/package-lock.json index 94e82a6c7..d7d32978e 100644 --- a/packages/secrets-manager/package-lock.json +++ b/packages/secrets-manager/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/secrets-manager", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/secrets-manager/package.json b/packages/secrets-manager/package.json index 924ac742b..e8b3f15f4 100644 --- a/packages/secrets-manager/package.json +++ b/packages/secrets-manager/package.json @@ -1,6 +1,6 @@ { "name": "@middy/secrets-manager", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Secrets Manager middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/ssm/package-lock.json b/packages/ssm/package-lock.json index 1da9b4afe..64f892e08 100644 --- a/packages/ssm/package-lock.json +++ b/packages/ssm/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/ssm", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/ssm/package.json b/packages/ssm/package.json index b3e47388e..962b4191a 100644 --- a/packages/ssm/package.json +++ b/packages/ssm/package.json @@ -1,6 +1,6 @@ { "name": "@middy/ssm", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "SSM (EC2 Systems Manager) parameters middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/validator/package-lock.json b/packages/validator/package-lock.json index 2ef487b3b..b4b51c490 100644 --- a/packages/validator/package-lock.json +++ b/packages/validator/package-lock.json @@ -1,6 +1,6 @@ { "name": "@middy/validator", - "version": "1.0.0-alpha.30", + "version": "1.0.0-alpha.31", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/validator/package.json b/packages/validator/package.json index 7f6ca3759..bc64525ee 100644 --- a/packages/validator/package.json +++ b/packages/validator/package.json @@ -1,6 +1,6 @@ { "name": "@middy/validator", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Validator middleware for the middy framework", "engines": { "node": ">=6.10" diff --git a/packages/warmup/package.json b/packages/warmup/package.json index d757fa6a8..b20ca6d6e 100644 --- a/packages/warmup/package.json +++ b/packages/warmup/package.json @@ -1,6 +1,6 @@ { "name": "@middy/warmup", - "version": "1.0.0-alpha.31", + "version": "1.0.0-alpha.32", "description": "Warmup (cold start mitigation) middleware for the middy framework", "engines": { "node": ">=6.10"