From f08abb578a8e7116c4a281d212df45e007b6bda0 Mon Sep 17 00:00:00 2001 From: John Zhang Date: Sat, 9 Apr 2022 15:12:12 +1000 Subject: [PATCH 1/3] separate cookies by space --- .../ktor/client/plugins/cookies/HttpCookies.kt | 2 +- .../ktor-client-core/common/test/CookiesTest.kt | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/cookies/HttpCookies.kt b/ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/cookies/HttpCookies.kt index 1f9d577b5e..28f6bdc785 100644 --- a/ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/cookies/HttpCookies.kt +++ b/ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/cookies/HttpCookies.kt @@ -119,7 +119,7 @@ public class HttpCookies internal constructor( } private fun renderClientCookies(cookies: List): String = - cookies.joinToString(";", transform = ::renderCookieHeader) + cookies.joinToString("; ", transform = ::renderCookieHeader) /** * Gets all the cookies for the specified [url] for this [HttpClient]. diff --git a/ktor-client/ktor-client-core/common/test/CookiesTest.kt b/ktor-client/ktor-client-core/common/test/CookiesTest.kt index 701936901c..6883ac6a62 100644 --- a/ktor-client/ktor-client-core/common/test/CookiesTest.kt +++ b/ktor-client/ktor-client-core/common/test/CookiesTest.kt @@ -42,10 +42,23 @@ class CookiesTest { assertEquals("test=value", builder.headers[HttpHeaders.Cookie]) } + @Test + fun testCookiesAreRenderedWithSpaceInBetween() = testSuspend{ + var storage = AcceptAllCookiesStorage() + storage.addCookie("http://localhost/", Cookie("name1", "value1")) + storage.addCookie("http://localhost/", Cookie("name2", "value2")) + val feature = HttpCookies(storage, emptyList()) + var builder = HttpRequestBuilder() + + feature.sendCookiesWith(builder) + + assertContains(builder.headers[HttpHeaders.Cookie]!!, "; ") + } + @Test fun testRequestCookiesArePreservedWhenAddingCookiesFromStorage() = testSuspend { val storage = AcceptAllCookiesStorage() - storage.addCookie("http://localhost/", parseServerSetCookieHeader("SOMECOOKIE=somevalue;")) + storage.addCookie("http://localhost/", Cookie("SOMECOOKIE", "somevalue")) val feature = HttpCookies(storage, emptyList()) val builder = HttpRequestBuilder() @@ -53,7 +66,7 @@ class CookiesTest { feature.captureHeaderCookies(builder) feature.sendCookiesWith(builder) - val renderedCookies = builder.headers[HttpHeaders.Cookie]!!.split(";") + val renderedCookies = builder.headers[HttpHeaders.Cookie]!!.split("; ") assertContains(renderedCookies, "test=value") assertContains(renderedCookies, "SOMECOOKIE=somevalue") } From a7e30fbe5d7a4fdb02cc858ce6ae32303cad6343 Mon Sep 17 00:00:00 2001 From: John Zhang Date: Sat, 9 Apr 2022 15:42:55 +1000 Subject: [PATCH 2/3] fix tests --- .../test/io/ktor/client/tests/CookiesTest.kt | 226 ++++++++++++++++++ .../client/tests/plugins/CookiesMockTest.kt | 2 +- .../tests/CookiesAndRedirectMockedTest.kt | 2 +- 3 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CookiesTest.kt diff --git a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CookiesTest.kt b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CookiesTest.kt new file mode 100644 index 0000000000..c90ab03276 --- /dev/null +++ b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CookiesTest.kt @@ -0,0 +1,226 @@ +/* +* Copyright 2014-2021 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. +*/ + +package io.ktor.client.tests + +import io.ktor.client.* +import io.ktor.client.call.* +import io.ktor.client.plugins.cookies.* +import io.ktor.client.request.* +import io.ktor.client.statement.* +import io.ktor.client.tests.utils.* +import io.ktor.http.* +import kotlin.test.* + +class CookiesTest : ClientLoader() { + private val hostname = "http://127.0.0.1/cookies" + private val TEST_HOST = "$TEST_SERVER/cookies" + private val domain = "127.0.0.1" + + @Test + fun testAccept() = clientTests(listOf("Js")) { + config { + install(HttpCookies) + } + + test { client -> + client.get(TEST_HOST).body() + client.cookies(hostname).let { + assertEquals(1, it.size) + assertEquals("my-awesome-value", it["hello-cookie"]!!.value) + } + } + } + + @Test + fun testUpdate() = clientTests(listOf("Js")) { + config { + install(HttpCookies) { + default { + addCookie(hostname, Cookie("id", "1", domain = domain)) + } + } + } + + test { client -> + repeat(10) { + val before = client.getId() + client.get("$TEST_HOST/update-user-id").body() + assertEquals(before + 1, client.getId()) + assertEquals("ktor", client.cookies(hostname)["user"]?.value) + } + } + } + + @Test + fun testExpiration() = clientTests(listOf("Js")) { + config { + install(HttpCookies) { + default { + addCookie(hostname, Cookie("id", "777", domain = domain, path = "/")) + } + } + + test { client -> + assertFalse(client.cookies(hostname).isEmpty()) + client.get("$TEST_HOST/expire").body() + assertTrue(client.cookies(hostname).isEmpty(), "cookie should be expired") + } + } + } + + @Test + fun testConstant() = clientTests(listOf("Js")) { + config { + install(HttpCookies) { + storage = ConstantCookiesStorage(Cookie("id", "1", domain = domain)) + } + } + + test { client -> + repeat(3) { + client.get("$TEST_HOST/update-user-id").body() + assertEquals(1, client.getId()) + assertNull(client.cookies(hostname)["user"]?.value) + } + } + } + + @Test + fun testMultipleCookies() = clientTests(listOf("Js")) { + config { + install(HttpCookies) { + default { + addCookie(hostname, Cookie("first", "first-cookie", domain = domain)) + addCookie(hostname, Cookie("second", "second-cookie", domain = domain)) + } + } + } + + test { client -> + val response = client.get("$TEST_HOST/multiple").body() + assertEquals("Multiple done", response) + } + } + + @Test + fun testPath() = clientTests(listOf("js")) { + config { + install(HttpCookies) + } + + test { client -> + assertEquals("OK", client.get("$TEST_HOST/withPath").body()) + assertEquals("OK", client.get("$TEST_HOST/withPath/something").body()) + } + } + + @Test + fun testWithLeadingDot() = clientTests(listOf("Js", "Darwin", "native:CIO")) { + config { + install(HttpCookies) + } + + test { client -> + client.get("https://m.vk.com").body() + assertTrue(client.cookies("https://.vk.com").isNotEmpty()) + assertTrue(client.cookies("https://vk.com").isNotEmpty()) + assertTrue(client.cookies("https://m.vk.com").isNotEmpty()) + assertTrue(client.cookies("https://m.vk.com").isNotEmpty()) + + assertTrue(client.cookies("https://google.com").isEmpty()) + } + } + + @Test + fun caseSensitive() = clientTests(listOf("Js", "Darwin")) { + config { + install(HttpCookies) + } + + test { client -> + try { + client.get("$TEST_HOST/foo").body() + client.get("$TEST_HOST/FOO").body() + } catch (cause: Throwable) { + throw cause + } + } + } + + @Test + fun testMultipleCookiesWithComma() = clientTests(listOf("Js")) { + config { + install(HttpCookies) { + default { + addCookie(hostname, Cookie("fir,st", "first, cookie", domain = domain)) + addCookie(hostname, Cookie("sec,ond", "second, cookie", domain = domain)) + } + } + } + + test { client -> + val response = client.get("$TEST_HOST/multiple-comma").body() + val cookies = client.cookies(hostname) + assertEquals("first, cookie", cookies["fir,st"]!!.value) + assertEquals("second, cookie", cookies["sec,ond"]!!.value) + assertEquals("third cookie", cookies["third"]!!.value) + assertEquals("fourth cookie", cookies["fourth"]!!.value) + + assertEquals("Multiple done", response) + } + } + + @Test + fun testCookiesEncodedWithRespectiveEncoding() = clientTests(listOf("Js")) { + val cookies = listOf( + Cookie("uri", "first, cookie", domain = domain, encoding = CookieEncoding.URI_ENCODING), + Cookie("raw", "first%2C+cookie", domain = domain, encoding = CookieEncoding.RAW), + Cookie("base64", "first, cookie", domain = domain, encoding = CookieEncoding.BASE64_ENCODING), + Cookie("dquotes", "first, cookie", domain = domain, encoding = CookieEncoding.DQUOTES) + ) + config { + install(HttpCookies) { + default { + cookies.forEach { addCookie(hostname, it) } + } + } + } + + test { client -> + client.prepareGet("$TEST_HOST/encoded").execute { httpResponse -> + val response = httpResponse.bodyAsText() + val cookieStrings = response.split("; ").filter { it.isNotBlank() } + assertEquals(4, cookieStrings.size) + assertEquals("uri=first%2C+cookie", cookieStrings[0]) + assertEquals("raw=first%2C+cookie", cookieStrings[1]) + assertEquals("base64=Zmlyc3QsIGNvb2tpZQ==", cookieStrings[2]) + assertEquals("dquotes=\"first, cookie\"", cookieStrings[3]) + } + } + } + + @Test + fun testRequestBuilderSingleCookie() = clientTests(listOf("Js")) { + test { client -> + val result = client.get("$TEST_HOST/respond-single-cookie") { + cookie("single", value = "abacaba") + }.body() + assertEquals("abacaba", result) + } + } + + @Test + fun testRequestBuilderMultipleCookies() = clientTests(listOf("Js")) { + test { client -> + val result = client.get("$TEST_HOST/respond-a-minus-b") { + cookie("a", value = "10") + cookie("b", value = "4") + }.body() + assertEquals("6", result) + } + } + + private suspend fun HttpClient.getId() = cookies(hostname)["id"]!!.value.toInt() +} diff --git a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CookiesMockTest.kt b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CookiesMockTest.kt index e52f29c2a3..4d442d35bc 100644 --- a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CookiesMockTest.kt +++ b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CookiesMockTest.kt @@ -22,7 +22,7 @@ class CookiesMockTest { assertEquals("*/*", request.headers[HttpHeaders.Accept]) val rawCookies = request.headers[HttpHeaders.Cookie]!! assertEquals(1, request.headers.getAll(HttpHeaders.Cookie)?.size!!) - assertEquals("first=\"1,2,3,4\";second=abc", rawCookies) + assertEquals("first=\"1,2,3,4\"; second=abc", rawCookies) respondOk() } diff --git a/ktor-client/ktor-client-tests/jvm/test/io/ktor/client/tests/CookiesAndRedirectMockedTest.kt b/ktor-client/ktor-client-tests/jvm/test/io/ktor/client/tests/CookiesAndRedirectMockedTest.kt index d38b90b58d..c81fee3ee9 100644 --- a/ktor-client/ktor-client-tests/jvm/test/io/ktor/client/tests/CookiesAndRedirectMockedTest.kt +++ b/ktor-client/ktor-client-tests/jvm/test/io/ktor/client/tests/CookiesAndRedirectMockedTest.kt @@ -76,7 +76,7 @@ class CookiesAndRedirectMockedTest { val storage = AcceptAllCookiesStorage() config { server { request -> - val cookies = request.headers[HttpHeaders.Cookie]!!.split(";") + val cookies = request.headers[HttpHeaders.Cookie]!!.split("; ") assertContains(cookies, "test=value") assertContains(cookies, "other=abc") respondOk() From 9bf7d5af684f4b63a840bd4cd59035d791225653 Mon Sep 17 00:00:00 2001 From: John Zhang Date: Sat, 9 Apr 2022 15:54:23 +1000 Subject: [PATCH 3/3] fix duplicate test --- .../test/io/ktor/client/tests/CookiesTest.kt | 226 ------------------ .../ktor/client/tests/plugins/CookiesTest.kt | 2 +- 2 files changed, 1 insertion(+), 227 deletions(-) delete mode 100644 ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CookiesTest.kt diff --git a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CookiesTest.kt b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CookiesTest.kt deleted file mode 100644 index c90ab03276..0000000000 --- a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CookiesTest.kt +++ /dev/null @@ -1,226 +0,0 @@ -/* -* Copyright 2014-2021 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. -*/ - -package io.ktor.client.tests - -import io.ktor.client.* -import io.ktor.client.call.* -import io.ktor.client.plugins.cookies.* -import io.ktor.client.request.* -import io.ktor.client.statement.* -import io.ktor.client.tests.utils.* -import io.ktor.http.* -import kotlin.test.* - -class CookiesTest : ClientLoader() { - private val hostname = "http://127.0.0.1/cookies" - private val TEST_HOST = "$TEST_SERVER/cookies" - private val domain = "127.0.0.1" - - @Test - fun testAccept() = clientTests(listOf("Js")) { - config { - install(HttpCookies) - } - - test { client -> - client.get(TEST_HOST).body() - client.cookies(hostname).let { - assertEquals(1, it.size) - assertEquals("my-awesome-value", it["hello-cookie"]!!.value) - } - } - } - - @Test - fun testUpdate() = clientTests(listOf("Js")) { - config { - install(HttpCookies) { - default { - addCookie(hostname, Cookie("id", "1", domain = domain)) - } - } - } - - test { client -> - repeat(10) { - val before = client.getId() - client.get("$TEST_HOST/update-user-id").body() - assertEquals(before + 1, client.getId()) - assertEquals("ktor", client.cookies(hostname)["user"]?.value) - } - } - } - - @Test - fun testExpiration() = clientTests(listOf("Js")) { - config { - install(HttpCookies) { - default { - addCookie(hostname, Cookie("id", "777", domain = domain, path = "/")) - } - } - - test { client -> - assertFalse(client.cookies(hostname).isEmpty()) - client.get("$TEST_HOST/expire").body() - assertTrue(client.cookies(hostname).isEmpty(), "cookie should be expired") - } - } - } - - @Test - fun testConstant() = clientTests(listOf("Js")) { - config { - install(HttpCookies) { - storage = ConstantCookiesStorage(Cookie("id", "1", domain = domain)) - } - } - - test { client -> - repeat(3) { - client.get("$TEST_HOST/update-user-id").body() - assertEquals(1, client.getId()) - assertNull(client.cookies(hostname)["user"]?.value) - } - } - } - - @Test - fun testMultipleCookies() = clientTests(listOf("Js")) { - config { - install(HttpCookies) { - default { - addCookie(hostname, Cookie("first", "first-cookie", domain = domain)) - addCookie(hostname, Cookie("second", "second-cookie", domain = domain)) - } - } - } - - test { client -> - val response = client.get("$TEST_HOST/multiple").body() - assertEquals("Multiple done", response) - } - } - - @Test - fun testPath() = clientTests(listOf("js")) { - config { - install(HttpCookies) - } - - test { client -> - assertEquals("OK", client.get("$TEST_HOST/withPath").body()) - assertEquals("OK", client.get("$TEST_HOST/withPath/something").body()) - } - } - - @Test - fun testWithLeadingDot() = clientTests(listOf("Js", "Darwin", "native:CIO")) { - config { - install(HttpCookies) - } - - test { client -> - client.get("https://m.vk.com").body() - assertTrue(client.cookies("https://.vk.com").isNotEmpty()) - assertTrue(client.cookies("https://vk.com").isNotEmpty()) - assertTrue(client.cookies("https://m.vk.com").isNotEmpty()) - assertTrue(client.cookies("https://m.vk.com").isNotEmpty()) - - assertTrue(client.cookies("https://google.com").isEmpty()) - } - } - - @Test - fun caseSensitive() = clientTests(listOf("Js", "Darwin")) { - config { - install(HttpCookies) - } - - test { client -> - try { - client.get("$TEST_HOST/foo").body() - client.get("$TEST_HOST/FOO").body() - } catch (cause: Throwable) { - throw cause - } - } - } - - @Test - fun testMultipleCookiesWithComma() = clientTests(listOf("Js")) { - config { - install(HttpCookies) { - default { - addCookie(hostname, Cookie("fir,st", "first, cookie", domain = domain)) - addCookie(hostname, Cookie("sec,ond", "second, cookie", domain = domain)) - } - } - } - - test { client -> - val response = client.get("$TEST_HOST/multiple-comma").body() - val cookies = client.cookies(hostname) - assertEquals("first, cookie", cookies["fir,st"]!!.value) - assertEquals("second, cookie", cookies["sec,ond"]!!.value) - assertEquals("third cookie", cookies["third"]!!.value) - assertEquals("fourth cookie", cookies["fourth"]!!.value) - - assertEquals("Multiple done", response) - } - } - - @Test - fun testCookiesEncodedWithRespectiveEncoding() = clientTests(listOf("Js")) { - val cookies = listOf( - Cookie("uri", "first, cookie", domain = domain, encoding = CookieEncoding.URI_ENCODING), - Cookie("raw", "first%2C+cookie", domain = domain, encoding = CookieEncoding.RAW), - Cookie("base64", "first, cookie", domain = domain, encoding = CookieEncoding.BASE64_ENCODING), - Cookie("dquotes", "first, cookie", domain = domain, encoding = CookieEncoding.DQUOTES) - ) - config { - install(HttpCookies) { - default { - cookies.forEach { addCookie(hostname, it) } - } - } - } - - test { client -> - client.prepareGet("$TEST_HOST/encoded").execute { httpResponse -> - val response = httpResponse.bodyAsText() - val cookieStrings = response.split("; ").filter { it.isNotBlank() } - assertEquals(4, cookieStrings.size) - assertEquals("uri=first%2C+cookie", cookieStrings[0]) - assertEquals("raw=first%2C+cookie", cookieStrings[1]) - assertEquals("base64=Zmlyc3QsIGNvb2tpZQ==", cookieStrings[2]) - assertEquals("dquotes=\"first, cookie\"", cookieStrings[3]) - } - } - } - - @Test - fun testRequestBuilderSingleCookie() = clientTests(listOf("Js")) { - test { client -> - val result = client.get("$TEST_HOST/respond-single-cookie") { - cookie("single", value = "abacaba") - }.body() - assertEquals("abacaba", result) - } - } - - @Test - fun testRequestBuilderMultipleCookies() = clientTests(listOf("Js")) { - test { client -> - val result = client.get("$TEST_HOST/respond-a-minus-b") { - cookie("a", value = "10") - cookie("b", value = "4") - }.body() - assertEquals("6", result) - } - } - - private suspend fun HttpClient.getId() = cookies(hostname)["id"]!!.value.toInt() -} diff --git a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CookiesTest.kt b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CookiesTest.kt index 6e7b1f0ddb..152886f432 100644 --- a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CookiesTest.kt +++ b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CookiesTest.kt @@ -192,7 +192,7 @@ class CookiesTest : ClientLoader() { test { client -> client.prepareGet("$TEST_HOST/encoded").execute { httpResponse -> val response = httpResponse.bodyAsText() - val cookieStrings = response.split(";").filter { it.isNotBlank() } + val cookieStrings = response.split("; ").filter { it.isNotBlank() } assertEquals(4, cookieStrings.size) assertEquals("uri=first%2C+cookie", cookieStrings[0]) assertEquals("raw=first%2C+cookie", cookieStrings[1])