From a5a404dddf77ccf9516bde0463d9d18f5536af9c Mon Sep 17 00:00:00 2001 From: Santiago Pericasgeertsen Date: Fri, 12 Jan 2024 11:17:26 -0500 Subject: [PATCH] This commit addresses two problems: (1) it prevents the Helidon connector from re-encoding and already encoded URI and (2) extends our URI decoder logic to support '+' which is used by Jersey to encode a space character. See issue #8228. --- .../io/helidon/common/uri/UriEncoding.java | 9 +++-- .../helidon/common/uri/UriEncodingTest.java | 33 +++++++++++++++++++ .../jersey/connector/HelidonConnector.java | 13 +------- .../jersey/connector/ConnectorBase.java | 6 ++-- 4 files changed, 41 insertions(+), 20 deletions(-) create mode 100644 common/uri/src/test/java/io/helidon/common/uri/UriEncodingTest.java diff --git a/common/uri/src/main/java/io/helidon/common/uri/UriEncoding.java b/common/uri/src/main/java/io/helidon/common/uri/UriEncoding.java index 654faac2a7e..fa1d33250d3 100644 --- a/common/uri/src/main/java/io/helidon/common/uri/UriEncoding.java +++ b/common/uri/src/main/java/io/helidon/common/uri/UriEncoding.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2023 Oracle and/or its affiliates. + * Copyright (c) 2000, 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. @@ -48,10 +48,10 @@ private UriEncoding() { * @return decoded string */ public static String decodeUri(String uriSegment) { - if (uriSegment.length() == 0) { + if (uriSegment.isEmpty()) { return ""; } - if (uriSegment.indexOf('%') == -1) { + if (uriSegment.indexOf('%') == -1 && uriSegment.indexOf('+') == -1) { return uriSegment; } @@ -124,7 +124,6 @@ private static void appendEscape(StringBuilder appender, int b) { } private static String decode(String string) { - int len = string.length(); StringBuilder sb = new StringBuilder(len); @@ -143,7 +142,7 @@ private static String decode(String string) { betweenBrackets = false; } if (c != '%' || betweenBrackets) { - sb.append(c); + sb.append(c == '+' && !betweenBrackets ? ' ' : c); // handles '+' decoding if (++i >= len) { break; } diff --git a/common/uri/src/test/java/io/helidon/common/uri/UriEncodingTest.java b/common/uri/src/test/java/io/helidon/common/uri/UriEncodingTest.java new file mode 100644 index 00000000000..56d791e35d2 --- /dev/null +++ b/common/uri/src/test/java/io/helidon/common/uri/UriEncodingTest.java @@ -0,0 +1,33 @@ +/* + * 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.common.uri; + +import org.junit.jupiter.api.Test; + +import static io.helidon.common.uri.UriEncoding.decodeUri; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.CoreMatchers.is; + +class UriEncodingTest { + + @Test + void testSpaceDecoding() { + assertThat(decodeUri("%20hello%20world%20"), is(" hello world ")); + assertThat(decodeUri("[%20]hello[%20]world[%20]"), is("[%20]hello[%20]world[%20]")); + assertThat(decodeUri("+hello+world+"), is(" hello world ")); + assertThat(decodeUri("[+]hello[+]world[+]"), is("[+]hello[+]world[+]")); + } +} diff --git a/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java b/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java index 062367a642d..eee6937cf29 100644 --- a/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java +++ b/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java @@ -29,7 +29,6 @@ import io.helidon.common.LazyValue; import io.helidon.common.Version; import io.helidon.common.tls.Tls; -import io.helidon.common.uri.UriQueryWriteable; import io.helidon.config.Config; import io.helidon.http.Header; import io.helidon.http.HeaderNames; @@ -143,19 +142,9 @@ private HttpClientRequest mapRequest(ClientRequest request) { HttpClientRequest httpRequest = webClient .method(Method.create(request.getMethod())) .proxy(requestProxy) + .skipUriEncoding(true) // already encoded by Jersey .uri(uri); - // map query parameters - String queryString = uri.getQuery(); - if (queryString != null) { - UriQueryWriteable query = UriQueryWriteable.create(); - query.fromQueryString(queryString); - query.names().forEach(name -> { - String[] values = query.all(name).toArray(new String[0]); - httpRequest.queryParam(name, values); - }); - } - // map request headers request.getRequestHeaders().forEach((key, value) -> { String[] values = value.toArray(new String[0]); diff --git a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java index 388eda1ac1e..0ab11b7896e 100644 --- a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java +++ b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. + * Copyright (c) 2023, 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. @@ -115,11 +115,11 @@ public void testBasicPost() { @Test public void queryGetTest() { try (Response response = target("basic").path("getquery") - .queryParam("first", "hello") + .queryParam("first", "hello there ") .queryParam("second", "world") .request().get()) { assertThat(response.getStatus(), is(200)); - assertThat(response.readEntity(String.class), is("helloworld")); + assertThat(response.readEntity(String.class), is("hello there world")); } }