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

LambdaHttpHandler does not properly handle multiple Accept headers #21559

Closed
freddieFishCake opened this issue Nov 19, 2021 · 13 comments · Fixed by #21744
Closed

LambdaHttpHandler does not properly handle multiple Accept headers #21559

freddieFishCake opened this issue Nov 19, 2021 · 13 comments · Fixed by #21744

Comments

@freddieFishCake
Copy link

freddieFishCake commented Nov 19, 2021

Scenario:

  1. Create a simple Quarkus REST application
  2. Verify that the application works locally with mvn quarkus:dev and calling your endpoint from a browser
  3. Add a dependency to quarkus-amazon-lambda-http
  4. Deploy to AWS using sam.cmd deploy (either jvm or native, it doesn't matter)
  5. Use a browser to test if the application works as expected by visiting the link produced by the SAM tool. Notice that whilst a request to the default index.html works fine, calls to your REST endpoints that are annotated with @get @produces(MediaType.APPLICATION_JSON) fail with HTTP 406

(Actually the deployment to AWS is not required to reproduce - the problem is still evident locally with mvn quarkus:dev after adding the dependency to quarkus-amazon-lambda-http)

The problem seems to be that LambdaHttpHandler splits the HTTP Accept headers before forwarding to the application:

Original header from the client with multiple accepted media types:
Accept: text/html,application/text,*/*

Request received after LambdaHttpHandler has done its magic:
Accept: text/html
Accept: application/text
Accept: */*

Splitting the accept header into multiple accept headers is OK from a HTTP perspective, but whichever component routes the request to the application appears to only use the first of these headers, so a typical REST service that can only produce application/json then responds with HTTP 406

The problem can be reproduced with non-lambda version as well with curl:
curl 'http://localhost:8080/someRestEndpoint' -H "Accept: application/xml" -H "Accept: application/json" -i -v

Switching the header order fixes or sidesteps the problem, because then application/json is first in the list.
curl 'http://localhost:8080/someRestEndpoint' -H "Accept: application/json" -H "Accept: application/xml" -i -v

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 19, 2021

/cc @matejvasek, @patriot1burke

@geoand
Copy link
Contributor

geoand commented Nov 26, 2021

I could not reproduce this problem at all.

What version of Quarkus are you using?

geoand added a commit to geoand/quarkus that referenced this issue Nov 26, 2021
This came about while trying to reproduce the
problem mentioned in quarkusio#21559
@freddieFishCake
Copy link
Author

freddieFishCake commented Nov 26, 2021

Hi, we are using Quarkus 2.3.1

Maven pom.xml:

<?xml version="1.0"?>
<project xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns="http://maven.apache.org/POM/4.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <modelVersion>4.0.0</modelVersion>
  <groupId>com.vaisala.sampo</groupId>
  <artifactId>forecast-service</artifactId>
  <version>1.0.0-SNAPSHOT</version>
  <name>sampo-forecast-service-quarkus</name>
  <properties>
    <compiler-plugin.version>3.8.1</compiler-plugin.version>
    <maven.compiler.parameters>true</maven.compiler.parameters>
    <maven.compiler.source>11</maven.compiler.source>
    <maven.compiler.target>11</maven.compiler.target>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
    <quarkus.platform.artifact-id>quarkus-bom</quarkus.platform.artifact-id>
    <quarkus.platform.group-id>io.quarkus.platform</quarkus.platform.group-id>
    <quarkus.platform.version>2.3.1.Final</quarkus.platform.version>
    <surefire-plugin.version>3.0.0-M5</surefire-plugin.version>
  </properties>
  <dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>${quarkus.platform.group-id}</groupId>
        <artifactId>${quarkus.platform.artifact-id}</artifactId>
        <version>${quarkus.platform.version}</version>
        <type>pom</type>
        <scope>import</scope>
      </dependency>
    </dependencies>
  </dependencyManagement>
  <dependencies>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-arc</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-resteasy-reactive</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-resteasy-reactive-jackson</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-rest-client-reactive-jackson</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-cache</artifactId>
    </dependency>
    <dependency>
      <groupId>org.immutables</groupId>
      <artifactId>value</artifactId>
      <version>2.8.2</version>
      <scope>provided</scope>
    </dependency>
    <dependency>
      <groupId>org.immutables</groupId>
      <artifactId>serial</artifactId>
      <version>2.8.2</version>
      <scope>provided</scope>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-junit5</artifactId>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>io.rest-assured</groupId>
      <artifactId>rest-assured</artifactId>
      <scope>test</scope>
    </dependency>
  </dependencies>
  <build>
    <plugins>
      <plugin>
        <groupId>${quarkus.platform.group-id}</groupId>
        <artifactId>quarkus-maven-plugin</artifactId>
        <version>${quarkus.platform.version}</version>
        <extensions>true</extensions>
        <executions>
          <execution>
            <goals>
              <goal>build</goal>
              <goal>generate-code</goal>
              <goal>generate-code-tests</goal>
            </goals>
          </execution>
        </executions>
      </plugin>
      <plugin>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>${compiler-plugin.version}</version>
        <configuration>
          <parameters>${maven.compiler.parameters}</parameters>
        </configuration>
      </plugin>
      <plugin>
        <artifactId>maven-surefire-plugin</artifactId>
        <version>${surefire-plugin.version}</version>
        <configuration>
          <systemPropertyVariables>
            <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
            <maven.home>${maven.home}</maven.home>
          </systemPropertyVariables>
        </configuration>
      </plugin>
    </plugins>
  </build>
  <profiles>
    <profile>
      <id>native</id>
      <activation>
        <property>
          <name>native</name>
        </property>
      </activation>
      <build>
        <plugins>
          <plugin>
            <artifactId>maven-failsafe-plugin</artifactId>
            <version>${surefire-plugin.version}</version>
            <executions>
              <execution>
                <goals>
                  <goal>integration-test</goal>
                  <goal>verify</goal>
                </goals>
                <configuration>
                  <systemPropertyVariables>
                    <native.image.path>${project.build.directory}/${project.build.finalName}-runner</native.image.path>
                    <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
                    <maven.home>${maven.home}</maven.home>
                  </systemPropertyVariables>
                </configuration>
              </execution>
            </executions>
          </plugin>
        </plugins>
      </build>
      <properties>
        <quarkus.package.type>native</quarkus.package.type>
      </properties>
    </profile>
  </profiles>
</project>

@geoand
Copy link
Contributor

geoand commented Nov 26, 2021

Can you try with Quarkus 2.5.0.Final please?

@freddieFishCake
Copy link
Author

Sure, your question prompted me to check the latest version and I just noticed that 2.5 has just been released ;-)

@freddieFishCake
Copy link
Author

TLDR - upgrading to Quarkus 2.5.0 hasn't helped, sorry about that.

Result with version 2.3.1 when passing application/xml as the first accept header:

pjl@pjl-ThinkPad-P1-Gen-3:~$ curl 'http://localhost:8080/api/v1/forecasts/latest?latitude=60.32&longitude=24.97&altitude=23.4&duration=PT3H&resolution=PT15M&validTime=2021-10-27T11:00:00Z' -H "Accept: application/xml" -H "Accept: application/json" -i -v
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /api/v1/forecasts/latest?latitude=60.32&longitude=24.97&altitude=23.4&duration=PT3H&resolution=PT15M&validTime=2021-10-27T11:00:00Z HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.68.0
> Accept: application/xml
> Accept: application/json
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 406 Not Acceptable
HTTP/1.1 406 Not Acceptable
< content-length: 0
content-length: 0

Result with 2.3.1 when switching the accept headers around:

pjl@pjl-ThinkPad-P1-Gen-3:~$ curl 'http://localhost:8080/api/v1/forecasts/latest?latitude=60.32&longitude=24.97&altitude=23.4&duration=PT3H&resolution=PT15M&validTime=2021-10-27T11:00:00Z' -H "Accept: application/json" -H "Accept: application/xml" -i -v
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /api/v1/forecasts/latest?latitude=60.32&longitude=24.97&altitude=23.4&duration=PT3H&resolution=PT15M&validTime=2021-10-27T11:00:00Z HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.68.0
> Accept: application/json
> Accept: application/xml
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
HTTP/1.1 200 OK
< transfer-encoding: chunked
transfer-encoding: chunked
< Content-Type: application/json
Content-Type: application/json
{json response here}

And then with Quarkus 2.5.0

jl@pjl-ThinkPad-P1-Gen-3:~$ curl 'http://localhost:8080/api/v1/forecasts/latest?latitude=60.32&longitude=24.97&altitude=23.4&duration=PT3H&resolution=PT15M&validTime=2021-10-27T11:00:00Z' -H "Accept: application/xml" -H "Accept: application/json" -i -v
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /api/v1/forecasts/latest?latitude=60.32&longitude=24.97&altitude=23.4&duration=PT3H&resolution=PT15M&validTime=2021-10-27T11:00:00Z HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.68.0
> Accept: application/xml
> Accept: application/json
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 406 Not Acceptable
HTTP/1.1 406 Not Acceptable
< content-length: 0
content-length: 0

< 
* Connection #0 to host localhost left intact

reversing the order of the accept headers:

