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

@PathParam reactive client encoding vs. classic #26572

Open
runzhammer opened this issue Jul 6, 2022 · 14 comments
Open

@PathParam reactive client encoding vs. classic #26572

runzhammer opened this issue Jul 6, 2022 · 14 comments

Comments

@runzhammer
Copy link

runzhammer commented Jul 6, 2022

Please reopen and make this behavior configurable.
Now, this fails setting variable paths to GET or POST to.

Take this example, where path segments are variable in a docker image name.
I have a service which fetches manifests from the image registry which fails now.

	/**
	 * @param repoName
	 * @param imageTag
	 * @return docker manifest of image
	 */
	@GET
	@Path("{repoName}/manifests/{imageTag}")
	public Manifest getManifest(@PathParam(value = "repoName") String repoName, @PathParam(value = "imageTag") String imageTag);

Now, that @PathParam gets urlencoded, all my wiremock tests fail :[

-----------------------------------------------------------------------------------------------------------------------
| Closest stub                                             | Request                                                  |
-----------------------------------------------------------------------------------------------------------------------
                                                           |
                                                           |
GET                                                        | GET
/v2/public/testing/base/manifests/latest                   | /v2/public%2Ftesting%2Fbase/manifests/latest       <<<<< URL does not match
                                                           |
                                                           |
-----------------------------------------------------------------------------------------------------------------------

In classic client, where no encoding was enabled by default, this worked perfectly.
I think not to encode by default should be the way to go, as the neccessity of encoding strings before using as @PathParam could be done without any hassle.

Originally posted by @runzhammer in #25418 (comment)

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 6, 2022

/cc @michalszynkiewicz

@sbert
Copy link

sbert commented Aug 4, 2022

This problem was introduced in quarkus 2.8.3
until 2.8.2 everything is ok

@michalszynkiewicz
Copy link
Member

I think encoding should be the default behavior as it is but having a switch to disable it definitely sounds reasonable.
Would you like to try to contribute such a change?

@geoand
Copy link
Contributor

geoand commented Mar 20, 2023

Is this still an issue?

@runzhammer
Copy link
Author

yes, it is ... but i workaround it by splitting up the path into segments ... and filling them into variables as needed.

@fabricepipart1a
Copy link

fabricepipart1a commented Apr 18, 2023

I just hit the same issue. Since PathParam can't be configured, I can't achieve what I wanted to do.
My use case is the following. I have a URL (Jenkins) that follows this pattern:

https://hostname/(/job/[A-Za-z0-9]+)*/api/json

Since /job/<keyword> can be repeated from 0 to N times in my URL, I can't really introduce a JAX RS Client method for each number of times the pattern is repeated.

When reading documentation, I should be able to do:
@Path(value = "{folder:.+}/api/json")
And then folder would match the /job/<keyword> repetition.

I does. But then it actually calls %2Fjob%2Fsomething/api/json

@snazy
Copy link
Contributor

snazy commented Dec 5, 2023

Just hit a similar issue, but on the server side, using Quarkus 3.6.0.
With io.quarkus:quarkus-resteasy, a @PathParameter gets properly decoded.
But with io.quarkus:quarkus-resteasy-reactive, a @PathParameter is not decoded.
The "reactive" behavior feels wrong, because the @jakarta.ws.rs.Encoded annotation is there to request the encoded value.

Example:
A request like http://127.0.0.1:19120/foo/name%40host/bar against an endpoint like

interface ExampleService {
  @GET
  @Path("/foo/{name:.*}/bar")
  String foo(@PathParam("name") String name);
}

For io.quarkus:quarkus-resteasy: name=="name@host"
For io.quarkus:quarkus-resteasy-reactive: name=="name%40host" <-- this feels wrong

@geoand
Copy link
Contributor

geoand commented Dec 5, 2023

@snazy any chance you can try main?

I merged a PR today that may also fix your case

@fabricepipart1a
Copy link

Should I try too? Or no chance in my case?

@geoand
Copy link
Contributor

geoand commented Dec 5, 2023

@fabricepipart1a the client issue hasn't been addressed

@snazy
Copy link
Contributor

snazy commented Dec 5, 2023

@geoand I can probably try tomorrow.

@geoand
Copy link
Contributor

geoand commented Dec 5, 2023

🙏

@snazy
Copy link
Contributor

snazy commented Dec 6, 2023

@geoand Can confirm that the behavior is still wrong in 3.6.1, but correct on quarkus/main on 903cd31

@geoand
Copy link
Contributor

geoand commented Dec 6, 2023

Thanks for checking!

#37513 is the change that fixed it. The plan is to get it in 3.6.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants