Skip to content

Conversation

@erunion
Copy link
Member

@erunion erunion commented Oct 16, 2020

This fixes a regression that was introduced in the recent header case-insensitivity work (Kong#178) that is causing edge cases where, on certain targets, it's possible for a header to be:

  • Duplicated and both instances having differing data. This was mostly apparent on multipart/form-data requests where one header would just be multipart/form-data while the other would be that plus the boundary.
  • Some targets would completely fail to interpret the header, resulting in either an undefined header being set, being set in ways that it shouldn't be, or the target completely failing to function because a header wasn't being removed.

To resolve this I've:

  • Added a new headers helper with a couple methods to traverse the now case-insensitive object of headers so that these targets that need special case handling for something like content-type can safely search for that in the available headers.
  • Updated every target that was looking for case-sensitive headers to now use this new helper to do the same work it needs to do.

For testing all of this, I've updated the multipart/form-data test fixture to have its content-type header as Content-Type to force this case-insensitive lookup for targets that need it.

😰

@erunion erunion added the bug Something isn't working label Oct 16, 2020
@domharrington
Copy link
Member

Have you considered using https://www.npmjs.com/package/caseless for this?

@erunion
Copy link
Member Author

erunion commented Oct 16, 2020

caseless should probably fully replace the headerObj object that the library has, but I might PR that upstream in another PR since that's going to be a larger refactor than this.

@erunion erunion merged commit 1b4e5ef into master Oct 16, 2020
@erunion erunion deleted the fix/header-insensitivity-issues branch October 16, 2020 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants