From ced8b0a739651f312744e721ca9f11c49f1834f0 Mon Sep 17 00:00:00 2001 From: Jose Date: Wed, 17 May 2023 15:11:25 +0200 Subject: [PATCH] Fix location and content location headers in Resteasy Reactive When providing a location, the URI was being decoded, so the value was being altered from what users set. Note that these changes are based on what Resteasy already does: https://github.com/resteasy/resteasy/blob/dadddfb699a875c44ba05c0abe176873acbd9aa2/resteasy-core/src/main/java/org/jboss/resteasy/specimpl/ResponseBuilderImpl.java#L187 Fix https://github.com/quarkusio/quarkus/issues/33419 --- .../server/jaxrs/ResponseBuilderImpl.java | 19 ++++++++++++------- .../server/jaxrs/RestResponseBuilderImpl.java | 19 ++++++++++++------- .../vertx/test/response/ResponseTest.java | 17 +++++++++++++++++ .../test/response/RestResponseResource.java | 19 +++++++++++++++++++ .../vertx/test/response/RestResponseTest.java | 8 ++++++++ 5 files changed, 68 insertions(+), 14 deletions(-) diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/ResponseBuilderImpl.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/ResponseBuilderImpl.java index c4c2847152c43..208c1093d4fd8 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/ResponseBuilderImpl.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/ResponseBuilderImpl.java @@ -40,10 +40,12 @@ public Response.ResponseBuilder location(URI location) { prefix = deployment.getPrefix(); } // Spec says relative to request, but TCK tests relative to Base URI, so we do that - location = new URI(req.getRequestScheme(), null, host, port, - prefix + - (location.getPath().startsWith("/") ? location.getPath() : "/" + location.getPath()), - location.getQuery(), null); + String path = location.toString(); + if (!path.startsWith("/")) { + path = "/" + path; + } + URI baseUri = new URI(req.getRequestScheme(), null, host, port, null, null, null); + location = baseUri.resolve(prefix + path); } catch (URISyntaxException e) { throw new RuntimeException(e); } @@ -72,9 +74,12 @@ public Response.ResponseBuilder contentLocation(URI location) { port = Integer.parseInt(host.substring(index + 1)); host = host.substring(0, index); } - location = new URI(req.getRequestScheme(), null, host, port, - location.getPath().startsWith("/") ? location.getPath() : "/" + location.getPath(), - location.getQuery(), null); + String path = location.toString(); + if (!path.startsWith("/")) { + path = "/" + path; + } + location = new URI(req.getRequestScheme(), null, host, port, null, null, null) + .resolve(path); } catch (URISyntaxException e) { throw new RuntimeException(e); } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/RestResponseBuilderImpl.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/RestResponseBuilderImpl.java index 21d74d4b9f828..a62003cbef6a5 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/RestResponseBuilderImpl.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/RestResponseBuilderImpl.java @@ -40,10 +40,12 @@ public RestResponse.ResponseBuilder location(URI location) { prefix = deployment.getPrefix(); } // Spec says relative to request, but TCK tests relative to Base URI, so we do that - location = new URI(req.getRequestScheme(), null, host, port, - prefix + - (location.getPath().startsWith("/") ? location.getPath() : "/" + location.getPath()), - location.getQuery(), null); + String path = location.toString(); + if (!path.startsWith("/")) { + path = "/" + path; + } + URI baseUri = new URI(req.getRequestScheme(), null, host, port, null, null, null); + location = baseUri.resolve(prefix + path); } catch (URISyntaxException e) { throw new RuntimeException(e); } @@ -72,9 +74,12 @@ public RestResponse.ResponseBuilder contentLocation(URI location) { port = Integer.parseInt(host.substring(index + 1)); host = host.substring(0, index); } - location = new URI(req.getRequestScheme(), null, host, port, - location.getPath().startsWith("/") ? location.getPath() : "/" + location.getPath(), - location.getQuery(), null); + String path = location.toString(); + if (!path.startsWith("/")) { + path = "/" + path; + } + location = new URI(req.getRequestScheme(), null, host, port, null, null, null) + .resolve(path); } catch (URISyntaxException e) { throw new RuntimeException(e); } diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/ResponseTest.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/ResponseTest.java index 40af3653860a6..849646f9eb373 100644 --- a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/ResponseTest.java +++ b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/ResponseTest.java @@ -2,6 +2,7 @@ import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.UriBuilder; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -16,4 +17,20 @@ public void testCaseInsensitivity() { Assertions.assertEquals("HEAD", response.getHeaders().getFirst("allow")); Assertions.assertEquals("HEAD", response.getHeaders().getFirst(HttpHeaders.ALLOW)); } + + @Test + public void testLocation() { + final var location = UriBuilder.fromUri("http://localhost:8080").path("{language}") + .build("en/us"); + Response response = Response.ok("Hello").location(location).build(); + Assertions.assertEquals("http://localhost:8080/en%2Fus", response.getLocation().toString()); + } + + @Test + public void testContentLocation() { + final var location = UriBuilder.fromUri("http://localhost:8080").path("{language}") + .build("en/us"); + Response response = Response.ok("Hello").contentLocation(location).build(); + Assertions.assertEquals("http://localhost:8080/en%2Fus", response.getHeaderString("Content-Location")); + } } diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/RestResponseResource.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/RestResponseResource.java index 1c2f8f1e5a8b0..d72fdf301ac5e 100644 --- a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/RestResponseResource.java +++ b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/RestResponseResource.java @@ -13,6 +13,7 @@ import jakarta.ws.rs.core.CacheControl; import jakarta.ws.rs.core.NewCookie; import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.UriBuilder; import jakarta.ws.rs.core.Variant; import org.jboss.resteasy.reactive.RestResponse; @@ -47,6 +48,24 @@ public RestResponse wildcard() { return RestResponse.ResponseBuilder.ok("Hello").header("content-type", "text/plain").build(); } + @GET + @Path("rest-response-location") + public RestResponse location() { + final var location = UriBuilder.fromResource(RestResponseResource.class).path("{language}") + .queryParam("user", "John") + .build("en/us"); + return RestResponse.ResponseBuilder.ok("Hello").location(location).build(); + } + + @GET + @Path("rest-response-content-location") + public RestResponse contentLocation() { + final var location = UriBuilder.fromResource(RestResponseResource.class).path("{language}") + .queryParam("user", "John") + .build("en/us"); + return RestResponse.ResponseBuilder.ok("Hello").contentLocation(location).build(); + } + @GET @Path("rest-response-full") @SuppressWarnings("deprecation") diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/RestResponseTest.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/RestResponseTest.java index 03d83f208b2c9..2563091c4f076 100644 --- a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/RestResponseTest.java +++ b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/response/RestResponseTest.java @@ -1,5 +1,7 @@ package org.jboss.resteasy.reactive.server.vertx.test.response; +import static org.hamcrest.CoreMatchers.endsWith; + import java.util.function.Supplier; import org.hamcrest.Matchers; @@ -107,5 +109,11 @@ public void test() { .then().statusCode(200) .and().body(Matchers.equalTo("Uni request filter")) .and().contentType("text/plain"); + RestAssured.get("/rest-response-location") + .then().statusCode(200) + .header("Location", endsWith("/en%2Fus?user=John")); + RestAssured.get("/rest-response-content-location") + .then().statusCode(200) + .header("Content-Location", endsWith("/en%2Fus?user=John")); } }