Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ALB multi value headers #189

Conversation

jurasofish
Copy link
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

mangum/handlers/aws_alb.py Outdated Show resolved Hide resolved
mangum/handlers/aws_alb.py Show resolved Hide resolved
mangum/handlers/aws_alb.py Outdated Show resolved Hide resolved
mangum/handlers/aws_alb.py Outdated Show resolved Hide resolved
@jordaneremieff
Copy link
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
mangum/handlers/aws_alb.py Show resolved Hide resolved
mangum/handlers/aws_alb.py Outdated Show resolved Hide resolved
@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
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