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

RESTEasy Reactive does not execute response filters for OPTIONS requests #26828

Closed
Foobartender opened this issue Jul 20, 2022 · 7 comments · Fixed by #26954
Closed

RESTEasy Reactive does not execute response filters for OPTIONS requests #26828

Foobartender opened this issue Jul 20, 2022 · 7 comments · Fixed by #26954
Assignees
Labels
area/resteasy-reactive kind/bug Something isn't working
Milestone

Comments

@Foobartender
Copy link
Contributor

Describe the bug

JAX-RS ContainerResponseFilters as well as Quarkus-style @ServerResponseFilters are not executed for OPTIONS method requests by RESTEasy Reactive. Works with RESTEasy Classic (the former only, as the latter is unavailable).

Expected behavior

They should be executed.

Actual behavior

They aren't.

How to Reproduce?

Please see the demo project: crf-test.tar.gz

  1. Run ./gradlew test. All tests will pass.
  2. In build.gradle.kts, replace io.quarkus:quarkus-resteasy with io.quarkus:quarkus-resteasy-reactive.
  3. Run ./gradlew test again. The OPTIONS test won't pass.

Note that there is no Quarkus-style @ServerResponseFilter test, because it's unavailable for RESTEasy classic. But it's affected, too.

Output of uname -a or ver

Linux ***** 5.18.12-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 15 Jul 2022 15:33:02 +0000 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.15" 2022-04-19 OpenJDK Runtime Environment GraalVM CE 22.1.0 (build 11.0.15+10-jvmci-22.1-b06) OpenJDK 64-Bit Server VM GraalVM CE 22.1.0 (build 11.0.15+10-jvmci-22.1-b06, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.10.3.Final

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

Gradle 7.4.2

Additional information

This bug hinders implementing custom CORS filters.

@Foobartender Foobartender added the kind/bug Something isn't working label Jul 20, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 20, 2022

/cc @FroMage, @geoand, @stuartwdouglas

@geoand
Copy link
Contributor

geoand commented Jul 26, 2022

The test in your sample worked perfectly - the filter was executed and the header added.

Is there anything you are missing?

@Foobartender
Copy link
Contributor Author

Did you follow step 2., i.e. replace RESTEasy Classic with Reactive?

@geoand
Copy link
Contributor

geoand commented Jul 26, 2022

My bad 🤦🏼

@geoand
Copy link
Contributor

geoand commented Jul 26, 2022

I checked and in my opinion, the RESTEasy Reactive's behavior is more correct. If you actually need to have the filters run, then you need to have a Resource Method that actually handles @OPTIONS.

@FroMage WDYT?

@Foobartender
Copy link
Contributor Author

I looked at the JAX-RS spec. It indeed only says that globally bound ContainerResponseFilters should be executed for all resource methods. It doesn't say anything about non-existing resource methods.

However, I added some more tests to explore the behavior, and found that the filter is executed for non-existing resource methods, too, except for OPTIONS with RESTEasy Reactive.

All of the following tests pass with RESTEasy Classic. Only the OPTIONS tests fail with RESTEasy Reactive. That doesn't really make sense.

package com.example

import io.quarkus.test.common.http.TestHTTPEndpoint
import io.quarkus.test.junit.QuarkusTest
import io.restassured.module.kotlin.extensions.Given
import io.restassured.module.kotlin.extensions.Then
import io.restassured.module.kotlin.extensions.When
import org.hamcrest.Matchers.`is`
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.Test



@Suppress("UastIncorrectHttpHeaderInspection")
@QuarkusTest
@TestHTTPEndpoint(ExampleResource::class)
class ExampleResourceTest {
 @Test
 fun `GET hello`() {
  When {
   get()
  } Then {
   statusCode(200)
   header("X-Foo", "Bar")
   body(`is`("Hello from RESTEasy Reactive"))
  }
 }


 @Test
 fun `OPTIONS hello`() {
  When {
   options()
  } Then {
   statusCode(200)
   header("X-Foo", "Bar")
  }
 }


 @Test
 fun `DELETE hello`() {
  When {
   delete()
  } Then {
   statusCode(405)
   header("X-Foo", "Bar")
  }
 }


 @Test
 fun `GET doesnotexist`() {
  When {
   get("doesnotexist")
  } Then {
   statusCode(404)
   header("X-Foo", "Bar")
  }
 }


 @Test
 fun `OPTIONS doesnotexist`() {
  When {
   options("doesnotexist")
  } Then {
   statusCode(404)
   header("X-Foo", "Bar")
  }
 }


 @Test
 fun `DELETE doesnotexist`() {
  When {
   delete("doesnotexist")
  } Then {
   statusCode(404)
   header("X-Foo", "Bar")
  }
 }
}

@geoand
Copy link
Contributor

geoand commented Jul 26, 2022

Indeed that doesn't make for a consistent experience.

I'll have a closer look

@geoand geoand self-assigned this Jul 27, 2022
geoand added a commit to geoand/quarkus that referenced this issue Jul 27, 2022
This is done in order to be more consistent with how
other cases are handled and how RESTEasy Classic handles things as well

Fixes: quarkusio#26828
geoand added a commit to geoand/quarkus that referenced this issue Jul 27, 2022
This is done in order to be more consistent with how
other cases are handled and how RESTEasy Classic handles things as well

Fixes: quarkusio#26828
geoand added a commit to geoand/quarkus that referenced this issue Aug 24, 2022
This is done in order to be more consistent with how
other cases are handled and how RESTEasy Classic handles things as well

Fixes: quarkusio#26828
@quarkus-bot quarkus-bot bot added this to the 2.13 - main milestone Aug 24, 2022
geoand added a commit that referenced this issue Aug 24, 2022
Ensure that HTTP OPTIONS handling involves the proper handler chain
fercomunello pushed a commit to fercomunello/quarkus that referenced this issue Aug 31, 2022
This is done in order to be more consistent with how
other cases are handled and how RESTEasy Classic handles things as well

Fixes: quarkusio#26828
@gsmet gsmet modified the milestones: 2.13 - main, 2.12.1.Final Sep 5, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Sep 5, 2022
This is done in order to be more consistent with how
other cases are handled and how RESTEasy Classic handles things as well

Fixes: quarkusio#26828
(cherry picked from commit c8380a0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/resteasy-reactive kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants