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

quarkus-rest-client and quarkus-rest-client-reactive remove trailing slash #41689

Closed
brunobastosg opened this issue Jul 4, 2024 · 10 comments
Closed
Labels
area/rest-client kind/bug Something isn't working

Comments

@brunobastosg
Copy link

brunobastosg commented Jul 4, 2024

Describe the bug

I was upgrading an app from Quarkus 2.9 to Quarkus 3.12 that needs to call a REST endpoint ending with a trailing slash.

In Quarkus 3.12, it doesn't work because the trailing slash is removed, so the API returns an error.

I also tested it in 3.2, 3.8 and 3.11, and the same error occurs.

I found this issue reporting the same problem in Quarkus 2.11, but it says it was fixed in 2.12. But now in Quarkus 3 it is not working again.

Expected behavior

The trailing slash should be kept by the rest client

Actual behavior

Rest client should removes the trailing slash

How to Reproduce?

Unfortunately I don't know any public APIs ending with a trailing slash. But the steps are as follows:

  1. Find an API that only works with a trailing slash
  2. Use quarkus-rest-client or quarkus-rest-client-reactive to call this API as follows:
Path("/endpoint/ending/with/trailing/slash/")
public interface MyService {
  // the actual methods to call
}
  1. The trailing slash will be removed and the API will return an error

Output of uname -a or ver

Linux serpro-1570142 5.19.0-45-generic #46~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Jun 7 15:06:04 UTC 20 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk 21.0.3 2024-04-16 LTS OpenJDK Runtime Environment Temurin-21.0.3+9 (build 21.0.3+9-LTS) OpenJDK 64-Bit Server VM Temurin-21.0.3+9 (build 21.0.3+9-LTS, mixed mode, sharing)

Quarkus version or git rev

3.12.1

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

Apache Maven 3.9.6 (bc0240f3c744dd6b6ec2920b3cd08dcc295161ae) Maven home: /home/s009414875/.sdkman/candidates/maven/current Java version: 21.0.3, vendor: Eclipse Adoptium, runtime: /home/s009414875/.sdkman/candidates/java/21.0.3-tem Default locale: pt_BR, platform encoding: UTF-8 OS name: "linux", version: "5.19.0-45-generic", arch: "amd64", family: "unix"

Additional information

No response

@brunobastosg brunobastosg added the kind/bug Something isn't working label Jul 4, 2024
Copy link

quarkus-bot bot commented Jul 4, 2024

/cc @cescoffier (rest-client), @geoand (rest-client)

@geoand
Copy link
Contributor

geoand commented Jul 4, 2024

Any chance you can put together a sample that shows this issue in action?

Thanks

@geoand geoand added the triage/needs-feedback We are waiting for feedback. label Jul 4, 2024
@brunobastosg
Copy link
Author

@geoand
Copy link
Contributor

geoand commented Jul 4, 2024

Thanks

@geoand geoand removed the triage/needs-feedback We are waiting for feedback. label Jul 4, 2024
@brunobastosg
Copy link
Author

I just discovered that the error only occurs when the @Path annotation is at the class level. Moving it to the method level resolves the issue.

@geoand
Copy link
Contributor

geoand commented Jul 5, 2024

Thanks for looking into it.

I am actually not going to fix this because it can cause regressions to existing code.
In your case, you can easily overcome the issue by changing:

to:

@Path("/endpoint-with-trailing-slash/")
@RegisterRestClient(configKey = "remote-service")
public interface RemoteService {

    @GET
    String callService();
}
@Path("/endpoint-with-trailing-slash")
@RegisterRestClient(configKey = "remote-service")
public interface RemoteService {

    @GET
    @Path("/")
    String callService();
}

@geoand geoand closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2024
@Sgitario
Copy link
Contributor

@geoand please reconsider your thoughts about this since there are tools that autogenerate the interfaces (openapi generators), so it can't easily change where to put the @Path annotation. Indeed, magically removing the slash is controversial since it's not respecting what the users configure.

Going to history, this was already reported in the past by #27050 and your fix worked for a while, though the class ClientEndpointIndexer is not using the method "handleTrailingSlash" but this.

@geoand
Copy link
Contributor

geoand commented Oct 18, 2024

But wouldn't it be better to fix the generator in that case?

@Sgitario
Copy link
Contributor

But wouldn't it be better to fix the generator in that case?

No, because the server rejects calls without the "/". Technically speaking, the generator works ok, but the rest-client magically drops the "/" without respecting the API contract.

@geoand
Copy link
Contributor

geoand commented Oct 18, 2024

If you want to add some new property that would opt-in to keeping the slash, that's fine with me, but I don't want to change the default behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest-client kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants