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

Commas in Headers with AWS Lambda #25580

Closed
c-classen opened this issue May 14, 2022 · 7 comments · Fixed by #25666
Closed

Commas in Headers with AWS Lambda #25580

c-classen opened this issue May 14, 2022 · 7 comments · Fixed by #25666
Labels
area/amazon-lambda kind/bug Something isn't working
Milestone

Comments

@c-classen
Copy link
Contributor

Describe the bug

When you are dealing with headers that can have commas in their value, such as Access-Control-Request-Headers, the Lambda Handler seems to interpret them as different header values which causes CORS to not work correctly.

Expected behavior

When executing the curl

curl http://localhost:8080/hello -X OPTIONS -H 'Origin: http://example.com' -H 'Access-Control-Request-Headers: bar,foo' -v

the following header is returned

access-control-allow-headers: bar,foo

Actual behavior

The following header is returned

access-control-allow-headers: bar

How to Reproduce?

  1. Generate a new quarkus project with "AWS Lambda HTTP" and "RestEasy Classic Jackson" extension
  2. Add the following lines to the application.properties:
quarkus.http.cors=true
quarkus.http.cors.origins=http://example.com
  1. Run ./mvnw quarkus:dev
  2. Execute the curls given above
  3. Uncomment the "quarkus-amazon-lambda-http" in the pom.xml to see the expected behaviour

Output of uname -a or ver

Linux [REMOVED] 5.13.0-40-generic #45~20.04.1-Ubuntu SMP Mon Apr 4 09:38:31 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "14.0.2" 2020-07-14

GraalVM version (if different from Java)

N/A, the issue can be reproduced without GraalVM

Quarkus version or git rev

2.9.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.4

Additional information

Event though the reproduction steps do not include deployment to Lambda, I have observed the same behavior with a different project also when it was deployed to AWS Lambda

In the io.quarkus.vertx.http.runtime.cors.CORSFilter class, only the first header value is retrieved in this line:

final String requestedHeaders = request.getHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS);

But in the io.quarkus.amazon.lambda.http.LambdaHttpHandler, the string parts separated by comma are stored as different header values:

for (String val : header.getValue().split(","))
  nettyRequest.headers().add(header.getKey(), val);

This is probably necessary because the API Gateway will send multiple header values separated with comma and there is no way to reconstruct the original request from the API Gateway's event. Maybe it would be an acceptable solution to have the CORSFilter expect multiple header values

@c-classen c-classen added the kind/bug Something isn't working label May 14, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented May 14, 2022

/cc @matejvasek, @patriot1burke

@jklingsporn
Copy link

I am experiencing the same issue. While debugging, I figured the following line in CORSFilter is wrong / suspicious:

final String requestedHeaders = request.getHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS);

getHeader returns the first header of the header-MultiMap. It should be rather something like:

final List<String> requestedHeaders = request.headers().getAll(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS);

@c-classen
Copy link
Contributor Author

getHeader returns the first header of the header-MultiMap. It should be rather something like:

final List<String> requestedHeaders = request.headers().getAll(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS);

Event though I think the current code is correct if you go by the documentation here (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Request-Headers), your suggestion seems to be a good workaround for the ambiguity of the API Gateway's events regarding header values.

@patriot1burke
Copy link
Contributor

patriot1burke commented May 18, 2022

I could add a lookup map where if a header like Access-Control-Request-Headers is sent to NOT split it up into multiple headers. I think that might be the better option. I know there are other Comma sensitive headers too. I don't think Access-Control-Request-Headers was meant to be a multi-header so I don't think changing CorsFilter is the right approach. Thoughts?

@c-classen
Copy link
Contributor Author

Sounds good! This would also have the advantage of the fix only being in the Lambda library and not having a workaround lying around in applications that do not use Lambda. The fix should optimally cover all headers that are used in Quarkus extensions.

Maybe it would also be nice to have a hint about this issue in the Quarkus Lambda documentation so people who use custom headers with commas know how to retrieve them.

Thank you for addressing this issue. At our company we really like the possibility of running our backends in AWS Lambda as it should save us some money especially when using multiple stages. But this issue is a real blocker for us, since we cannot send any requests that modify data from our frontend as they require both the Content-Type and Authorization header.

@jklingsporn
Copy link

@c-classen I've used this hack for local development:

quarkus.http.header."access-control-allow-origin".value=http://localhost:8081
quarkus.http.header."access-control-allow-origin".methods=OPTIONS,POST,GET
quarkus.http.header."access-control-allow-methods".value=OPTIONS,POST,GET
quarkus.http.header."access-control-allow-methods".methods=OPTIONS,POST,GET
quarkus.http.header."access-control-allow-headers".value=authorization,content-type,origin
quarkus.http.header."access-control-allow-headers".methods=OPTIONS,POST,GET

It adds the headers to every request as described here.

@c-classen
Copy link
Contributor Author

@jklingsporn Thanks for the workaround. However, this is somewhat limited since afaik you can only have one allowed origin this way. I think I'll wait for a Quarkus version without this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/amazon-lambda kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants