http-json-body-parser: Body should be allowed to be undefined when Content-Length is 0 or missing

[bpadgette]

Is your feature request related to a problem? Please describe.

The middy middleware http-json-body-parser returns an HTTP 415 Unprocessable Entity response whenever the request body is undefined.

This case is tested: awesome

As test "It should handle undefined as an UnprocessableEntity" in packages/http-json-body-parser/__\tests__/index.js states, an undefined request body will result in the HTTP 415.

I propose that an undefined body is not a generally justifiable cause for an HTTP 415.

Consider this AWS API Gateway scenario:

Send a GET request with content-type: application/json but no content-length header, and AWS Gateway would parse that request and pass it on to Middy with no body key (meaning event.body === undefined) in packages/http-json-body-parser/index.js.

Describe the solution you'd like

In the middleware http-json-body-parser:

IF `event.body === undefined` AND the content-length header is 0 or undefined
   ignore event, do not try to parse as JSON body

Describe alternatives you've considered

My team currently defines our own http-json-body-parser wrapper that implements a version of this solution.

We also believe that content-type without content-length is an anti-pattern but not illegal in its own right, but it is besides the point since we had undefined body fields making it to middy, passing the basic parsing of AWS API Gateway to get there.

Additional context

My team uses middy on a project consuming HTTP events from AWS API Gateway, and we noticed that the body keys of our events were undefined which caused the aforementioned HTTP 415 to show on several GET routes.

Our SDK client in development was sending GET requests with a default inserted content-type: application/json header but no content-length header, and AWS API Gateway passed events with no body key to our middy middleware object.

Since the test case is so clear, I suggest that the context it provides is sufficient.

[bpadgette]

I have created a pull request that implements my suggested fix #1267

Thank you for your time and work on this great library :)

[willfarrell]

Thanks for the report, and a PR. But, why include the middleware to begin with? As you mentioned GET don't have a body, thus don't need this middleware.

[bpadgette]

Thanks for your response

why include the middleware to begin with? As you mentioned GET don't have a body, thus don't need this middleware.

This is a great point: a GET route does not need http-json-body-parser, but keeping a single serialization middleware stack is common and convenient.

The option disableContentTypeError
I looked into a related option for this middleware: disableContentTypeError

disableContentTypeError (boolean) (optional): Skip throwing 415 when Content-Type is invalid. Default: true, will default to false in next major version.

Seeing that middy has plans to make this middleware more strict in the next major version, requiring a conscious decision to enable disableContentTypeError by the developer, perhaps this option could cover this case of undefined body values, implementing a version of my suggested solution?

This way, the same use-case of disabling content type errors (probably for common serialization middleware stacks validating any sort of incoming request) could be extended to cover the case where the body could be reasonably undefined

What do you think about this in-the-middle approach?

[willfarrell]

Those docs are out of date (now updated), sorry about that. The default is false as of v6.0.0.

Because you're sharing middlewares across multiple endpoints (where at least one is POST). I'm worried that if we add in the logic, there may be a vector for attacker to pass in a content-length of 0 when there is a length, causing the handler to do some unexpected. I'll reflect on this more though.

From a security and performance stand point I would suggest having more than 1 shared middleware stacks or build the stack dynamically based on options. I personally use both these patterns in my projects.