From 5d8c4860c4903c91a818fa500f0bf82cdfbaabbd Mon Sep 17 00:00:00 2001 From: Alex Brooke Date: Wed, 30 May 2018 15:44:30 -0500 Subject: [PATCH 1/5] set Content-Length header - without content-length specified, it seems the body of DELETE requests get ignored - per RFC 2616, section 4.3: The presence of a message-body in a request is signaled by the inclusion of a Content-Length or Transfer-Encoding header field in the request's message-headers. A message-body MUST NOT be included in a request if the specification of the request method (section 5.1.1) does not allow sending an entity-body in requests. A server SHOULD read and forward a message-body on any request; if the request method does not include defined semantics for an entity-body, then the message-body SHOULD be ignored when handling the request. - in other words, whether or not it is recommended to have a body in a DELETE request, if the content-length header (or the transfer-encoding header) is set, the message body should be forwarded - concerns: - I'm not sure whether it is necessary to perform more type checking on `event.body`. for instance, if it is an object, it should be passed through a call to `JSON.stringify` before getting the byte length - the header should probably be set only if not already specified This commit is one possible solution to awslabs/aws-serverless-express#106 --- index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index d5b080c4..34f38f47 100644 --- a/index.js +++ b/index.js @@ -35,7 +35,8 @@ function mapApiGatewayEventToHttpRequest(event, context, socketPath) { const headers = event.headers || {} // NOTE: Mutating event.headers; prefer deep clone of event.headers const eventWithoutBody = Object.assign({}, event) delete eventWithoutBody.body - + + headers['Content-Length'] = Buffer.byteLength(event.body) headers['x-apigateway-event'] = encodeURIComponent(JSON.stringify(eventWithoutBody)) headers['x-apigateway-context'] = encodeURIComponent(JSON.stringify(context)) @@ -60,7 +61,7 @@ function forwardResponseToApiGateway(server, response, context) { const bodyBuffer = Buffer.concat(buf) const statusCode = response.statusCode const headers = response.headers - + // chunked transfer not currently supported by API Gateway if (headers['transfer-encoding'] === 'chunked') delete headers['transfer-encoding'] From 1ef34b8ec4f6f388ec3a0dee79bfa1b938caf6f6 Mon Sep 17 00:00:00 2001 From: Alex Brooke Date: Tue, 19 Jun 2018 13:27:34 -0500 Subject: [PATCH 2/5] only specify content-length in certain cases will now only set the content-length header if the request has a body and the header is not already set --- index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 34f38f47..d111ba04 100644 --- a/index.js +++ b/index.js @@ -36,7 +36,9 @@ function mapApiGatewayEventToHttpRequest(event, context, socketPath) { const eventWithoutBody = Object.assign({}, event) delete eventWithoutBody.body - headers['Content-Length'] = Buffer.byteLength(event.body) + if (event.body && !headers['Content-Length']) { + headers['Content-Length'] = Buffer.byteLength(event.body) + } headers['x-apigateway-event'] = encodeURIComponent(JSON.stringify(eventWithoutBody)) headers['x-apigateway-context'] = encodeURIComponent(JSON.stringify(context)) From 9745be8f58c4d49a6e813af109db7781757dd896 Mon Sep 17 00:00:00 2001 From: Alex Brooke Date: Thu, 21 Jun 2018 10:31:18 -0500 Subject: [PATCH 3/5] update tests - fix failing test cases - add comment about motivation for change in index.js --- __tests__/index.js | 4 ++++ index.js | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/__tests__/index.js b/__tests__/index.js index 60805700..9b76b2f3 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -51,6 +51,8 @@ function mapApiGatewayEventToHttpRequest(headers) { headers } const eventClone = JSON.parse(JSON.stringify(event)) + // NOTE: mapApiGatewayEventToHttpRequest will specify Content-Length if not specified + eventClone.headers = Object.assign(eventClone.headers || {}, {'Content-Length': Buffer.byteLength(eventClone.body)}) delete eventClone.body const context = { 'foo': 'bar' @@ -68,6 +70,7 @@ test('mapApiGatewayEventToHttpRequest: with headers', () => { method: 'GET', path: '/foo', headers: { + 'Content-Length': Buffer.byteLength('Hello serverless!'), 'x-foo': 'foo', 'x-apigateway-event': encodeURIComponent(JSON.stringify(r.eventClone)), 'x-apigateway-context': encodeURIComponent(JSON.stringify(r.context)) @@ -83,6 +86,7 @@ test('mapApiGatewayEventToHttpRequest: without headers', () => { method: 'GET', path: '/foo', headers: { + 'Content-Length': Buffer.byteLength('Hello serverless!'), 'x-apigateway-event': encodeURIComponent(JSON.stringify(r.eventClone)), 'x-apigateway-context': encodeURIComponent(JSON.stringify(r.context)) }, diff --git a/index.js b/index.js index d111ba04..2332d3f3 100644 --- a/index.js +++ b/index.js @@ -35,7 +35,8 @@ function mapApiGatewayEventToHttpRequest(event, context, socketPath) { const headers = event.headers || {} // NOTE: Mutating event.headers; prefer deep clone of event.headers const eventWithoutBody = Object.assign({}, event) delete eventWithoutBody.body - + + // NOTE: API Gateway is not setting Content-Length header on any requests even when they have a body if (event.body && !headers['Content-Length']) { headers['Content-Length'] = Buffer.byteLength(event.body) } From 462bfdfad60ab813a90faca0c6c64521aa716d55 Mon Sep 17 00:00:00 2001 From: Alex Brooke Date: Thu, 21 Jun 2018 15:11:04 -0500 Subject: [PATCH 4/5] forcibly inject Content-Length into event.headers - only if body is present and there are no headers - seems hacky, but ensures that the `x-apigateway-event` header is set properly(?) later (i.e. including the content-length field) --- index.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 2332d3f3..68a2840a 100644 --- a/index.js +++ b/index.js @@ -33,13 +33,16 @@ function isContentTypeBinaryMimeType(params) { function mapApiGatewayEventToHttpRequest(event, context, socketPath) { const headers = event.headers || {} // NOTE: Mutating event.headers; prefer deep clone of event.headers - const eventWithoutBody = Object.assign({}, event) - delete eventWithoutBody.body - // NOTE: API Gateway is not setting Content-Length header on any requests even when they have a body if (event.body && !headers['Content-Length']) { - headers['Content-Length'] = Buffer.byteLength(event.body) + // set header on event directly so x-apigateway-event is set correctly later + if (!event.headers) event.headers = {} + event.headers['Content-Length'] = Buffer.byteLength(event.body) + headers['Content-Length'] = Buffer.byteLength(event.body) } + const eventWithoutBody = Object.assign({}, event) + delete eventWithoutBody.body + headers['x-apigateway-event'] = encodeURIComponent(JSON.stringify(eventWithoutBody)) headers['x-apigateway-context'] = encodeURIComponent(JSON.stringify(context)) From c86713d0e6054307ab129fc369972cc4e2e42173 Mon Sep 17 00:00:00 2001 From: Alex Brooke Date: Thu, 21 Jun 2018 15:26:58 -0500 Subject: [PATCH 5/5] remove extraneous whitespace --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 68a2840a..2f614f74 100644 --- a/index.js +++ b/index.js @@ -67,7 +67,7 @@ function forwardResponseToApiGateway(server, response, context) { const bodyBuffer = Buffer.concat(buf) const statusCode = response.statusCode const headers = response.headers - + // chunked transfer not currently supported by API Gateway if (headers['transfer-encoding'] === 'chunked') delete headers['transfer-encoding']