From b10b661475398a494fb4c4bd8e67bb2fd1a547c5 Mon Sep 17 00:00:00 2001
From: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
Date: Tue, 30 Jan 2024 15:56:05 -0500
Subject: [PATCH] Fixes problems converting a ClientUri to a URI. Improved
 test.

Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
---
 .../common/uri/UriQueryWriteableImpl.java     |  4 ++--
 .../jersey/connector/ConnectorBase.java       |  6 ++---
 .../io/helidon/webclient/api/ClientUri.java   | 22 ++++++++++++-------
 3 files changed, 19 insertions(+), 13 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
index 0dc4b5e6857..4db501a77ab 100644
--- 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/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 0ab11b7896e..b1179dbbe76 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
@@ -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/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java b/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java
index 45149e92469..ba5191da302 100644
--- 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.
@@ -103,7 +103,8 @@ public String toString() {
     }
 
     /**
-     * Convert instance to {@link java.net.URI}.
+     * Convert instance to {@link java.net.URI}. Note that we must use raw here
+     * to make sure no illegal URL characters are passed to the create method.
      *
      * @return the converted URI
      */
@@ -111,7 +112,7 @@ public URI toUri() {
         UriInfo info = uriBuilder.build();
         return URI.create(info.scheme() + "://"
                 + info.authority()
-                + pathWithQueryAndFragment());
+                + pathWithQueryAndFragment(false));     // URI needs encoded
     }
 
     /**
@@ -293,15 +294,20 @@ public UriQueryWriteable writeableQuery() {
     }
 
     /**
-     * Encoded path with query and fragment.
+     * Path with query and fragment. Result is encoded or decoded depending on the
+     * value of {@link #skipUriEncoding}.
      *
-     * @return string containing encoded path with query
+     * @return string containing path with query
      */
     public String pathWithQueryAndFragment() {
+        return pathWithQueryAndFragment(skipUriEncoding);
+    }
+
+    private String pathWithQueryAndFragment(boolean decoded) {
         UriInfo info = uriBuilder.query(query).build();
 
-        String queryString = skipUriEncoding ? info.query().value() : info.query().rawValue();
-        String path = skipUriEncoding ? info.path().path() : info.path().rawPath();
+        String queryString = decoded ? info.query().value() : info.query().rawValue();
+        String path = decoded ? info.path().path() : info.path().rawPath();
 
         boolean hasQuery = !queryString.isEmpty();
 
@@ -314,7 +320,7 @@ public String pathWithQueryAndFragment() {
         }
 
         if (info.fragment().hasValue()) {
-            String fragmentValue = skipUriEncoding ? info.fragment().value() : info.fragment().rawValue();
+            String fragmentValue = decoded ? info.fragment().value() : info.fragment().rawValue();
             path = path + '#' + fragmentValue;
         }