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

ContainerResponseFilter with @Priority(Integer.MIN_VALUE) will be actually invoked with max priority #44229

Closed
dteleguin opened this issue Oct 31, 2024 · 4 comments · Fixed by #44237
Labels
area/rest kind/bug Something isn't working
Milestone

Comments

@dteleguin
Copy link
Contributor

dteleguin commented Oct 31, 2024

Describe the bug

According to the specification:

Execution chains for extension points ContainerResponse and ClientResponse are sorted in descending order; the higher the number the higher the priority. These rules ensure that response filters are executed in reversed order of request filters.

With Quarkus REST (and previously Resteasy Reactive), a ContainerResponseFilter having @Priority(Integer.MIN_VALUE) will be actually invoked the first in the chain, even before @Priority(Integer.MIN_VALUE). This is opposite to Resteasy Classic, where it would be executed the last.

Thus, migration to Quarkus REST could break projects that depend on filter ordering and use @Priority(Integer.MIN_VALUE) to ensure minimal priority for the filters.

Expected behavior

The filters should be invoked in the following order:

N @Priority
1 Integer.MAX_VALUE
2 Priorities.USER
3 0
4 Integer.MIN_VALUE + 1
5 Integer.MIN_VALUE

Actual behavior

The filters are invoked in the following order:

N @Priority
1 Integer.MIN_VALUE
2 Integer.MAX_VALUE
3 Priorities.USER
4 0
5 Integer.MIN_VALUE + 1

How to Reproduce?

Reproducer: https://github.com/dteleguin/filter-priority-poc

./mvnw -Presteasy-classic clean test

2024-10-31 14:05:10,149 INFO  [io.quarkus] (main) filter-priority-poc 1.0.0-SNAPSHOT on JVM (powered by Quarkus 3.16.1) started in 2.944s. Listening on: http://localhost:8081
2024-10-31 14:05:10,152 INFO  [io.quarkus] (main) Profile test activated. 
2024-10-31 14:05:10,152 INFO  [io.quarkus] (main) Installed features: [cdi, resteasy, resteasy-jackson, smallrye-context-propagation, vertx]
2024-10-31 14:05:11,379 INFO  [org.acm.fil.FilterMAX] (executor-thread-1) filter
2024-10-31 14:05:11,379 INFO  [org.acm.fil.FilterDEF] (executor-thread-1) filter
2024-10-31 14:05:11,380 INFO  [org.acm.fil.Filter0] (executor-thread-1) filter
2024-10-31 14:05:11,380 INFO  [org.acm.fil.FilterMIN1] (executor-thread-1) filter
2024-10-31 14:05:11,380 INFO  [org.acm.fil.FilterMIN] (executor-thread-1) filter

./mvnw -Presteasy-reactive clean test

2024-10-31 14:05:49,341 INFO  [io.quarkus] (main) filter-priority-poc 1.0.0-SNAPSHOT on JVM (powered by Quarkus 3.16.1) started in 3.348s. Listening on: http://localhost:8081
2024-10-31 14:05:49,344 INFO  [io.quarkus] (main) Profile test activated. 
2024-10-31 14:05:49,344 INFO  [io.quarkus] (main) Installed features: [cdi, rest, smallrye-context-propagation, vertx]
2024-10-31 14:05:50,678 INFO  [org.acm.fil.FilterMIN] (executor-thread-1) filter
2024-10-31 14:05:50,678 INFO  [org.acm.fil.FilterMAX] (executor-thread-1) filter
2024-10-31 14:05:50,679 INFO  [org.acm.fil.FilterDEF] (executor-thread-1) filter
2024-10-31 14:05:50,679 INFO  [org.acm.fil.Filter0] (executor-thread-1) filter
2024-10-31 14:05:50,679 INFO  [org.acm.fil.FilterMIN1] (executor-thread-1) filter

Output of uname -a or ver

No response

Output of java -version

java version "21.0.1" 2023-10-17 LTS

Quarkus version or git rev

3.16.1

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

Apache Maven 3.9.9

Additional information

No response

@dteleguin dteleguin added the kind/bug Something isn't working label Oct 31, 2024
Copy link

quarkus-bot bot commented Oct 31, 2024

/cc @FroMage (rest), @stuartwdouglas (rest)

@geoand
Copy link
Contributor

geoand commented Oct 31, 2024

Thanks for reporting.

I'll have a look

geoand added a commit to geoand/quarkus that referenced this issue Oct 31, 2024
@dteleguin dteleguin changed the title ContainerResponseFilter with @Priority(Integer.MIN_VALUE) will be actually invoked with max priority ContainerResponseFilter with @Priority(Integer.MIN_VALUE) will be actually invoked with max priority Oct 31, 2024
@geoand geoand closed this as completed in a6edf30 Nov 1, 2024
geoand added a commit that referenced this issue Nov 1, 2024
Properly implement priority of ContainerResponseFilter
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Nov 1, 2024
@dteleguin
Copy link
Contributor Author

Great job @geoand 💪

@geoand
Copy link
Contributor

geoand commented Nov 1, 2024

Thanks for reporting the issue!

@gsmet gsmet modified the milestones: 3.17 - main, 3.16.2 Nov 5, 2024
gsmet pushed a commit to gsmet/quarkus that referenced this issue Nov 5, 2024
benkard pushed a commit to benkard/quarkus-googlecloud-jsonlogging that referenced this issue Nov 13, 2024
…us-googlecloud-jsonlogging!24)

