Skip to content

Commit

Permalink
fix: content type request bug in http serializer (middyjs#335)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
noetix authored and benjifs committed May 21, 2020
1 parent 5e337dd commit de26ff8
Show file tree
Hide file tree
Showing 39 changed files with 111 additions and 48 deletions.
2 changes: 1 addition & 1 deletion lerna.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
"packages": [
"packages/*"
],
"version": "1.0.0-alpha.31"
"version": "1.0.0-alpha.32"
}
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/cache/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/do-not-wait-for-empty-event-loop/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/error-logger/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/error-logger/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/function-shield/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/function-shield/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/http-content-negotiation/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/http-content-negotiation/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/http-cors/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/http-error-handler/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/http-error-handler/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/http-event-normalizer/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/http-header-normalizer/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/http-json-body-parser/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/http-json-body-parser/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/http-partial-response/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/http-partial-response/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
75 changes: 67 additions & 8 deletions packages/http-response-serializer/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -46,6 +46,7 @@ describe('📦 Middleware Http Response Serializer', () => {

handler({}, {}, (_, response) => {
expect(response).toEqual(handlerResponse)
done()
})
})
})
Expand All @@ -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))
Expand All @@ -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'

Expand All @@ -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())
)
Expand All @@ -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'

Expand All @@ -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())
)
Expand All @@ -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())
)
Expand All @@ -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()
})
Expand All @@ -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()
})
})
})
10 changes: 7 additions & 3 deletions packages/http-response-serializer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand All @@ -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 || []
)
Expand All @@ -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 })

Expand Down
2 changes: 1 addition & 1 deletion packages/http-response-serializer/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/http-response-serializer/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/http-security-header/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
Loading

0 comments on commit de26ff8

Please sign in to comment.