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

RestEasyReactive Client no longer maps List items in query param as multivalued query field in JDK21 #36170

Closed
Alex-Wauters opened this issue Sep 26, 2023 · 12 comments · Fixed by #36197
Assignees
Labels
area/rest kind/bug Something isn't working
Milestone

Comments

@Alex-Wauters
Copy link

Alex-Wauters commented Sep 26, 2023

Describe the bug

With quarkus-resteasy-reactive, the behavior of mapping a List<> to a query param appears to have changed when running on OpenJDK 21.

Example resteasy reactive interface client:

@GET
@Path("/exampleAPI")
@Produces(APPLICATION_JSON)
Uni<List<ResponseDTO>> doRequest(
    @RestQuery("status") List<EnumField> statuses,
) throws BaseException;

Expected behavior

The above code snippet maps in OpenJdk17 to

http://service:8080/exampleAPI?status=PUBLISHED&status=RELEASED

Actual behavior

After updating to JDK21 (Amazoncoretto), it maps to

http://service:8080/exampleAPI?status=%5BPUBLISHED%2C+RELEASED%5D

or URL-decoded:

http://service:8080/exampleAPI?status=[PUBLISHED,+RELEASED]

How to Reproduce?

Create a RESTEASY Reactive Rest client method that accepts a List of enum fields.

Output of uname -a or ver

No response

Output of java -version

openjdk version "21" 2023-09-19 OpenJDK Runtime Environment (build 21+35-2513) OpenJDK 64-Bit Server VM (build 21+35-2513, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.3.3

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

maven 3.9.4

Additional information

No response

@Alex-Wauters Alex-Wauters added the kind/bug Something isn't working label Sep 26, 2023
@Alex-Wauters Alex-Wauters changed the title RestEasyReactive maps List items in query param as field=[ITEM1+,ITEM2+,] instead of field=ITEM1,field=ITEM2 on JDK21 RestEasyReactive maps List items in query param in JDK21 as string output of a list instead of multivalued query field Sep 26, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 27, 2023

/cc @FroMage (resteasy-reactive), @stuartwdouglas (resteasy-reactive)

@geoand
Copy link
Contributor

geoand commented Sep 27, 2023

Weird, I'll have to try it out.

@geoand
Copy link
Contributor

geoand commented Sep 27, 2023

I cannot reproduce the problem with OpenJDK 21

@geoand geoand added the triage/needs-feedback We are waiting for feedback. label Sep 28, 2023
@FroMage
Copy link
Member

FroMage commented Sep 28, 2023

Just in case you missed it (I did at first), this is about the client, not the server.

@geoand
Copy link
Contributor

geoand commented Sep 28, 2023

I did indeed :)

@FroMage
Copy link
Member

FroMage commented Sep 28, 2023

:)

@Alex-Wauters
Copy link
Author

Adding an example repo in just a few moments

@Alex-Wauters
Copy link
Author

https://github.com/Alex-Wauters/quarkus-resteasy-listparam-bug

Run and launch a request to http://localhost:8080/hello - it makes a request to itself and logs the request to console

2023-09-28 12:05:35,136 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, rest-client-reactive, rest-client-reactive-jackson, resteasy-reactive, smallrye-context-propagation, vertx]
2023-09-28 12:05:35,137 INFO  [io.qua.dep.dev.RuntimeUpdatesProcessor] (vert.x-worker-thread-1) Live reload total time: 0.490s 
2023-09-28 12:05:35,192 DEBUG [org.jbo.res.rea.cli.log.DefaultClientLogger] (vert.x-eventloop-thread-1) Request: GET http://localhost:8080/exampleAPI?status=%5BIN_PROGRESS%2C+PUBLISHED%5D Headers[Accept=text/plain;charset=UTF-8 User-Agent=Resteasy Reactive Client], Empty body
2023-09-28 12:05:35,197 DEBUG [org.jbo.res.rea.cli.log.DefaultClientLogger] (vert.x-eventloop-thread-1) Response: GET http://localhost:8080/exampleAPI?status=%5BIN_PROGRESS%2C+PUBLISHED%5D, Status[200 OK], Headers[content-length=47 Content-Type=text/plain;charset=UTF-8], Body:
Received request - see console logs for params 

Where exampleAPI?status=%5BIN_PROGRESS%2C+PUBLISHED%5D
differs from exampleAPI?status=IN_PROGRESS&status=PUBLISHED

JDK:
openjdk 21 2023-09-19
OpenJDK Runtime Environment (build 21+35-2513)
OpenJDK 64-Bit Server VM (build 21+35-2513, mixed mode, sharing)

@Alex-Wauters
Copy link
Author

Here's the output of the same code running on openJDK17:

2023-09-28 12:17:41,933 DEBUG [org.jbo.res.rea.cli.log.DefaultClientLogger] (vert.x-eventloop-thread-1) Request: GET http://localhost:8080/exampleAPI?status=IN_PROGRESS&status=PUBLISHED Headers[Accept=text/plain;charset=UTF-8 User-Agent=Resteasy Reactive Client], Empty body
2023-09-28 12:17:41,939 DEBUG [org.jbo.res.rea.cli.log.DefaultClientLogger] (vert.x-eventloop-thread-1) Response: GET http://localhost:8080/exampleAPI?status=IN_PROGRESS&status=PUBLISHED, Status[200 OK], Headers[content-length=47 Content-Type=text/plain;charset=UTF-8], Body:
Received request - see console logs for params 

@geoand
Copy link
Contributor

geoand commented Sep 28, 2023

Thanks

@geoand geoand removed the triage/needs-feedback We are waiting for feedback. label Sep 28, 2023
@Alex-Wauters Alex-Wauters changed the title RestEasyReactive maps List items in query param in JDK21 as string output of a list instead of multivalued query field RestEasyReactive Client no longer maps List items in query param as multivalued query field in JDK21 Sep 28, 2023
@geoand
Copy link
Contributor

geoand commented Sep 28, 2023

This is an interesting one, I'll soon have a fix for.

@geoand geoand self-assigned this Sep 28, 2023
geoand added a commit to geoand/quarkus that referenced this issue Sep 28, 2023
The check that was being done is broken
in Java 21 as that version bring SequencedCollection
which List now implements

Closes: quarkusio#36170
@geoand
Copy link
Contributor

geoand commented Sep 28, 2023

#36197 is the fix

geoand added a commit to geoand/quarkus that referenced this issue Sep 28, 2023
The check that was being done is broken
in Java 21 as that version bring SequencedCollection
which List now implements

Closes: quarkusio#36170
geoand added a commit that referenced this issue Sep 28, 2023
Fix broken collection assignability check
@quarkus-bot quarkus-bot bot added this to the 3.5 - main milestone Sep 28, 2023
@gsmet gsmet modified the milestones: 3.5 - main, 3.4.2 Oct 3, 2023
gsmet pushed a commit to gsmet/quarkus that referenced this issue Oct 3, 2023
The check that was being done is broken
in Java 21 as that version bring SequencedCollection
which List now implements

Closes: quarkusio#36170
(cherry picked from commit ffa805a)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Oct 3, 2023
The check that was being done is broken
in Java 21 as that version bring SequencedCollection
which List now implements

Closes: quarkusio#36170
(cherry picked from commit ffa805a)
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Feb 8, 2024
The check that was being done is broken
in Java 21 as that version bring SequencedCollection
which List now implements

Closes: quarkusio#36170
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.

4 participants