This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [io.quarkus:quarkus-extension-processor](https://github.com/quarkusio/quarkus) |  | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-extension-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-bom](https://github.com/quarkusio/quarkus) | import | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.16.1` -> `3.16.3` |

---

### Release Notes

<details>
<summary>quarkusio/quarkus</summary>

### [`v3.16.3`](quarkusio/quarkus@3.16.2...3.16.3)

[Compare Source](quarkusio/quarkus@3.16.2...3.16.3)

### [`v3.16.2`](https://github.com/quarkusio/quarkus/releases/tag/3.16.2)

[Compare Source](quarkusio/quarkus@3.16.1...3.16.2)

##### Complete changelog

-   [#&#8203;34824](quarkusio/quarkus#34824) - AmazonLambdaRecorder Handler Discovery Erroneously Considers Decorators
-   [#&#8203;38086](quarkusio/quarkus#38086) - Documentation about `RecordCodecProvider` in MongoDB with Panache
-   [#&#8203;42149](quarkusio/quarkus#42149) - Upgrade Postgres 16
-   [#&#8203;44039](quarkusio/quarkus#44039) - WebSockets Next: create a new event loop context for each client
-   [#&#8203;44132](quarkusio/quarkus#44132) - Update getting-started-reactive.adoc
-   [#&#8203;44149](quarkusio/quarkus#44149) - Fix Config Error Screen
-   [#&#8203;44152](quarkusio/quarkus#44152) - Do not throw NPE in AfterAll interceptor if application didn't start
-   [#&#8203;44155](quarkusio/quarkus#44155) - Amazon Lambda - Support decorators
-   [#&#8203;44156](quarkusio/quarkus#44156) - Make OidcRequestContextProperties modifiable
-   [#&#8203;44178](quarkusio/quarkus#44178) - Bump com.fasterxml.jackson:jackson-bom from 2.18.0 to 2.18.1
-   [#&#8203;44183](quarkusio/quarkus#44183) - Properly apply the update recipes in version order
-   [#&#8203;44184](quarkusio/quarkus#44184) - Add Support for Trusted Proxy Detection on Forwarded Requests
-   [#&#8203;44185](quarkusio/quarkus#44185) - Repeating `@PermissionsAllowed` annotations totally disable method authentication
-   [#&#8203;44189](quarkusio/quarkus#44189) - Small improvements to the Deploying to Google Cloud guide
-   [#&#8203;44190](quarkusio/quarkus#44190) - ResteasyReactiveProcessor#setupEndpoints reports duplicate endpoints when a rest client matches a resource
-   [#&#8203;44191](quarkusio/quarkus#44191) - Update PostgreSQL image to 17
-   [#&#8203;44201](quarkusio/quarkus#44201) - Broken link
-   [#&#8203;44202](quarkusio/quarkus#44202) - Broken link?
-   [#&#8203;44203](quarkusio/quarkus#44203) - Make OidcRequestContextProperties modifiable
-   [#&#8203;44204](quarkusio/quarkus#44204) - Fix broken doc links
-   [#&#8203;44207](quarkusio/quarkus#44207) - Bump bouncycastle.version from 1.78.1 to 1.79
-   [#&#8203;44209](quarkusio/quarkus#44209) - Bump io.quarkus.develocity:quarkus-project-develocity-extension from 1.1.6 to 1.1.7
-   [#&#8203;44221](quarkusio/quarkus#44221) - Add extension description for websockets next
-   [#&#8203;44227](quarkusio/quarkus#44227) - Add stork-configuration-generator as an annotationProcessorPath
-   [#&#8203;44229](quarkusio/quarkus#44229) - ContainerResponseFilter with `@Priority(Integer.MIN_VALUE)` will be actually invoked with max priority
-   [#&#8203;44232](quarkusio/quarkus#44232) - Quartz: use a more reasonable default for quarkus.quartz.thread-count
-   [#&#8203;44235](quarkusio/quarkus#44235) - 3.16: `@WithTestResource` starts all test resources (regression)
-   [#&#8203;44237](quarkusio/quarkus#44237) - Properly implement priority of ContainerResponseFilter
-   [#&#8203;44238](quarkusio/quarkus#44238) - Refactor SecurityTransformerUtils to consider repeated annotations
-   [#&#8203;44239](quarkusio/quarkus#44239) - Replace oidc auth facebook screenshots with generic ones
-   [#&#8203;44244](quarkusio/quarkus#44244) - Bump `quarkiverse-parent` to 18
-   [#&#8203;44245](quarkusio/quarkus#44245) - Delete disabled job
-   [#&#8203;44248](quarkusio/quarkus#44248) - Ignore client interfaces when detecting duplicate endpoints
-   [#&#8203;44263](quarkusio/quarkus#44263) - Quarkus Dev UI - Clicking on gRPC - Services - service implementation class  Uncaught exception received by Vert.x
-   [#&#8203;44277](quarkusio/quarkus#44277) - Dev UI Open in IDE make sure lineNumber is in quotes
-   [#&#8203;44279](quarkusio/quarkus#44279) - Limit `MATCHING_RESOURCES` TestResources to the test that declares them
-   [#&#8203;44281](quarkusio/quarkus#44281) - Included pages within a fragment ignores rendered=false property.
-   [#&#8203;44298](quarkusio/quarkus#44298) - Qute: fix rendered=false if a fragment includes nested fragment
-   [#&#8203;44300](quarkusio/quarkus#44300) - When testing request payload is populated with string "null" if enable-reflection-free-serializers enabled
-   [#&#8203;44309](quarkusio/quarkus#44309) - Avoid deserializing null nodes in reflection free Jackson serialization
-   [#&#8203;44316](quarkusio/quarkus#44316) - Duplicated field serialization using the generated reflection free Jackson serializers
-   [#&#8203;44317](quarkusio/quarkus#44317) - Avoid duplicated field serialization in reflection free Jackson serializers
-   [#&#8203;44321](quarkusio/quarkus#44321) - Use Java 21 by default in the Deploying to Google Cloud guide
-   [#&#8203;44322](quarkusio/quarkus#44322) - Explain in MongoDB docs that records are supported
-   [#&#8203;44324](quarkusio/quarkus#44324) - Take config annotation when trying to match test resources

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
bschuhmann pushed a commit to bschuhmann/quarkus that referenced this issue Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants