You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2022/11/06 10:19:43 UTC

[httpcomponents-client] branch master updated (233a5bdbb -> 16b2cf467)

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git


    from 233a5bdbb Use docker-compose to start compatibility test containers
     new fe1e095ef Regression: connection managers fail to take into account per route connection config when closing expired connections
     new 16b2cf467 HTTPCLIENT-2242: RoutingSupport fails to copy InetAddress when normalizing HttpHost

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../testing/sync/TestConnectionManagement.java     |  4 +--
 .../hc/client5/http/config/ConnectionConfig.java   |  2 +-
 .../io/PoolingHttpClientConnectionManager.java     | 35 +++++++++++++++++++---
 .../nio/PoolingAsyncClientConnectionManager.java   | 35 +++++++++++++++++++---
 .../hc/client5/http/routing/RoutingSupport.java    |  2 +-
 .../http/impl/routing/TestRoutingSupport.java      | 22 ++++++++++++++
 6 files changed, 88 insertions(+), 12 deletions(-)


[httpcomponents-client] 02/02: HTTPCLIENT-2242: RoutingSupport fails to copy InetAddress when normalizing HttpHost

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit 16b2cf467dec08a6f8f0db790f0b6e4157dacb92
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sun Nov 6 11:18:15 2022 +0100

    HTTPCLIENT-2242: RoutingSupport fails to copy InetAddress when normalizing HttpHost
---
 .../hc/client5/http/routing/RoutingSupport.java    |  2 +-
 .../http/impl/routing/TestRoutingSupport.java      | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/routing/RoutingSupport.java b/httpclient5/src/main/java/org/apache/hc/client5/http/routing/RoutingSupport.java
index 4205a1923..7db6933d1 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/routing/RoutingSupport.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/routing/RoutingSupport.java
@@ -73,7 +73,7 @@ public final class RoutingSupport {
         if (host.getPort() < 0) {
             final int port = (schemePortResolver != null ? schemePortResolver: DefaultSchemePortResolver.INSTANCE).resolve(host);
             if (port > 0) {
-                return new HttpHost(host.getSchemeName(), host.getHostName(), port);
+                return new HttpHost(host.getSchemeName(), host.getAddress(), host.getHostName(), port);
             }
         }
         return host;
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/routing/TestRoutingSupport.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/routing/TestRoutingSupport.java
index 35de7b147..073be3dc5 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/routing/TestRoutingSupport.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/routing/TestRoutingSupport.java
@@ -29,8 +29,10 @@ package org.apache.hc.client5.http.impl.routing;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 
+import java.net.InetAddress;
 import java.net.URI;
 
+import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
 import org.apache.hc.client5.http.routing.RoutingSupport;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequest;
@@ -62,4 +64,24 @@ public class TestRoutingSupport {
                 RoutingSupport.determineHost(request1));
     }
 
+    @Test
+    public void testNormalizeHost() throws Exception {
+        Assertions.assertEquals(new HttpHost("http", "somehost", 80),
+                RoutingSupport.normalize(
+                        new HttpHost("http", "somehost", -1),
+                        DefaultSchemePortResolver.INSTANCE));
+        Assertions.assertEquals(new HttpHost("https", "somehost", 443),
+                RoutingSupport.normalize(
+                        new HttpHost("https", "somehost", -1),
+                        DefaultSchemePortResolver.INSTANCE));
+        Assertions.assertEquals(new HttpHost("http", InetAddress.getLocalHost(), "localhost", 80),
+                RoutingSupport.normalize(
+                        new HttpHost("http", InetAddress.getLocalHost(), "localhost", -1),
+                        DefaultSchemePortResolver.INSTANCE));
+        Assertions.assertEquals(new HttpHost("http", "somehost", 8080),
+                RoutingSupport.normalize(
+                        new HttpHost("http", "somehost", 8080),
+                        DefaultSchemePortResolver.INSTANCE));
+    }
+
 }


[httpcomponents-client] 01/02: Regression: connection managers fail to take into account per route connection config when closing expired connections

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit fe1e095ef9e0354cf59694e0002e71ba284ffbb6
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sun Nov 6 11:01:10 2022 +0100

    Regression: connection managers fail to take into account per route connection config when closing expired connections
---
 .../testing/sync/TestConnectionManagement.java     |  4 +--
 .../hc/client5/http/config/ConnectionConfig.java   |  2 +-
 .../io/PoolingHttpClientConnectionManager.java     | 35 +++++++++++++++++++---
 .../nio/PoolingAsyncClientConnectionManager.java   | 35 +++++++++++++++++++---
 4 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java
index a9dd59b44..37f8c46d2 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java
@@ -58,7 +58,6 @@ import org.apache.hc.core5.testing.classic.ClassicTestServer;
 import org.apache.hc.core5.util.TimeValue;
 import org.apache.hc.core5.util.Timeout;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
@@ -276,7 +275,7 @@ public class TestConnectionManagement {
         connManager.close();
     }
 
-    @Test @Disabled
+    @Test
     public void testCloseExpiredTTLConnections() throws Exception {
         final ClassicTestServer server = startServer();
         server.registerHandler("/random/*", new RandomHandler());
@@ -294,6 +293,7 @@ public class TestConnectionManagement {
         );
 
         final PoolingHttpClientConnectionManager connManager = testResources.connManager();
+        connManager.setMaxTotal(1);
 
         final HttpRoute route = new HttpRoute(target, null, false);
         final HttpContext context = new BasicHttpContext();
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/ConnectionConfig.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/ConnectionConfig.java
index 126b38dae..4aafcd223 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/config/ConnectionConfig.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/config/ConnectionConfig.java
@@ -107,7 +107,7 @@ public class ConnectionConfig implements Cloneable {
     public String toString() {
         final StringBuilder builder = new StringBuilder();
         builder.append("[");
-        builder.append(", connectTimeout=").append(connectTimeout);
+        builder.append("connectTimeout=").append(connectTimeout);
         builder.append(", socketTimeout=").append(socketTimeout);
         builder.append(", validateAfterInactivity=").append(validateAfterInactivity);
         builder.append(", timeToLive=").append(timeToLive);
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java
index 953be5f2f..7be931266 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java
@@ -188,19 +188,33 @@ public class PoolingHttpClientConnectionManager
         this.connectionOperator = Args.notNull(httpClientConnectionOperator, "Connection operator");
         switch (poolConcurrencyPolicy != null ? poolConcurrencyPolicy : PoolConcurrencyPolicy.STRICT) {
             case STRICT:
-                this.pool = new StrictConnPool<>(
+                this.pool = new StrictConnPool<HttpRoute, ManagedHttpClientConnection>(
                         DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
                         DEFAULT_MAX_TOTAL_CONNECTIONS,
                         timeToLive,
                         poolReusePolicy,
-                        null);
+                        null) {
+
+                    @Override
+                    public void closeExpired() {
+                        enumAvailable(e -> closeIfExpired(e));
+                    }
+
+                };
                 break;
             case LAX:
-                this.pool = new LaxConnPool<>(
+                this.pool = new LaxConnPool<HttpRoute, ManagedHttpClientConnection>(
                         DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
                         timeToLive,
                         poolReusePolicy,
-                        null);
+                        null) {
+
+                    @Override
+                    public void closeExpired() {
+                        enumAvailable(e -> closeIfExpired(e));
+                    }
+
+                };
                 break;
             default:
                 throw new IllegalArgumentException("Unexpected PoolConcurrencyPolicy value: " + poolConcurrencyPolicy);
@@ -570,6 +584,19 @@ public class PoolingHttpClientConnectionManager
         this.tlsConfigResolver = tlsConfigResolver;
     }
 
+    void closeIfExpired(final PoolEntry<HttpRoute, ManagedHttpClientConnection> entry) {
+        final long now = System.currentTimeMillis();
+        if (entry.getExpiryDeadline().isBefore(now)) {
+            entry.discardConnection(CloseMode.GRACEFUL);
+        } else {
+            final ConnectionConfig connectionConfig = resolveConnectionConfig(entry.getRoute());
+            final TimeValue timeToLive = connectionConfig.getTimeToLive();
+            if (timeToLive != null && Deadline.calculate(entry.getCreated(), timeToLive).isBefore(now)) {
+                entry.discardConnection(CloseMode.GRACEFUL);
+            }
+        }
+    }
+
     /**
      * @deprecated Use custom {@link #setConnectionConfigResolver(Resolver)}
      */
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java
index 66d621fa8..90bf0925f 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java
@@ -172,19 +172,33 @@ public class PoolingAsyncClientConnectionManager implements AsyncClientConnectio
         this.connectionOperator = Args.notNull(connectionOperator, "Connection operator");
         switch (poolConcurrencyPolicy != null ? poolConcurrencyPolicy : PoolConcurrencyPolicy.STRICT) {
             case STRICT:
-                this.pool = new StrictConnPool<>(
+                this.pool = new StrictConnPool<HttpRoute, ManagedAsyncClientConnection>(
                         DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
                         DEFAULT_MAX_TOTAL_CONNECTIONS,
                         timeToLive,
                         poolReusePolicy,
-                        null);
+                        null) {
+
+                    @Override
+                    public void closeExpired() {
+                        enumAvailable(e -> closeIfExpired(e));
+                    }
+
+                };
                 break;
             case LAX:
-                this.pool = new LaxConnPool<>(
+                this.pool = new LaxConnPool<HttpRoute, ManagedAsyncClientConnection>(
                         DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
                         timeToLive,
                         poolReusePolicy,
-                        null);
+                        null) {
+
+                    @Override
+                    public void closeExpired() {
+                        enumAvailable(e -> closeIfExpired(e));
+                    }
+
+                };
                 break;
             default:
                 throw new IllegalArgumentException("Unexpected PoolConcurrencyPolicy value: " + poolConcurrencyPolicy);
@@ -619,6 +633,19 @@ public class PoolingAsyncClientConnectionManager implements AsyncClientConnectio
         this.tlsConfigResolver = tlsConfigResolver;
     }
 
+    void closeIfExpired(final PoolEntry<HttpRoute, ManagedAsyncClientConnection > entry) {
+        final long now = System.currentTimeMillis();
+        if (entry.getExpiryDeadline().isBefore(now)) {
+            entry.discardConnection(CloseMode.GRACEFUL);
+        } else {
+            final ConnectionConfig connectionConfig = resolveConnectionConfig(entry.getRoute());
+            final TimeValue timeToLive = connectionConfig.getTimeToLive();
+            if (timeToLive != null && Deadline.calculate(entry.getCreated(), timeToLive).isBefore(now)) {
+                entry.discardConnection(CloseMode.GRACEFUL);
+            }
+        }
+    }
+
     /**
      * @deprecated Use custom {@link #setConnectionConfigResolver(Resolver)}
      */