diff --git a/webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java b/webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java index bdd3f053203..f13ee6e47e2 100644 --- a/webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java +++ b/webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java @@ -37,6 +37,7 @@ import io.helidon.common.tls.Tls; import io.helidon.common.uri.UriEncoding; import io.helidon.common.uri.UriFragment; +import io.helidon.common.uri.UriQuery; import io.helidon.http.ClientRequestHeaders; import io.helidon.http.Header; import io.helidon.http.HeaderNames; @@ -437,10 +438,19 @@ protected MediaContext mediaContext() { protected ClientUri resolveUri(ClientUri toResolve) { if (uriTemplate != null) { String resolved = resolvePathParams(uriTemplate); + URI uri; if (skipUriEncoding) { - toResolve.resolve(URI.create(resolved)); + uri = URI.create(resolved); } else { - toResolve.resolve(URI.create(UriEncoding.encodeUri(resolved))); + uri = URI.create(UriEncoding.encodeUri(resolved)); + } + toResolve.resolve(uri); + + if (uri.isAbsolute()) { // query parameters are cleared when resolving against absolute URIs + UriQuery query = clientUri.query(); + if (!query.isEmpty()) { + toResolve.writeableQuery().from(query); + } } } return toResolve; diff --git a/webclient/api/src/test/java/io/helidon/webclient/api/ClientRequestBaseTest.java b/webclient/api/src/test/java/io/helidon/webclient/api/ClientRequestBaseTest.java new file mode 100644 index 00000000000..4910d85eeeb --- /dev/null +++ b/webclient/api/src/test/java/io/helidon/webclient/api/ClientRequestBaseTest.java @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2024 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.webclient.api; + +import io.helidon.common.uri.UriPath; +import io.helidon.http.Method; +import org.junit.jupiter.api.Test; + +import java.util.Collections; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +class ClientRequestBaseTest { + + /** + * Verify that query parameters are preserved when resolving URI templates (cf. issue #8566). + * Make sure to test both absolute and relative URIs as they are handled differently when resolving. + */ + @Test + public void resolvedUriTest() { + ClientUri uri = new FakeClientRequest() + .uri("https://www.example.com/") + .queryParam("k", "v").resolvedUri(); + assertThat(uri.authority(), is("www.example.com:443")); + assertThat(uri.host(), is("www.example.com")); + assertThat(uri.path(), is(UriPath.root())); + assertThat(uri.port(), is(443)); + assertThat(uri.scheme(), is("https")); + assertThat(uri.query().get("k"), is("v")); + + uri = new FakeClientRequest() + .uri("https://www.example.com/{path}") + .pathParam("path", "p") + .queryParam("k", "v").resolvedUri(); + assertThat(uri.authority(), is("www.example.com:443")); + assertThat(uri.host(), is("www.example.com")); + assertThat(uri.path(), is(UriPath.create("/p"))); + assertThat(uri.port(), is(443)); + assertThat(uri.scheme(), is("https")); + assertThat(uri.query().get("k"), is("v")); + + uri = new FakeClientRequest() + .uri("example/{path}") + .pathParam("path", "p") + .queryParam("k", "v").resolvedUri(); + assertThat(uri.authority(), is("localhost:80")); + assertThat(uri.host(), is("localhost")); + assertThat(uri.path(), is(UriPath.create("/example/p"))); + assertThat(uri.port(), is(80)); + assertThat(uri.scheme(), is("http")); + assertThat(uri.query().get("k"), is("v")); + + uri = new FakeClientRequest() + .uri("https://www.example.com/p?k={k}") + .pathParam("k", "v") + .queryParam("k", "v").resolvedUri(); + assertThat(uri.authority(), is("www.example.com:443")); + assertThat(uri.host(), is("www.example.com")); + assertThat(uri.path(), is(UriPath.create("/p%3Fk=v"))); + assertThat(uri.port(), is(443)); + assertThat(uri.scheme(), is("https")); + assertThat(uri.query().get("k"), is("v")); + } + + + private static final class FakeClientRequest extends ClientRequestBase { + private FakeClientRequest() { + super(WebClientConfig.create(), null, "fake", Method.GET, ClientUri.create(), Collections.emptyMap()); + } + + @Override + protected HttpClientResponse doSubmit(Object entity) { + return null; + } + + @Override + protected HttpClientResponse doOutputStream(OutputStreamHandler outputStreamHandler) { + return null; + } + } +} diff --git a/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java b/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java index a670bc45586..b7471e286fd 100644 --- a/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java +++ b/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java @@ -131,4 +131,30 @@ void testResolveQuery() { assertThat(clientUri.query().get("filter"), is("a b c")); assertThat(clientUri.query().getRaw("filter"), is("a%20b%20c")); } -} \ No newline at end of file + + /** + * Verifies that {@link ClientUri#resolve(ClientUri)} retains query parameters but {@link ClientUri#resolve(URI)} + * doesn't (which is the same behaviour as {@link URI#resolve(URI)}). + */ + @Test + void testResolveQueryParameter() { + ClientUri helper = ClientUri.create(URI.create("http://localhost:8080/?k=v")); + URI uri = URI.create("https://www.example.com:80"); + helper.resolve(uri); + assertThat(helper.authority(), is("www.example.com:80")); + assertThat(helper.host(), is("www.example.com")); + assertThat(helper.path(), is(UriPath.root())); + assertThat(helper.port(), is(80)); + assertThat(helper.scheme(), is("https")); + assertThat("unexpected query parameter: 'k'", !helper.query().contains("k")); + + helper = ClientUri.create(URI.create("http://localhost:8080/?k=v")); + helper.resolve(ClientUri.create(uri)); + assertThat(helper.authority(), is("www.example.com:80")); + assertThat(helper.host(), is("www.example.com")); + assertThat(helper.path(), is(UriPath.root())); + assertThat(helper.port(), is(80)); + assertThat(helper.scheme(), is("https")); + assertThat(helper.query().get("k"), is("v")); + } +}