pjl@pjl-ThinkPad-P1-Gen-3:~$ curl 'http://localhost:8080/api/v1/forecasts/latest?latitude=60.32&longitude=24.97&altitude=23.4&duration=PT3H&resolution=PT15M&validTime=2021-10-27T11:00:00Z' -H "Accept: application/json" -H "Accept: application/xml" -i -v
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /api/v1/forecasts/latest?latitude=60.32&longitude=24.97&altitude=23.4&duration=PT3H&resolution=PT15M&validTime=2021-10-27T11:00:00Z HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.68.0
> Accept: application/json
> Accept: application/xml
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
HTTP/1.1 200 OK
< transfer-encoding: chunked
transfer-encoding: chunked
< Content-Type: application/json
Content-Type: application/json

@freddieFishCake
Copy link
Author

Output from mvn quarkus:dev to confirm the version

pjl@pjl-ThinkPad-P1-Gen-3:~/Source/SAMPO/sampo-forecast-service-quarkus$ mvn quarkus:dev 
[INFO] Scanning for projects...
[INFO] 
[INFO] -----------------< com.vaisala.sampo:forecast-service >-----------------
[INFO] Building sampo-forecast-service-quarkus 1.0.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- quarkus-maven-plugin:2.5.0.Final:dev (default-cli) @ forecast-service ---
[INFO] Invoking org.apache.maven.plugins:maven-resources-plugin:2.6:resources) @ forecast-service
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 2 resources
[INFO] Invoking io.quarkus.platform:quarkus-maven-plugin:2.5.0.Final:generate-code) @ forecast-service
[INFO] Invoking org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile) @ forecast-service
[INFO] Nothing to compile - all classes are up to date
[INFO] Invoking org.apache.maven.plugins:maven-resources-plugin:2.6:testResources) @ forecast-service
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /home/pjl/Source/SAMPO/sampo-forecast-service-quarkus/src/test/resources
[INFO] Invoking io.quarkus.platform:quarkus-maven-plugin:2.5.0.Final:generate-code-tests) @ forecast-service
[INFO] Invoking org.apache.maven.plugins:maven-compiler-plugin:3.8.1:testCompile) @ forecast-service
[INFO] Nothing to compile - all classes are up to date
Listening for transport dt_socket at address: 5005
__  ____  __  _____   ___  __ ____  ______ 
 --/ __ \/ / / / _ | / _ \/ //_/ / / / __/ 
 -/ /_/ / /_/ / __ |/ , _/ ,< / /_/ /\ \   
--\___\_\____/_/ |_/_/|_/_/|_|\____/___/   
2021-11-26 15:13:51,773 INFO  [io.quarkus] (Quarkus Main Thread) forecast-service 1.0.0-SNAPSHOT on JVM (powered by Quarkus 2.5.0.Final) started in 1.690s. Listening on: http://localhost:8080

2021-11-26 15:13:51,787 INFO  [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
2021-11-26 15:13:51,788 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cache, cdi, jaxrs-client-reactive, rest-client-reactive, rest-client-reactive-jackson, resteasy-reactive, resteasy-reactive-jackson, smallrye-context-propagation, vertx]

--
Tests paused
Press [r] to resume testing, [o] Toggle test output, [h] for more options>

@geoand
Copy link
Contributor

geoand commented Nov 26, 2021

Can you attach a sample project that exhibits this behavior?

@freddieFishCake
Copy link
Author

The test application here is basically a wrapper for a different web service - we use the RestEasy Client to call the remote project, I am just wondering if there is some unexpected interaction between RestEasy and the client?
.. cross post with your request for a sample project - sure I will try to do that asap.

@patriot1burke
Copy link
Contributor

THis looks like a resteasy reactive issue.

@geoand
Copy link
Contributor

geoand commented Nov 26, 2021

Good point - I didn't see that RR was being used, so I'll give it another pass.

@geoand
Copy link
Contributor

geoand commented Nov 26, 2021

Turned out it was a RESTEasy Reactive issue after all.

geoand added a commit to geoand/quarkus that referenced this issue Nov 26, 2021
geoand added a commit to geoand/quarkus that referenced this issue Nov 26, 2021
Postremus pushed a commit to Postremus/quarkus that referenced this issue Nov 27, 2021
This came about while trying to reproduce the
problem mentioned in quarkusio#21559
@freddieFishCake
Copy link
Author

Excellent, many thanks for your help!

geoand added a commit that referenced this issue Nov 29, 2021
Take multiple Accept headers case into account in RESTEasy Reactive
@quarkus-bot quarkus-bot bot added this to the 2.6 - main milestone Nov 29, 2021
@gsmet gsmet modified the milestones: 2.6 - main, 2.5.1.Final Nov 29, 2021
gsmet pushed a commit to gsmet/quarkus that referenced this issue Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants