Skip to content

Fix ALB multi value headers#189

Merged
jordaneremieff merged 4 commits intoKludex:mainfrom
jurasofish:multivalue_headers_querystrings_ii_electric_boogaloo
Jun 5, 2021
Merged

Fix ALB multi value headers#189
jordaneremieff merged 4 commits intoKludex:mainfrom
jurasofish:multivalue_headers_querystrings_ii_electric_boogaloo

Conversation

@jurasofish
Copy link
Copy Markdown
Contributor

Fix broken multi value headers for ALB, as described in #187.
This is largely a copy past of #179 just with some small changes.

Sorry the changes to testing are so substantial, just had to add a lot of logic to handle with/without multi value headers.

Closes #187
Closes #179

I spoke to AWS support about the case mutations.
They wouldn't guarantee it as a supported way of using ALB, but they didn't say it wasn't supported either....
They guy just spun up an ALB and confirmed empirically that it worked, which we already knew.
Whatever, it didn't hurt to ask

Comment thread mangum/handlers/aws_alb.py Outdated
Comment thread mangum/handlers/aws_alb.py
Comment thread mangum/handlers/aws_alb.py Outdated
Comment thread mangum/handlers/aws_alb.py Outdated
@jordaneremieff
Copy link
Copy Markdown
Collaborator

jordaneremieff commented May 23, 2021

@jurasofish thanks! The ALB support in Mangum is still fairly new territory and not something I'm particularly across, so I appreciate the investigation/fixing up the test coverage.

I've opened a review with some questions. Mainly I'm trying to understand the differences between the initial implementation, the refactored version, and the new changes in your PR.

…t[bytes]]` up to where the headers are initially transformed
@jurasofish jurasofish requested a review from jordaneremieff May 23, 2021 08:53
Comment thread mangum/handlers/aws_alb.py
Comment thread mangum/handlers/aws_alb.py Outdated
@jurasofish jurasofish requested a review from jordaneremieff May 30, 2021 05:16
@jordaneremieff jordaneremieff merged commit d9ea835 into Kludex:main Jun 5, 2021
@jordaneremieff
Copy link
Copy Markdown
Collaborator

Thanks!

khamaileon pushed a commit to khamaileon/mangum that referenced this pull request Jan 13, 2024
* copy with some slight changes from Kludex#179

* move headers conversion from `List[Tuple[bytes, bytes]]` to `List[List[bytes]]` up to where the headers are initially transformed

* no longer assert that first case mutated string is lowercase

* only pull headers from one of `multiValueHeaders` and `headers`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants