#7127 | Uri: inconsistent query param names (url-encode) when .renderString-ing

core/shared/src/main/scala/org/http4s/Query.scala

@@ -165,6 +165,29 @@ final class Query private (value: Either[Vector[KeyValue], String])
       CollectionCompat.mapValues(m.toMap)(_.toList)
     }
 
+  /** Creates a new encoded `Self` with all the specified parameters in the [[Query]].
+    * If the list of parameters is empty, it will return the original `Self`.
+    */
+  def encode: Self =
+    if (query.toVector.isEmpty) {
+      query
+    } else {
+      val result = query.toVector.map {
+        case (k, None) => k -> None
+        case (k, Some(v)) =>
+          k -> Some(
+            UriCoding.encode(
+              v,
+              StandardCharsets.UTF_8,
+              spaceIsPlus = false,
+              toSkip = UriCoding.QueryNoEncode,
+            )
+          )
+      }
+
+      Query.fromVector(result)
+    }
+
   override def equals(that: Any): Boolean =
     that match {
       case that: Query => that.toVector == toVector

core/shared/src/main/scala/org/http4s/Uri.scala

@@ -210,7 +210,9 @@ object Uri extends UriPlatform {
 
   /** Decodes the String to a [[Uri]] using the RFC 3986 uri decoding specification */
   def fromString(s: String): ParseResult[Uri] =
-    ParseResult.fromParser(Parser.uriReferenceUtf8, "Invalid URI")(s)
+    ParseResult
+      .fromParser(Parser.uriReferenceUtf8, "Invalid URI")(s)
+      .map(uri => uri.copy(query = uri.query.encode))
 
   /** Parses a String to a [[Uri]] according to RFC 3986.  If decoding
     *  fails, throws a [[ParseFailure]].

tests/shared/src/test/scala/org/http4s/UriSuite.scala

@@ -209,11 +209,11 @@ class UriSuite extends Http4sSuite {
     // Issue #75
     assertEquals(
       getQueryParams("http://localhost:8080/blah?x=a+bc&y=ijk"),
-      Map("x" -> "a bc", "y" -> "ijk"),
+      Map("x" -> "a%20bc", "y" -> "ijk"),
     )
     assertEquals(
       getQueryParams("http://localhost:8080/blah?x=a%20bc&y=ijk"),
-      Map("x" -> "a bc", "y" -> "ijk"),
+      Map("x" -> "a%20bc", "y" -> "ijk"),
     )
   }
 
@@ -561,7 +561,7 @@ class UriSuite extends Http4sSuite {
       Uri
         .fromString("http://localhost:8080/index?filter[state]=public")
         .map(_.toString),
-      Right("http://localhost:8080/index?filter[state]=public"),
+      Right("http://localhost:8080/index?filter%5Bstate%5D=public"),
     )
   }
 
@@ -1073,6 +1073,9 @@ class UriSuite extends Http4sSuite {
 
   test("Uri.renderString should Encode special chars in the query") {
     val u = Uri(path = Uri.Path.Root).withQueryParam("foo", " !$&'()*+,;=:/?@~")
+
+    // /?foo=%20%21%24%26%27%28%29%2A%2B%2C%3B%3D%3A/?%40~
+    // /?foo=%2520%2521%2524%2526%2527%2528%2529%252A%252B%252C%253B%253D%253A/?%2540~
     assertEquals(u.renderString, "/?foo=%20%21%24%26%27%28%29%2A%2B%2C%3B%3D%3A/?%40~")
   }
   test("Uri.renderString should Encode special chars in the fragment") {
@@ -1421,11 +1424,23 @@ class UriSuite extends Http4sSuite {
   }
 
   test("Use lazy query model parsing in uri parsing") {
-    val ori = "http://domain.com/path?param1=asd;fgh"
+    val ori = "http://domain.com/path?param1=asd&fgh"
     val res = org.http4s.Uri.unsafeFromString(ori).renderString
     assertEquals(ori, res)
   }
 
+  test("Uri.renderString should behave correctly") {
+    val uri = Uri.fromString("https://foo.com/?with:colon=abc").toOption.get
+    println(uri)
+    val copy = uri.copy(query = Query.empty).setQueryParams(uri.multiParams)
+    println(copy)
+
+    assertEquals(
+      uri.renderString,
+      copy.renderString,
+    )
+  }
+
   property("resolving root sets path to root") {
     forAll { (uri: Uri) =>
       assertEquals(uri.resolve(uri"/").path, Uri.Path.Root)