Fixes a few problems handling character encodings in URIs (helidon-io…

…#8327) * Fixes problems converting a ClientUri to a URI. Improved test. Signed-off-by: Santiago Pericasgeertsen <[email protected]> * Drops skip encoding setting in connector after fixing handling of + in ClientUri query string. Signed-off-by: Santiago Pericasgeertsen <[email protected]> * Trying using forkCount set to 1. Signed-off-by: Santiago Pericasgeertsen <[email protected]> --------- Signed-off-by: Santiago Pericasgeertsen <[email protected]>
barchetta · Feb 5, 2024 · 1cf9272 · 1cf9272
1 parent 4a5623c
commit 1cf9272
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 35 deletions.
diff --git a/common/uri/src/main/java/io/helidon/common/uri/UriQueryWriteableImpl.java b/common/uri/src/main/java/io/helidon/common/uri/UriQueryWriteableImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 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.
@@ -79,7 +79,7 @@ public UriQueryWriteable from(UriQuery uriQuery) {
                         .addAll(raw);
                 List<String> decoded = uriQuery.all(name);
                 decodedQueryParams.computeIfAbsent(name, it -> new ArrayList<>())
-                        .addAll(raw);
+                        .addAll(decoded);
             }
         }
 

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
@@ -151,7 +151,6 @@ private HttpClientRequest mapRequest(ClientRequest request) {
         HttpClientRequest httpRequest = webClient
                 .method(Method.create(request.getMethod()))
                 .proxy(requestProxy)
-                .skipUriEncoding(true)      // already encoded by Jersey
                 .uri(uri);
 
         // map request headers

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
@@ -115,11 +115,11 @@ public void testBasicPost() {
     @Test
     public void queryGetTest() {
         try (Response response = target("basic").path("getquery")
-                .queryParam("first", "hello there ")
-                .queryParam("second", "world")
+                .queryParam("first", "\"hello there ")
+                .queryParam("second", "world\"")
                 .request().get()) {
             assertThat(response.getStatus(), is(200));
-            assertThat(response.readEntity(String.class), is("hello there world"));
+            assertThat(response.readEntity(String.class), is("\"hello there world\""));
         }
     }
 

diff --git a/microprofile/telemetry/pom.xml b/microprofile/telemetry/pom.xml
@@ -16,8 +16,8 @@
   -->
 
 <project xmlns="http://maven.apache.org/POM/4.0.0"
-        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-        xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
     <parent>
         <artifactId>helidon-microprofile-project</artifactId>
@@ -137,28 +137,10 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-surefire-plugin</artifactId>
-                <executions>
-                    <execution>
-                        <id>default-test</id>
-                        <configuration>
-                            <excludes>
-                                <exclude>**/WithSpanWithExplicitAppTest.java</exclude>
-                            </excludes>
-                        </configuration>
-                    </execution>
-                    <execution>
-                        <!-- Run in a separate execution so other tests don't have the Application bean in play. -->
-                        <id>test-with-explicit-app</id>
-                        <goals>
-                            <goal>test</goal>
-                        </goals>
-                        <configuration>
-                            <includes>
-                                <include>**/WithSpanWithExplicitAppTest.java</include>
-                            </includes>
-                        </configuration>
-                    </execution>
-                </executions>
+                <configuration>
+                    <forkCount>1</forkCount>
+                    <reuseForks>false</reuseForks>
+                </configuration>
             </plugin>
         </plugins>
     </build>

diff --git a/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java b/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 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.
@@ -18,6 +18,7 @@
 
 import java.net.URI;
 
+import io.helidon.common.uri.UriEncoding;
 import io.helidon.common.uri.UriFragment;
 import io.helidon.common.uri.UriInfo;
 import io.helidon.common.uri.UriPath;
@@ -193,8 +194,11 @@ public ClientUri resolve(URI uri) {
 
         uriBuilder.path(resolvePath(uriBuilder.path().path(), uri.getPath()));
 
-        if (uri.getQuery() != null) {
-            query.fromQueryString(uri.getQuery());
+        String queryString = uri.getQuery();
+        if (queryString != null) {
+            // class URI does not decode +'s, so we do it here
+            query.fromQueryString(queryString.indexOf('+') >= 0 ? UriEncoding.decodeUri(queryString)
+                    : queryString);
         }
 
         if (uri.getRawFragment() != 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
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 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.
@@ -20,7 +20,6 @@
 
 import io.helidon.common.uri.UriPath;
 import io.helidon.common.uri.UriQueryWriteable;
-
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -105,4 +104,17 @@ void testResolveAll() {
         assertThat(helper.port(), is(80));
         assertThat(helper.scheme(), is("https"));
     }
+
+    /**
+     * Verifies that "+" is interpreted as a space character the query strings.
+     * Note that the {@link URI} class does not appear to handle this correctly.
+     */
+    @Test
+    void testResolveQuery() {
+        URI uri = URI.create("http://localhost:8080/greet?filter=a+b+c");
+        ClientUri clientUri = ClientUri.create();
+        clientUri.resolve(uri);
+        assertThat(clientUri.query().get("filter"), is("a b c"));
+        assertThat(clientUri.query().getRaw("filter"), is("a%20b%20c"));
+    }
 }