disable connection pooling functionality

sdk/clientcore/http-netty4/pom.xml

@@ -205,6 +205,12 @@
       <version>2.5.2</version> <!-- {x-version-update;org.conscrypt:conscrypt-openjdk-uber;external_dependency} -->
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>4.11.0</version> <!-- {x-version-update;org.mockito:mockito-core;external_dependency} -->
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

sdk/clientcore/http-netty4/spotbugs-exclude.xml

@@ -66,4 +66,11 @@
     <Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
     <Class name="io.clientcore.http.netty4.implementation.Netty4ChannelInputStream" />
   </Match>
+  <Match>
+    <Bug pattern="DP_DO_INSIDE_DO_PRIVILEGED" />
+    <Or>
+    <Class name="io.clientcore.http.netty4.NettyHttpClientBuilderTests" />
+    <Class name="io.clientcore.http.netty4.implementation.Netty4ConnectionPoolTests" />
+    </Or>
+  </Match>
 </FindBugsFilter>

sdk/clientcore/http-netty4/src/main/java/io/clientcore/http/netty4/NettyHttpClientBuilder.java

@@ -293,15 +293,14 @@ public NettyHttpClientBuilder maximumHttpVersion(HttpProtocolVersion httpVersion
      * Sets the maximum number of connections allowed per remote address in the connection pool.
      * <p>
      * If not set, a default value of 500 is used.
+     * <p>
+     * <strong>A value of {@code 0} or less will disable connection pooling, and hence each request will
+     * get a newly created connection.</strong>
      *
-     * @param connectionPoolSize The maximum number of connections. Must be greater than or equal to 0.
+     * @param connectionPoolSize The maximum number of connections.
      * @return The updated builder.
      */
     public NettyHttpClientBuilder connectionPoolSize(int connectionPoolSize) {
-        if (connectionPoolSize < 0) {
-            throw LOGGER.throwableAtError()
-                .log("connectionPoolSize must be greater than or equal to 0", IllegalArgumentException::new);
-        }
         this.connectionPoolSize = connectionPoolSize;
         return this;
     }
@@ -404,14 +403,17 @@ public HttpClient build() {
         ProxyOptions buildProxyOptions
             = (proxyOptions == null) ? ProxyOptions.fromConfiguration(buildConfiguration, true) : proxyOptions;
 
-        Netty4ConnectionPool connectionPool
-            = new Netty4ConnectionPool(bootstrap, new ChannelInitializationProxyHandler(buildProxyOptions),
-                sslContextModifier, connectionPoolSize, connectionIdleTimeout, maxConnectionLifetime,
-                pendingAcquireTimeout, maxPendingAcquires, maximumHttpVersion);
+        Netty4ConnectionPool connectionPool = null;
+        if (connectionPoolSize > 0) {
+            connectionPool
+                = new Netty4ConnectionPool(bootstrap, new ChannelInitializationProxyHandler(buildProxyOptions),
+                    sslContextModifier, connectionPoolSize, connectionIdleTimeout, maxConnectionLifetime,
+                    pendingAcquireTimeout, maxPendingAcquires, maximumHttpVersion);
+        }
 
-        return new NettyHttpClient(group, connectionPool, buildProxyOptions,
-            new ChannelInitializationProxyHandler(buildProxyOptions), getTimeoutMillis(readTimeout),
-            getTimeoutMillis(responseTimeout), getTimeoutMillis(writeTimeout));
+        return new NettyHttpClient(bootstrap, group, connectionPool, buildProxyOptions,
+            new ChannelInitializationProxyHandler(buildProxyOptions), sslContextModifier, maximumHttpVersion,
+            getTimeoutMillis(readTimeout), getTimeoutMillis(responseTimeout), getTimeoutMillis(writeTimeout));
     }
 
     static long getTimeoutMillis(Duration duration) {

sdk/clientcore/http-netty4/src/main/java/io/clientcore/http/netty4/implementation/Netty4ChannelBinaryData.java

@@ -154,6 +154,10 @@ public void close() {
             // We need to drain it to ensure the connection can be safely reused.
             if (!streamDrained.get()) {
                 drainAndCleanupAsync();
+            } else {
+                eagerContent = null;
+                channel.disconnect();
+                channel.close();
             }
         }
     }
@@ -241,7 +245,6 @@ private void drainStream() {
                     bytes = eagerContent.toByteArray();
                 }
             } finally {
-                closed.set(true);
                 drainLatch.countDown();
                 eagerContent = null;
             }

