[ECO-5248] fix: make query params URL encoded

lib/src/main/java/io/ably/lib/http/HttpUtils.java

@@ -187,18 +187,12 @@ public static String encodeURIComponent(String str) {
         return builder.toString();
     }
 
-    private static void appendParams(StringBuilder uri, Param[] params) {
-        if(params != null && params.length > 0) {
-            uri.append('?').append(params[0].key).append('=').append(params[0].value);
-            for(int i = 1; i < params.length; i++) {
-                uri.append('&').append(params[i].key).append('=').append(params[i].value);
-            }
-        }
-    }
-
     static URL buildURL(String scheme, String host, int port, String path, Param[] params) {
-        StringBuilder builder = new StringBuilder(scheme).append(host).append(':').append(port).append(path);
-        appendParams(builder, params);
+        StringBuilder builder = new StringBuilder(scheme)
+            .append(host)
+            .append(':')
+            .append(port)
+            .append(HttpUtils.encodeParams(path, params));
 
         URL result = null;
         try {
@@ -208,12 +202,9 @@ static URL buildURL(String scheme, String host, int port, String path, Param[] p
     }
 
     static URL buildURL(String uri, Param[] params) {
-        StringBuilder builder = new StringBuilder(uri);
-        appendParams(builder, params);
-
         URL result = null;
         try {
-            result = new URL(builder.toString());
+            result = new URL(HttpUtils.encodeParams(uri, params));
         } catch (MalformedURLException e) {}
         return result;
     }

lib/src/test/java/io/ably/lib/test/rest/RestErrorTest.java

@@ -16,7 +16,6 @@
 import java.util.Map;
 import java.util.Vector;
 
-import static io.ably.lib.http.HttpUtils.encodeURIComponent;
 import static org.junit.Assert.assertTrue;
 
 public class RestErrorTest extends ParameterizedTest {
@@ -66,7 +65,7 @@ public void println(int severity, String tag, String msg, Throwable tr) {
             AblyRest ably = new AblyRest(opts);
 
             /* make a call that will generate an error */
-            ably.stats(new Param[]{new Param("message", encodeURIComponent("Test message")), new Param("href", href(12345))});
+            ably.stats(new Param[]{new Param("message", "Test message"), new Param("href", href(12345))});
         } catch (AblyException e) {
             /* verify that the expected error message is present */
             assertTrue(logMessages.get(0).contains(href(12345)));
@@ -95,7 +94,7 @@ public void println(int severity, String tag, String msg, Throwable tr) {
             AblyRest ably = new AblyRest(opts);
 
             /* make a call that will generate an error */
-            ably.stats(new Param[]{new Param("message", encodeURIComponent("Test message. See " + href(12345)))});
+            ably.stats(new Param[]{new Param("message", "Test message. See " + href(12345))});
         } catch (AblyException e) {
             /* verify that the expected error message is present */
             assertTrue(logMessages.get(0).contains(href(12345)));
@@ -124,7 +123,7 @@ public void println(int severity, String tag, String msg, Throwable tr) {
             AblyRest ably = new AblyRest(opts);
 
             /* make a call that will generate an error */
-            ably.stats(new Param[]{new Param("message", encodeURIComponent("Test message")), new Param("code", "12345")});
+            ably.stats(new Param[]{new Param("message", "Test message"), new Param("code", "12345")});
         } catch (AblyException e) {
             /* verify that the expected error message is present */
             assertTrue(logMessages.get(0).contains(href(12345)));

pubsub-adapter/src/test/kotlin/com/ably/EmbeddedServer.kt

@@ -15,11 +15,13 @@ data class Request(
 data class Response(
   val mimeType: String,
   val data: ByteArray,
+  val headers: Map<String, String> = emptyMap(),
 )
 
-fun json(json: String): Response = Response(
+fun json(json: String, headers: Map<String, String> = emptyMap()): Response = Response(
   mimeType = "application/json",
   data = json.toByteArray(),
+  headers = headers,
 )
 
 fun interface RequestHandler {
@@ -44,11 +46,17 @@ class EmbeddedServer(port: Int, private val requestHandler: RequestHandler? = nu
     val response = requestHandler?.handle(request)
     return response?.toNanoHttp() ?: newFixedLengthResponse("<!DOCTYPE html><title>404</title>")
   }
+
+  override fun start() {
+    start(SOCKET_READ_TIMEOUT, true)
+  }
 }
 
 private fun Response.toNanoHttp(): NanoHTTPD.Response = NanoHTTPD.newFixedLengthResponse(
   NanoHTTPD.Response.Status.OK,
   mimeType,
   ByteArrayInputStream(data),
   data.size.toLong(),
-)
+).apply {
+  headers.forEach { (key, value) -> addHeader(key, value) }
+}

pubsub-adapter/src/test/kotlin/com/ably/Utils.kt

@@ -1,5 +1,8 @@
 package com.ably
 
+import io.ably.lib.realtime.AblyRealtime
+import io.ably.lib.rest.AblyRest
+import io.ably.lib.types.ClientOptions
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.withContext
@@ -15,3 +18,29 @@ suspend fun waitFor(timeoutInMs: Long = 10_000, block: suspend () -> Boolean) {
     }
   }
 }
+
+fun createAblyRealtime(port: Int): AblyRealtime {
+  val options = ClientOptions("xxxxx:yyyyyyy").apply {
+    this.port = port
+    useBinaryProtocol = false
+    realtimeHost = "localhost"
+    restHost = "localhost"
+    tls = false
+    autoConnect = false
+  }
+
+  return AblyRealtime(options)
+}
+
+fun createAblyRest(port: Int): AblyRest {
+  val options = ClientOptions("xxxxx:yyyyyyy").apply {
+    this.port = port
+    useBinaryProtocol = false
+    realtimeHost = "localhost"
+    restHost = "localhost"
+    tls = false
+    autoConnect = false
+  }
+
+  return AblyRest(options)
+}

pubsub-adapter/src/test/kotlin/com/ably/pubsub/SdkWrapperAgentHeaderTest.kt

@@ -2,16 +2,13 @@ package com.ably.pubsub
 
 import app.cash.turbine.test
 import com.ably.EmbeddedServer
+import com.ably.createAblyRest
+import com.ably.createAblyRealtime
 import com.ably.json
-import com.ably.pubsub.SdkWrapperAgentHeaderTest.Companion.PORT
 import com.ably.waitFor
-import fi.iki.elonen.NanoHTTPD
 import io.ably.lib.BuildConfig
-import io.ably.lib.realtime.AblyRealtime
 import io.ably.lib.realtime.RealtimeClient
-import io.ably.lib.rest.AblyRest
 import io.ably.lib.rest.RestClient
-import io.ably.lib.types.ClientOptions
 import kotlinx.coroutines.test.runTest
 import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.BeforeAll
@@ -150,8 +147,8 @@ class SdkWrapperAgentHeaderTest {
 
   companion object {
 
-    const val PORT = 27332
-    lateinit var server: EmbeddedServer
+    private const val PORT = 27332
+    private lateinit var server: EmbeddedServer
 
     @JvmStatic
     @BeforeAll
@@ -162,7 +159,7 @@ class SdkWrapperAgentHeaderTest {
           else -> json("[]")
         }
       }
-      server.start(NanoHTTPD.SOCKET_READ_TIMEOUT, true)
+      server.start()
       waitFor { server.wasStarted() }
     }
 
@@ -171,31 +168,9 @@ class SdkWrapperAgentHeaderTest {
     fun tearDown() {
       server.stop()
     }
-  }
-}
-
-private fun createRealtimeClient(): RealtimeClient {
-  val options = ClientOptions("xxxxx:yyyyyyy").apply {
-    port = PORT
-    useBinaryProtocol = false
-    realtimeHost = "localhost"
-    restHost = "localhost"
-    tls = false
-    autoConnect = false
-  }
 
-  return RealtimeClient(AblyRealtime(options))
-}
+    private fun createRealtimeClient(): RealtimeClient = RealtimeClient(createAblyRealtime(PORT))
 
-private fun createRestClient(): RestClient {
-  val options = ClientOptions("xxxxx:yyyyyyy").apply {
-    port = PORT
-    useBinaryProtocol = false
-    realtimeHost = "localhost"
-    restHost = "localhost"
-    tls = false
-    autoConnect = false
+    private fun createRestClient(): RestClient = RestClient(createAblyRest(PORT))
   }
-
-  return RestClient(AblyRest(options))
 }

pubsub-adapter/src/test/kotlin/io/ably/lib/RequestsTest.kt

@@ -0,0 +1,92 @@
+package io.ably.lib
+
+import app.cash.turbine.test
+import com.ably.*
+import io.ably.lib.realtime.AblyRealtime
+import io.ably.lib.types.AsyncHttpPaginatedResponse
+import io.ably.lib.types.ErrorInfo
+import io.ably.lib.types.Param
+import kotlinx.coroutines.suspendCancellableCoroutine
+import kotlinx.coroutines.test.runTest
+import org.junit.jupiter.api.AfterAll
+import org.junit.jupiter.api.BeforeAll
+import kotlin.coroutines.resume
+import kotlin.coroutines.resumeWithException
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class RequestsTest {
+
+  @Test
+  fun `should encode params on pagination requests`() = runTest {
+    val client = createAblyRealtime()
+    server.servedRequests.test {
+      val paginatedResult = client.request("GET", "/page", arrayOf(Param("foo", "b a r")), null, null)
+      assertEquals(mapOf("foo" to "b a r"), awaitItem().params)
+      paginatedResult.next()
+      assertEquals(mapOf("param" to "1@1 2"), awaitItem().params)
+    }
+  }
+
+  @Test
+  fun `should encode params on async pagination requests`() = runTest {
+    val client = createAblyRealtime()
+    server.servedRequests.test {
+      val paginatedResult = suspendCancellableCoroutine { continuation ->
+        client.requestAsync("GET", "/page", arrayOf(Param("foo", "b a r")), null, null, object : AsyncHttpPaginatedResponse.Callback {
+          override fun onResponse(response: AsyncHttpPaginatedResponse?) {
+            continuation.resume(response!!)
+          }
+
+          override fun onError(reason: ErrorInfo?) {
+            continuation.resumeWithException(IllegalArgumentException(reason.toString()))
+          }
+
+        })
+      }
+      assertEquals(mapOf("foo" to "b a r"), awaitItem().params)
+      suspendCancellableCoroutine { continuation ->
+        paginatedResult.next(object : AsyncHttpPaginatedResponse.Callback {
+          override fun onResponse(response: AsyncHttpPaginatedResponse?) {
+            continuation.resume(response!!)
+          }
+
+          override fun onError(reason: ErrorInfo?) {
+            continuation.resumeWithException(IllegalArgumentException(reason.toString()))
+          }
+        })
+      }
+      assertEquals(mapOf("param" to "1@1 2"), awaitItem().params)
+    }
+  }
+
+  companion object {
+
+    private const val PORT = 27332
+    private lateinit var server: EmbeddedServer
+
+    @JvmStatic
+    @BeforeAll
+    fun setUp() = runTest {
+      server = EmbeddedServer(PORT) {
+        when (it.path) {
+          "/page" -> json("[]", buildMap {
+            put("Link", "<./page?param=1%401%202>; rel=\"next\"")
+          })
+
+          else -> error("Unhandled ${it.path}")
+        }
+      }
+      server.start()
+      waitFor { server.wasStarted() }
+    }
+
+    @JvmStatic
+    @AfterAll
+    fun tearDown() {
+      server.stop()
+    }
+
+    private fun createAblyRealtime(): AblyRealtime = createAblyRealtime(PORT)
+  }
+}