Review comments

Signed-off-by: Jorge Bescos Gascon <[email protected]>
eclipse-ee4j · Jun 27, 2022 · 7fe4ad3 · 7fe4ad3
1 parent f738329
commit 7fe4ad3
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 21 deletions.
diff --git a/core-client/src/main/java/org/glassfish/jersey/client/HttpUrlConnectorProvider.java b/core-client/src/main/java/org/glassfish/jersey/client/HttpUrlConnectorProvider.java
@@ -255,6 +255,20 @@ protected Connector createHttpUrlConnector(Client client, ConnectionFactory conn
      */
     public interface ConnectionFactory {
 
+        /**
+         * Get a {@link java.net.HttpURLConnection} for a given URL.
+         * <p>
+         * Implementation of the method MUST be thread-safe and MUST ensure that
+         * a dedicated {@link java.net.HttpURLConnection} instance is returned for concurrent
+         * requests.
+         * </p>
+         *
+         * @param url the endpoint URL.
+         * @return the {@link java.net.HttpURLConnection}.
+         * @throws java.io.IOException in case the connection cannot be provided.
+         */
+        public HttpURLConnection getConnection(URL url) throws IOException;
+
         /**
          * Get a {@link java.net.HttpURLConnection} for a given URL.
          * <p>
@@ -268,14 +282,16 @@ public interface ConnectionFactory {
          * @return the {@link java.net.HttpURLConnection}.
          * @throws java.io.IOException in case the connection cannot be provided.
          */
-        public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException;
+        default HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException {
+            return (proxy == null) ? getConnection(url) : (HttpURLConnection) url.openConnection(proxy);
+        }
     }
 
     private static class DefaultConnectionFactory implements ConnectionFactory {
 
         @Override
-        public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException {
-            return (HttpURLConnection) (proxy != null ? url.openConnection(proxy) : url.openConnection());
+        public HttpURLConnection getConnection(final URL url) throws IOException {
+            return (HttpURLConnection) url.openConnection();
         }
     }
 

diff --git a/core-client/src/test/java/org/glassfish/jersey/client/HttpUrlConnectorTest.java b/core-client/src/test/java/org/glassfish/jersey/client/HttpUrlConnectorTest.java
@@ -16,14 +16,11 @@
 
 package org.glassfish.jersey.client;
 
-import static org.junit.Assert.assertEquals;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
 import java.net.ProtocolException;
-import java.net.Proxy;
 import java.net.SocketTimeoutException;
 import java.net.URI;
 import java.net.URL;
@@ -33,10 +30,6 @@
 import java.util.List;
 import java.util.Map;
 
-import javax.net.ssl.HostnameVerifier;
-import javax.net.ssl.HttpsURLConnection;
-import javax.net.ssl.SSLPeerUnverifiedException;
-import javax.net.ssl.SSLSocketFactory;
 import javax.ws.rs.client.Client;
 import javax.ws.rs.client.ClientBuilder;
 import javax.ws.rs.client.Entity;
@@ -46,10 +39,17 @@
 import javax.ws.rs.core.Link;
 import javax.ws.rs.core.Response;
 
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLPeerUnverifiedException;
+import javax.net.ssl.SSLSocketFactory;
+
 import org.glassfish.jersey.client.internal.HttpUrlConnector;
+
 import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
+import static org.junit.Assert.assertEquals;
 
 /**
  * Various tests for the default client connector.
@@ -91,7 +91,7 @@ public void testConnectionTimeoutNoEntity() {
     public void testResolvedRequestUri() {
         HttpUrlConnectorProvider.ConnectionFactory factory = new HttpUrlConnectorProvider.ConnectionFactory() {
             @Override
-            public HttpURLConnection getConnection(URL endpointUrl, Proxy proxy) throws IOException {
+            public HttpURLConnection getConnection(URL endpointUrl) throws IOException {
                 HttpURLConnection result = (HttpURLConnection) endpointUrl.openConnection();
                 return wrapRedirectedHttp(result);
             }
@@ -435,7 +435,7 @@ public void testSSLConnection() {
         ClientRequest request = client.target("https://localhost:8080").request().buildGet().request();
         HttpUrlConnectorProvider.ConnectionFactory factory = new HttpUrlConnectorProvider.ConnectionFactory() {
             @Override
-            public HttpURLConnection getConnection(URL endpointUrl, Proxy proxy) throws IOException {
+            public HttpURLConnection getConnection(URL endpointUrl) throws IOException {
                 HttpURLConnection result = (HttpURLConnection) endpointUrl.openConnection();
                 return wrapNoContentHttps(result);
             }

diff --git a/...est/java/org/glassfish/jersey/tests/e2e/client/connector/ssl/SslHttpUrlConnectorTest.java b/...est/java/org/glassfish/jersey/tests/e2e/client/connector/ssl/SslHttpUrlConnectorTest.java
@@ -16,15 +16,11 @@
 
 package org.glassfish.jersey.tests.e2e.client.connector.ssl;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.net.HttpURLConnection;
 import java.net.InetAddress;
-import java.net.Proxy;
 import java.net.Socket;
 import java.net.URL;
 import java.util.List;
@@ -34,20 +30,24 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 
-import javax.net.ssl.HttpsURLConnection;
-import javax.net.ssl.SSLContext;
-import javax.net.ssl.SSLSocketFactory;
 import javax.ws.rs.client.Client;
 import javax.ws.rs.client.ClientBuilder;
 import javax.ws.rs.core.Response;
 
+import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSocketFactory;
+
 import org.glassfish.jersey.apache.connector.ApacheConnectorProvider;
 import org.glassfish.jersey.apache5.connector.Apache5ConnectorProvider;
 import org.glassfish.jersey.client.ClientConfig;
 import org.glassfish.jersey.client.HttpUrlConnectorProvider;
 import org.glassfish.jersey.client.authentication.HttpAuthenticationFeature;
 import org.glassfish.jersey.logging.LoggingFeature;
+
 import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Test custom socket factory in HttpUrlConnection using SSL
@@ -70,7 +70,7 @@ public void testSSLWithCustomSocketFactory() throws Exception {
                 .connectorProvider(new HttpUrlConnectorProvider().connectionFactory(
                         new HttpUrlConnectorProvider.ConnectionFactory() {
                             @Override
-                            public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException {
+                            public HttpURLConnection getConnection(final URL url) throws IOException {
                                 HttpsURLConnection connection = (HttpsURLConnection) url.openConnection();
                                 connection.setSSLSocketFactory(socketFactory);
                                 return connection;

diff --git a/...properties/src/test/java/org/glassfish/jersey/tests/externalproperties/HttpProxyTest.java b/...properties/src/test/java/org/glassfish/jersey/tests/externalproperties/HttpProxyTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2022 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at