From 741bb485c015b0431594864a36c33f88c2edcaf3 Mon Sep 17 00:00:00 2001 From: Renato Athaydes Date: Wed, 7 Dec 2022 22:25:11 +0100 Subject: [PATCH 1/4] Test for http to https redirect. --- .../client/RedirectingRawHttpClientTest.kt | 115 ++++++++++++------ 1 file changed, 76 insertions(+), 39 deletions(-) diff --git a/rawhttp-core/src/test/kotlin/rawhttp/core/client/RedirectingRawHttpClientTest.kt b/rawhttp-core/src/test/kotlin/rawhttp/core/client/RedirectingRawHttpClientTest.kt index e889f71..ca9a0a0 100644 --- a/rawhttp-core/src/test/kotlin/rawhttp/core/client/RedirectingRawHttpClientTest.kt +++ b/rawhttp-core/src/test/kotlin/rawhttp/core/client/RedirectingRawHttpClientTest.kt @@ -7,6 +7,7 @@ import org.junit.jupiter.api.Test import rawhttp.core.RawHttp import rawhttp.core.RawHttpRequest import java.net.URI +import java.util.concurrent.ConcurrentHashMap // a real HTTP response send by GitHub const val LARGE_REDIRECT = """ @@ -52,7 +53,8 @@ class RedirectingRawHttpClientTest { val redirectingClient = RedirectingRawHttpClient(mockClient) val actualResponse = redirectingClient.send( - http.parseRequest("GET /\nHost: myhost")) + http.parseRequest("GET /\nHost: myhost") + ) actualResponse shouldBe foo } @@ -78,7 +80,8 @@ class RedirectingRawHttpClientTest { val redirectingClient = RedirectingRawHttpClient(mockClient) val actualResponse = redirectingClient.send( - http.parseRequest("GET /\nHost: myhost")) + http.parseRequest("GET /\nHost: myhost") + ) actualResponse shouldBe resource } @@ -139,6 +142,7 @@ class RedirectingRawHttpClientTest { redirectRequest = req foo } + else -> fail("unexpected request: $req") } } @@ -146,13 +150,14 @@ class RedirectingRawHttpClientTest { val redirectingClient = RedirectingRawHttpClient(mockClient) val actualResponse = redirectingClient.send( - http.parseRequest("GET /\nHost: myhost\nAccept: application/json")) + http.parseRequest("GET /\nHost: myhost\nAccept: application/json") + ) actualResponse shouldBe foo redirectRequest!!.headers.asMap() shouldBe mapOf( - "HOST" to listOf("github-production-release-asset-2e65be.s3.amazonaws.com"), - "ACCEPT" to listOf("application/json") + "HOST" to listOf("github-production-release-asset-2e65be.s3.amazonaws.com"), + "ACCEPT" to listOf("application/json") ) redirectRequest!!.uri.path shouldBe "/40832723/b9093080-87e1-11ea-88bd-caad5f5731ae" } @@ -207,40 +212,40 @@ class RedirectingRawHttpClientTest { val actual = requests.map { it.uri.path to it.method } val expected = listOf( - "/301" to "GET", "/foo" to "GET", - "/301" to "POST", "/foo" to "POST", - "/301" to "PUT", "/foo" to "PUT", - "/301" to "DELETE", "/foo" to "DELETE", - "/301" to "HEAD", "/foo" to "HEAD", - "/301" to "OPTIONS", "/foo" to "OPTIONS", - - "/302" to "GET", "/foo" to "GET", - "/302" to "POST", "/foo" to "POST", - "/302" to "PUT", "/foo" to "PUT", - "/302" to "DELETE", "/foo" to "DELETE", - "/302" to "HEAD", "/foo" to "HEAD", - "/302" to "OPTIONS", "/foo" to "OPTIONS", - - "/303" to "GET", "/foo" to "GET", - "/303" to "POST", "/foo" to "GET", - "/303" to "PUT", "/foo" to "GET", - "/303" to "DELETE", "/foo" to "GET", - "/303" to "HEAD", "/foo" to "HEAD", - "/303" to "OPTIONS", "/foo" to "GET", - - "/307" to "GET", "/foo" to "GET", - "/307" to "POST", "/foo" to "POST", - "/307" to "PUT", "/foo" to "PUT", - "/307" to "DELETE", "/foo" to "DELETE", - "/307" to "HEAD", "/foo" to "HEAD", - "/307" to "OPTIONS", "/foo" to "OPTIONS", - - "/308" to "GET", "/foo" to "GET", - "/308" to "POST", "/foo" to "POST", - "/308" to "PUT", "/foo" to "PUT", - "/308" to "DELETE", "/foo" to "DELETE", - "/308" to "HEAD", "/foo" to "HEAD", - "/308" to "OPTIONS", "/foo" to "OPTIONS" + "/301" to "GET", "/foo" to "GET", + "/301" to "POST", "/foo" to "POST", + "/301" to "PUT", "/foo" to "PUT", + "/301" to "DELETE", "/foo" to "DELETE", + "/301" to "HEAD", "/foo" to "HEAD", + "/301" to "OPTIONS", "/foo" to "OPTIONS", + + "/302" to "GET", "/foo" to "GET", + "/302" to "POST", "/foo" to "POST", + "/302" to "PUT", "/foo" to "PUT", + "/302" to "DELETE", "/foo" to "DELETE", + "/302" to "HEAD", "/foo" to "HEAD", + "/302" to "OPTIONS", "/foo" to "OPTIONS", + + "/303" to "GET", "/foo" to "GET", + "/303" to "POST", "/foo" to "GET", + "/303" to "PUT", "/foo" to "GET", + "/303" to "DELETE", "/foo" to "GET", + "/303" to "HEAD", "/foo" to "HEAD", + "/303" to "OPTIONS", "/foo" to "GET", + + "/307" to "GET", "/foo" to "GET", + "/307" to "POST", "/foo" to "POST", + "/307" to "PUT", "/foo" to "PUT", + "/307" to "DELETE", "/foo" to "DELETE", + "/307" to "HEAD", "/foo" to "HEAD", + "/307" to "OPTIONS", "/foo" to "OPTIONS", + + "/308" to "GET", "/foo" to "GET", + "/308" to "POST", "/foo" to "POST", + "/308" to "PUT", "/foo" to "PUT", + "/308" to "DELETE", "/foo" to "DELETE", + "/308" to "HEAD", "/foo" to "HEAD", + "/308" to "OPTIONS", "/foo" to "OPTIONS" ) expected.forEachIndexed { i, item -> @@ -250,4 +255,36 @@ class RedirectingRawHttpClientTest { } + @Test + fun redirectFromHttpToHttps() { + val redirectWithSlash = http.parseResponse("302 Found\nLocation: /foo/").eagerly() + val redirectToHttps = http.parseResponse("302 Found\nLocation: https://myhost/foo/").eagerly() + val done = http.parseResponse("200 OK").eagerly() + val requests = ConcurrentHashMap.newKeySet() + + val mockClient = RawHttpClient { req -> + if (req.uri.scheme == "http") { + when (req.uri.path) { + "/foo" -> let { requests.add(1); redirectWithSlash } + "/foo/" -> let { requests.add(2); redirectToHttps } + else -> fail("unexpected request: $req") + } + } else { + requests.add(3) + done + } + } + + val redirectingClient = RedirectingRawHttpClient(mockClient) + + val actualResponse = redirectingClient.send( + http.parseRequest("GET /foo\nHost: myhost") + ) + + actualResponse shouldBe done + + // all expected requests were actually received + requests.containsAll(listOf(1, 2, 3)) + } + } From 022cc961f2a3eb0dd9b49033c83578530e9b6585 Mon Sep 17 00:00:00 2001 From: Renato Athaydes Date: Wed, 7 Dec 2022 22:26:44 +0100 Subject: [PATCH 2/4] Fixed #61 - consider host and scheme when redirecting. --- .../rawhttp/core/client/TcpRawHttpClient.java | 41 ++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/rawhttp-core/src/main/java/rawhttp/core/client/TcpRawHttpClient.java b/rawhttp-core/src/main/java/rawhttp/core/client/TcpRawHttpClient.java index 54daebe..6b361c4 100644 --- a/rawhttp-core/src/main/java/rawhttp/core/client/TcpRawHttpClient.java +++ b/rawhttp-core/src/main/java/rawhttp/core/client/TcpRawHttpClient.java @@ -21,6 +21,7 @@ import java.net.URI; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; @@ -311,7 +312,7 @@ default void removeSocket(Socket socket) { */ public static class DefaultOptions implements TcpRawHttpClientOptions { - private final Map socketByHost = new HashMap<>(4); + private final Map socketByHost = new HashMap<>(4); private final ExecutorService executorService; public DefaultOptions() { @@ -328,11 +329,11 @@ public DefaultOptions() { public Socket getSocket(URI uri) { String host = Optional.ofNullable(uri.getHost()).orElseThrow(() -> new RuntimeException("Host is not available in the URI")); + boolean useHttps = "https".equalsIgnoreCase(uri.getScheme()); - @Nullable Socket socket = socketByHost.get(host); + @Nullable Socket socket = socketByHost.get(new HostKey(host, useHttps)); if (socket == null || socket.isClosed() || !socket.isConnected()) { - boolean useHttps = "https".equalsIgnoreCase(uri.getScheme()); int port = uri.getPort(); if (port < 1) { port = useHttps ? 443 : 80; @@ -343,7 +344,7 @@ public Socket getSocket(URI uri) { } catch (IOException e) { throw new RuntimeException(e); } - socketByHost.put(host, socket); + socketByHost.put(new HostKey(host, useHttps), socket); } return socket; @@ -360,8 +361,10 @@ public RawHttpResponse onResponse(Socket socket, URI uri, RawHttpResponse httpResponse) throws IOException { if (RawHttpResponse.shouldCloseConnectionAfter(httpResponse)) { + boolean useHttps = "https".equalsIgnoreCase(uri.getScheme()); + // resolve the full response before closing the socket - try (Socket ignore = socketByHost.remove(uri.getHost())) { + try (Socket ignore = socketByHost.remove(new HostKey(uri.getHost(), useHttps))) { return httpResponse.eagerly(false); } } @@ -417,4 +420,32 @@ public RawHttpResponse call() throws Exception { } } + private static final class HostKey { + final String host; + final boolean https; + + HostKey(String host, boolean https) { + this.host = host; + this.https = https; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + HostKey hostKey = (HostKey) o; + + if (https != hostKey.https) return false; + return Objects.equals(host, hostKey.host); + } + + @Override + public int hashCode() { + int result = host != null ? host.hashCode() : 0; + result = 31 * result + (https ? 1 : 0); + return result; + } + } + } From 2f5e69a7a39ae79ae039bf60a21c128d9c9ef1a8 Mon Sep 17 00:00:00 2001 From: Renato Athaydes Date: Wed, 7 Dec 2022 22:34:15 +0100 Subject: [PATCH 3/4] #61 - test to ensure sockets are cached properly. --- .../core/client/ClientDefaultOptionsTest.kt | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 rawhttp-core/src/test/kotlin/rawhttp/core/client/ClientDefaultOptionsTest.kt diff --git a/rawhttp-core/src/test/kotlin/rawhttp/core/client/ClientDefaultOptionsTest.kt b/rawhttp-core/src/test/kotlin/rawhttp/core/client/ClientDefaultOptionsTest.kt new file mode 100644 index 0000000..7e102b5 --- /dev/null +++ b/rawhttp-core/src/test/kotlin/rawhttp/core/client/ClientDefaultOptionsTest.kt @@ -0,0 +1,27 @@ +package rawhttp.core.client + +import io.kotest.matchers.types.shouldBeSameInstanceAs +import io.kotest.matchers.types.shouldNotBeSameInstanceAs +import org.junit.jupiter.api.Test +import rawhttp.core.client.TcpRawHttpClient.DefaultOptions +import java.net.URI + +class ClientDefaultOptionsTest { + + @Test + fun doNotReuseSocketIfSchemeChanges() { + val options = DefaultOptions() + + val httpSocket1 = options.getSocket(URI.create("http://example.org")) + val httpsSocket1 = options.getSocket(URI.create("https://example.org")) + val httpSocket2 = options.getSocket(URI.create("http://example.org")) + val httpsSocket2 = options.getSocket(URI.create("https://example.org")) + + httpSocket1 shouldBeSameInstanceAs httpSocket2 + httpsSocket1 shouldBeSameInstanceAs httpsSocket2 + + httpSocket1 shouldNotBeSameInstanceAs httpsSocket1 + httpSocket2 shouldNotBeSameInstanceAs httpsSocket2 + } + +} From 947cfdc619100a23f5e429ccb3c42ba6fedc8141 Mon Sep 17 00:00:00 2001 From: Renato Athaydes Date: Wed, 7 Dec 2022 22:38:11 +0100 Subject: [PATCH 4/4] Bumped all versions. --- gradle.properties | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gradle.properties b/gradle.properties index 1451c76..73011b7 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,8 +1,8 @@ kotlinVersion=1.6.10 kotestVersion=4.6.2 kotlin.stdlib.default.dependency=false -rawHttpCoreVersion=2.5.1 -rawHttpCliVersion=1.5.1 -rawHttpDuplexVersion=1.4.1 -rawHttpReqInEditVersion=0.4.1 -rawHttpCookiesVersion=0.3.1 +rawHttpCoreVersion=2.5.2 +rawHttpCliVersion=1.5.2 +rawHttpDuplexVersion=1.4.2 +rawHttpReqInEditVersion=0.4.2 +rawHttpCookiesVersion=0.3.2