Parse json body when content-type header is lower-cased

[vladholubiev]

Looks like a strange exception. Didn't work for me without this fix

See https://serverless.com/framework/docs/providers/aws/events/apigateway#example-lambda-proxy-event-default

[codecov]

Codecov Report

Merging #75 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #75   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          10     10           
  Lines         165    167    +2     
  Branches       37     39    +2     
=====================================
+ Hits          165    167    +2

Impacted Files	Coverage Δ
src/middlewares/jsonBodyParser.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfc6438...24e2f11. Read the comment docs.

[lmammino]

That it's definitely interesting. At a first glance, it would look more like an issue with the current client making the request, rather than some weird internals.

Let me run some tests to try to clarify when this happens. Meanwhile, how are you generating the requests to your lambdas?

[lmammino]

It seems that API Gateway does not normalize incoming headers and they are transferred exactly with the casing specified by the client (see attached screenshot).

If the client pass Content-Type (with the right casing) the middleware should work as expected.

At the moment I would be against merging this because it tries to prevent an error in the client. I would instead encourage a client to follow the standard with care and submit headers with the expected casing.

With that being said I'd love an opinion from the other members of the team: @DavidWells @peterjcaulfield @dkatavic @padraigobrien @techfort @acambas

[dkatavic]

Looking at the specification RFC7230
Each header field consists of a case-insensitive field... we should accept lowercased Content-Type header. Problem is that in your solution, we are checking only for canonical format and all lowercase, not for all the other combinations.

Implementing this check properly isn't easy in JS, we would need to get all the keys of the headers and check is any of them matching the value Content-Type on the case-insensitive way.

Is there a possibility that we can have multiple Content-Type headers, like {"Content-type": "foo", "content-type": "bar"}?
@lmammino

[lmammino]

Considering what @dkatavic just said it might make sense to address the issue at middy's level.
Either we do the double check for "canonical" and "lowercase" headers (which is not perfect but should work most of the time), or we can create an additional middleware to normalize the headers and that should run before this one.

Also, as a separate matter, we should consider the case when the Content-Type contains also the encoding option as specified here and as similarly discussed in #82.

I will have a thought about these matters during the weekend and try to come up with a solution.

[lmammino]

After thinking about this for a while, I think the best solution, in order to keep things fast and simple, is to make middlewares check only the canonical representation of a header. If in a given application the header might come with another casing (eg. lowercase), you can easily write an additional middleware that takes care of the serialization as in the (untested) example below:

middy(handler)
  .before((handler, next) => {
    if (!handler.event.headers['Content-Type'] && handler.event.headers['content-type']) {
       handler.event.headers['Content-Type'] = handler.event.headers['content-type']
       delete handler.event.headers['content-type']
    }

    return next()
  })
  .use(jsonBodyParser())

Of course, you can create your own reusable middleware based on this example if you need this feature in multiple handlers.

@vladgolubev Based on this reasoning, I am closing this PR, but feel free to comment here if you have more questions/concerns.