Merge pull request #62 from renatoathaydes/dev

Next release

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

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<String, Socket> socketByHost = new HashMap<>(4);
+        private final Map<HostKey, Socket> 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<Void> onResponse(Socket socket,
                                                 URI uri,
                                                 RawHttpResponse<Void> 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<Void> 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;
+        }
+    }
+
 }

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
+    }
+
+}

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,20 +142,22 @@ class RedirectingRawHttpClientTest {
                     redirectRequest = req
                     foo
                 }
+
                 else -> fail("unexpected request: $req")
             }
         }
 
         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<Int>()
+
+        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))
+    }
+
 }