-
Notifications
You must be signed in to change notification settings - Fork 348
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: do not duplicate headers (single val, multi val) on return #852
fix: do not duplicate headers (single val, multi val) on return #852
Conversation
If required, I could also provide logic that actually splits the headers into single-value and multi-value headers and returns them as they should be returned. Edit: Actually, this seems non-trivial and this current simpler solution is most likely preferable. |
According to the docs, this change is not necessary:
It'd be necessary if you constructed the APIGW response event with different keys in the headers. Am I missing anything? |
Yup, that's the specification, but the real implementation does not behave that way unfortunately. See the linked PR above as well. Obviously, this is a workaround, although the "correctness" of the current implementation is also questionable, as there is no need to specify the same headers twice either. We have a web app that is supposed to be deployed as a Lambda with an API gateway in front, and we can't call its requests from Javascript, as the Browser treats duplicate CORS headers as a CORS error and blocks the requests. We are using https://github.com/lazear/axum-aws-lambda, which in turn calls this library. Seeing that the issue has not been fixed on AWS side for the at least 4 years that it has been known, this workaround seems like the most practical solution for the present time, although I can understand the hesitation considering the clear wording of the specification stating that the current implementation is fine. |
In fact, you can inspect the issue in action if you navigate to this endpoint, which is served via the Lambda in question: https://compass.api.labbase.jp/health_check You will note how a lot of the response header (including the CORS-relevant allow-credentials) are duplicated. The ones not duplicated are the ones added or updated by CloudFront, which is sitting in-between, or the API Gateway itself.
|
Well, that's unfortunate. The change looks safe. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@calavera Thanks a lot for the fast response! We have re-built our application with a git dependency on this crate (and the intermittent one) and deployed it. You can confirm that the header duplication has indeed been resolved by visiting the above link again. |
Issue
This is the Rust equivalent to these changes made in the Go runtime library for lambda HTTP: awslabs/aws-lambda-go-api-proxy#73
The issue here is that the API Gateway does not properly merge single-value headers and multi-value returned by the Lambda and instead returns both to the client. Since this library just puts the same headers into both, this means that every header is returned twice (more or less, in the case of actual multi-value headers, only the first element is duplicated). In the case of CORS, this is fatal, as browsers generally reject duplicate CORS headers. Unless this change can go through, we would be forced to turn off application-level CORS handling and rely on the API gateway instead (which we would really like to avoid..).
Description of changes:
Adapts the code to return all headers as multi-value headers only, returning an empty
HeaderMap
for the single-value headers.By submitting this pull request