sdk/clientcore/http-netty4/src/main/java/io/clientcore/http/netty4/implementation/Netty4ConnectionPool.java

@@ -15,14 +15,9 @@
 import io.netty.channel.ChannelInitializer;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.EventLoopGroup;
-import io.netty.handler.codec.http2.Http2SecurityUtil;
 import io.netty.handler.proxy.ProxyHandler;
-import io.netty.handler.ssl.ApplicationProtocolConfig;
-import io.netty.handler.ssl.ApplicationProtocolNames;
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslContextBuilder;
-import io.netty.handler.ssl.SslProvider;
-import io.netty.handler.ssl.SupportedCipherSuiteFilter;
 import io.netty.util.AttributeKey;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.Promise;
@@ -48,14 +43,16 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 
+import static io.clientcore.http.netty4.implementation.Netty4HandlerNames.CONNECTION_SETUP_ERROR;
 import static io.clientcore.http.netty4.implementation.Netty4HandlerNames.PROXY;
 import static io.clientcore.http.netty4.implementation.Netty4HandlerNames.SSL;
 import static io.clientcore.http.netty4.implementation.Netty4HandlerNames.SSL_INITIALIZER;
+import static io.clientcore.http.netty4.implementation.Netty4Utility.buildSslContext;
 
 /**
  * A pool of Netty channels that can be reused for requests to the same remote address.
  */
-public final class Netty4ConnectionPool implements Closeable {
+public class Netty4ConnectionPool implements Closeable {
 
     private static final AttributeKey<PooledConnection> POOLED_CONNECTION_KEY
         = AttributeKey.valueOf("pooled-connection-key");
@@ -373,7 +370,9 @@ public void initChannel(Channel channel) throws SSLException {
                         ProxyHandler proxyHandler = channelInitializationProxyHandler.createProxy(proxyChallenges);
                         pipeline.addFirst(PROXY, proxyHandler);
 
-                        pipeline.addLast("proxy-exception-handler", new ChannelInboundHandlerAdapter() {
+                        // This handler's only job is to catch any exception during the
+                        // connection setup (proxying, SSL handshake) and fail the promise.
+                        pipeline.addLast(CONNECTION_SETUP_ERROR, new ChannelInboundHandlerAdapter() {
                             @Override
                             public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
                                 promise.tryFailure(cause);
@@ -385,7 +384,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
                     // Add SSL handling if the request is HTTPS.
                     if (isHttps) {
                         InetSocketAddress inetSocketAddress = (InetSocketAddress) key.getFinalDestination();
-                        SslContext ssl = buildSslContext();
+                        SslContext ssl = buildSslContext(maximumHttpVersion, sslContextModifier);
                         // SSL handling is added last here. This is done as proxying could require SSL handling too.
                         channel.pipeline()
                             .addLast(SSL, ssl.newHandler(channel.alloc(), inetSocketAddress.getHostString(),
@@ -419,11 +418,15 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
                                 return;
                             }
                             ChannelPipeline pipeline = newChannel.pipeline();
-                            if (pipeline.get("proxy-exception-handler") != null) {
-                                pipeline.remove("proxy-exception-handler");
+                            if (pipeline.get(CONNECTION_SETUP_ERROR) != null) {
+                                pipeline.remove(CONNECTION_SETUP_ERROR);
                             }
                             promise.setSuccess(newChannel);
                         } else {
+                            ChannelPipeline pipeline = newChannel.pipeline();
+                            if (pipeline.get(CONNECTION_SETUP_ERROR) != null) {
+                                pipeline.remove(CONNECTION_SETUP_ERROR);
+                            }
                             promise.setFailure(proxyFuture.cause());
                             newChannel.close();
                             activeConnections.decrementAndGet();
@@ -437,37 +440,6 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
             return promise;
         }
 
-        private SslContext buildSslContext() throws SSLException {
-            SslContextBuilder sslContextBuilder
-                = SslContextBuilder.forClient().endpointIdentificationAlgorithm("HTTPS");
-            if (maximumHttpVersion == HttpProtocolVersion.HTTP_2) {
-                // If HTTP/2 is the maximum version, we need to ensure that ALPN is enabled.
-                SslProvider sslProvider = SslContext.defaultClientProvider();
-                ApplicationProtocolConfig.SelectorFailureBehavior selectorBehavior;
-                ApplicationProtocolConfig.SelectedListenerFailureBehavior selectedBehavior;
-                if (sslProvider == SslProvider.JDK) {
-                    selectorBehavior = ApplicationProtocolConfig.SelectorFailureBehavior.FATAL_ALERT;
-                    selectedBehavior = ApplicationProtocolConfig.SelectedListenerFailureBehavior.FATAL_ALERT;
-                } else {
-                    // Netty OpenSslContext doesn't support FATAL_ALERT, use NO_ADVERTISE and ACCEPT
-                    // instead.
-                    selectorBehavior = ApplicationProtocolConfig.SelectorFailureBehavior.NO_ADVERTISE;
-                    selectedBehavior = ApplicationProtocolConfig.SelectedListenerFailureBehavior.ACCEPT;
-                }
-
-                sslContextBuilder.ciphers(Http2SecurityUtil.CIPHERS, SupportedCipherSuiteFilter.INSTANCE)
-                    .applicationProtocolConfig(
-                        new ApplicationProtocolConfig(ApplicationProtocolConfig.Protocol.ALPN, selectorBehavior,
-                            selectedBehavior, ApplicationProtocolNames.HTTP_2, ApplicationProtocolNames.HTTP_1_1));
-            }
-            if (sslContextModifier != null) {
-                // Allow the caller to modify the SslContextBuilder before it is built.
-                sslContextModifier.accept(sslContextBuilder);
-            }
-
-            return sslContextBuilder.build();
-        }
-
         private boolean isHealthy(PooledConnection connection) {
             if (!connection.isActiveAndWriteable()) {
                 return false;

sdk/clientcore/http-netty4/src/main/java/io/clientcore/http/netty4/implementation/Netty4HandlerNames.java

@@ -74,6 +74,8 @@ public final class Netty4HandlerNames {
      */
     public static final String PIPELINE_CLEANUP = "clientcore.pipelinecleanup";
 
+    public static final String CONNECTION_SETUP_ERROR = "clientcore.connectionsetuperror";
+
     private Netty4HandlerNames() {
     }
 }

sdk/clientcore/http-netty4/src/main/java/io/clientcore/http/netty4/implementation/Netty4PipelineCleanupHandler.java

@@ -99,7 +99,7 @@ public void cleanup(ChannelHandlerContext ctx, boolean closeChannel) {
             pipeline.remove(this);
         }
 
-        if (closeChannel || !ctx.channel().isActive()) {
+        if (closeChannel || !ctx.channel().isActive() || connectionPool == null) {
             ctx.channel().close();
         } else {
             connectionPool.release(ctx.channel());

sdk/clientcore/http-netty4/src/main/java/io/clientcore/http/netty4/implementation/Netty4Utility.java

@@ -37,15 +37,23 @@
 import io.netty.handler.codec.http2.DelegatingDecompressorFrameListener;
 import io.netty.handler.codec.http2.Http2Connection;
 import io.netty.handler.codec.http2.Http2FrameListener;
+import io.netty.handler.codec.http2.Http2SecurityUtil;
 import io.netty.handler.codec.http2.Http2Settings;
 import io.netty.handler.codec.http2.HttpToHttp2ConnectionHandlerBuilder;
 import io.netty.handler.codec.http2.InboundHttp2ToHttpAdapterBuilder;
+import io.netty.handler.ssl.ApplicationProtocolConfig;
+import io.netty.handler.ssl.ApplicationProtocolNames;
+import io.netty.handler.ssl.SslContext;
+import io.netty.handler.ssl.SslContextBuilder;
+import io.netty.handler.ssl.SslProvider;
+import io.netty.handler.ssl.SupportedCipherSuiteFilter;
 import io.netty.handler.stream.ChunkedInput;
 import io.netty.handler.stream.ChunkedNioFile;
 import io.netty.handler.stream.ChunkedStream;
 import io.netty.handler.stream.ChunkedWriteHandler;
 import io.netty.util.Version;
 
+import javax.net.ssl.SSLException;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.channels.FileChannel;
@@ -58,6 +66,7 @@
 import java.util.Objects;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
 
 import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.trailersFactory;
 
@@ -554,6 +563,37 @@ private static io.netty.handler.codec.http.HttpRequest toNettyHttpRequest(HttpRe
         }
     }
 
+    public static SslContext buildSslContext(HttpProtocolVersion maximumHttpVersion,
+        Consumer<SslContextBuilder> sslContextModifier) throws SSLException {
+        SslContextBuilder sslContextBuilder = SslContextBuilder.forClient().endpointIdentificationAlgorithm("HTTPS");
+        if (maximumHttpVersion == HttpProtocolVersion.HTTP_2) {
+            // If HTTP/2 is the maximum version, we need to ensure that ALPN is enabled.
+            SslProvider sslProvider = SslContext.defaultClientProvider();
+            ApplicationProtocolConfig.SelectorFailureBehavior selectorBehavior;
+            ApplicationProtocolConfig.SelectedListenerFailureBehavior selectedBehavior;
+            if (sslProvider == SslProvider.JDK) {
+                selectorBehavior = ApplicationProtocolConfig.SelectorFailureBehavior.FATAL_ALERT;
+                selectedBehavior = ApplicationProtocolConfig.SelectedListenerFailureBehavior.FATAL_ALERT;
+            } else {
+                // Netty OpenSslContext doesn't support FATAL_ALERT, use NO_ADVERTISE and ACCEPT
+                // instead.
+                selectorBehavior = ApplicationProtocolConfig.SelectorFailureBehavior.NO_ADVERTISE;
+                selectedBehavior = ApplicationProtocolConfig.SelectedListenerFailureBehavior.ACCEPT;
+            }
+
+            sslContextBuilder.ciphers(Http2SecurityUtil.CIPHERS, SupportedCipherSuiteFilter.INSTANCE)
+                .applicationProtocolConfig(
+                    new ApplicationProtocolConfig(ApplicationProtocolConfig.Protocol.ALPN, selectorBehavior,
+                        selectedBehavior, ApplicationProtocolNames.HTTP_2, ApplicationProtocolNames.HTTP_1_1));
+        }
+        if (sslContextModifier != null) {
+            // Allow the caller to modify the SslContextBuilder before it is built.
+            sslContextModifier.accept(sslContextBuilder);
+        }
+
+        return sslContextBuilder.build();
+    }
+
     private Netty4Utility() {
     }
 }

sdk/clientcore/http-netty4/src/test/java/io/clientcore/http/netty4/NettyHttpClientBuilderTests.java

@@ -3,6 +3,7 @@
 
 package io.clientcore.http.netty4;
 
+import io.clientcore.core.http.client.HttpProtocolVersion;
 import io.clientcore.core.http.models.HttpMethod;
 import io.clientcore.core.http.models.HttpRequest;
 import io.clientcore.core.http.models.ProxyOptions;
@@ -32,6 +33,7 @@
 import java.io.IOException;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Field;
 import java.net.InetSocketAddress;
 import java.time.Duration;
 import java.util.ArrayList;
@@ -47,6 +49,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Tests {@link NettyHttpClientBuilder}.
@@ -372,6 +375,35 @@ public void getEventLoopGroupToUse(Class<?> expected, EventLoopGroup configuredG
         assertInstanceOf(expected, eventLoopGroup);
     }
 
+    @Test
+    public void buildNettyClientWithoutConnectionPool() throws NoSuchFieldException, IllegalAccessException {
+        NettyHttpClient client = (NettyHttpClient) new NettyHttpClientBuilder().connectionPoolSize(0).build();
+
+        Field connectionPoolField = NettyHttpClient.class.getDeclaredField("connectionPool");
+        connectionPoolField.setAccessible(true);
+        assertNull(connectionPoolField.get(client), "Connection pool should be null when pool size is 0.");
+    }
+
+    @Test
+    public void testInvalidMaxPendingAcquires() {
+        NettyHttpClientBuilder builder = new NettyHttpClientBuilder();
+        assertThrows(IllegalArgumentException.class, () -> builder.maxPendingAcquires(0));
+        assertThrows(IllegalArgumentException.class, () -> builder.maxPendingAcquires(-1));
+    }
+
+    @Test
+    public void testMaximumHttpVersion() throws NoSuchFieldException, IllegalAccessException {
+        NettyHttpClientBuilder builder = new NettyHttpClientBuilder();
+
+        NettyHttpClient clientv1 = (NettyHttpClient) builder.maximumHttpVersion(HttpProtocolVersion.HTTP_1_1).build();
+        Field httpVersionField = NettyHttpClient.class.getDeclaredField("maximumHttpVersion");
+        httpVersionField.setAccessible(true);
+        assertEquals(HttpProtocolVersion.HTTP_1_1, httpVersionField.get(clientv1));
+
+        NettyHttpClient clientv2 = (NettyHttpClient) builder.maximumHttpVersion(null).build();
+        assertEquals(HttpProtocolVersion.HTTP_2, httpVersionField.get(clientv2));
+    }
+
     private static Stream<Arguments> getEventLoopGroupToUseSupplier() throws ReflectiveOperationException {
         // Doesn't matter what this is calling, just needs to throw an exception.
         // This will as it doesn't accept the arguments that it will be called with.