GH-801: Adding support for JDK Proxy (#1045)

Fixes #801 Adding a `Proxied` client implementation that extends the `Default` client allowing for a JDK Proxy, along with explict credential support.
OpenFeign · Aug 26, 2019 · 4c0f7dc · 4c0f7dc
1 parent e608944
commit 4c0f7dc
Show file tree

Hide file tree

Showing 3 changed files with 141 additions and 37 deletions.
diff --git a/core/src/main/java/feign/Client.java b/core/src/main/java/feign/Client.java
@@ -13,26 +13,36 @@
  */
 package feign;
 
+import static feign.Util.CONTENT_ENCODING;
+import static feign.Util.CONTENT_LENGTH;
+import static feign.Util.ENCODING_DEFLATE;
+import static feign.Util.ENCODING_GZIP;
+import static feign.Util.checkArgument;
+import static feign.Util.checkNotNull;
+import static feign.Util.isBlank;
+import static feign.Util.isNotBlank;
 import static java.lang.String.format;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
+import java.net.Proxy;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
 import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.zip.DeflaterOutputStream;
 import java.util.zip.GZIPOutputStream;
+
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLSocketFactory;
+
 import feign.Request.Options;
-import static feign.Util.CONTENT_ENCODING;
-import static feign.Util.CONTENT_LENGTH;
-import static feign.Util.ENCODING_DEFLATE;
-import static feign.Util.ENCODING_GZIP;
 
 /**
  * Submits HTTP {@link Request requests}. Implementations are expected to be thread-safe.
@@ -68,9 +78,49 @@ public Response execute(Request request, Options options) throws IOException {
       return convertResponse(connection, request);
     }
 
+    Response convertResponse(HttpURLConnection connection, Request request) throws IOException {
+      int status = connection.getResponseCode();
+      String reason = connection.getResponseMessage();
+
+      if (status < 0) {
+        throw new IOException(format("Invalid status(%s) executing %s %s", status,
+            connection.getRequestMethod(), connection.getURL()));
+      }
+
+      Map<String, Collection<String>> headers = new LinkedHashMap<>();
+      for (Map.Entry<String, List<String>> field : connection.getHeaderFields().entrySet()) {
+        // response message
+        if (field.getKey() != null) {
+          headers.put(field.getKey(), field.getValue());
+        }
+      }
+
+      Integer length = connection.getContentLength();
+      if (length == -1) {
+        length = null;
+      }
+      InputStream stream;
+      if (status >= 400) {
+        stream = connection.getErrorStream();
+      } else {
+        stream = connection.getInputStream();
+      }
+      return Response.builder()
+          .status(status)
+          .reason(reason)
+          .headers(headers)
+          .request(request)
+          .body(stream, length)
+          .build();
+    }
+
+    public HttpURLConnection getConnection(final URL url) throws IOException {
+      return (HttpURLConnection) url.openConnection();
+    }
+
     HttpURLConnection convertAndSend(Request request, Options options) throws IOException {
-      final HttpURLConnection connection =
-          (HttpURLConnection) new URL(request.url()).openConnection();
+      final URL url = new URL(request.url());
+      final HttpURLConnection connection = this.getConnection(url);
       if (connection instanceof HttpsURLConnection) {
         HttpsURLConnection sslCon = (HttpsURLConnection) connection;
         if (sslContextFactory != null) {
@@ -138,41 +188,50 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
       }
       return connection;
     }
+  }
 
-    Response convertResponse(HttpURLConnection connection, Request request) throws IOException {
-      int status = connection.getResponseCode();
-      String reason = connection.getResponseMessage();
+  /**
+   * Client that supports a {@link java.net.Proxy}.
+   */
+  class Proxied extends Default {
 
-      if (status < 0) {
-        throw new IOException(format("Invalid status(%s) executing %s %s", status,
-            connection.getRequestMethod(), connection.getURL()));
-      }
+    public static final String PROXY_AUTHORIZATION = "Proxy-Authorization";
+    private final Proxy proxy;
+    private String credentials;
 
-      Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>();
-      for (Map.Entry<String, List<String>> field : connection.getHeaderFields().entrySet()) {
-        // response message
-        if (field.getKey() != null) {
-          headers.put(field.getKey(), field.getValue());
-        }
-      }
+    public Proxied(SSLSocketFactory sslContextFactory, HostnameVerifier hostnameVerifier,
+        Proxy proxy) {
+      super(sslContextFactory, hostnameVerifier);
+      checkNotNull(proxy, "a proxy is required.");
+      this.proxy = proxy;
+    }
 
-      Integer length = connection.getContentLength();
-      if (length == -1) {
-        length = null;
-      }
-      InputStream stream;
-      if (status >= 400) {
-        stream = connection.getErrorStream();
-      } else {
-        stream = connection.getInputStream();
+    public Proxied(SSLSocketFactory sslContextFactory, HostnameVerifier hostnameVerifier,
+        Proxy proxy, String proxyUser, String proxyPassword) {
+      this(sslContextFactory, hostnameVerifier, proxy);
+      checkArgument(isNotBlank(proxyUser), "proxy user is required.");
+      checkArgument(isNotBlank(proxyPassword), "proxy password is required.");
+      this.credentials = basic(proxyUser, proxyPassword);
+    }
+
+    @Override
+    public HttpURLConnection getConnection(URL url) throws IOException {
+      HttpURLConnection connection = (HttpURLConnection) url.openConnection(this.proxy);
+      if (isNotBlank(this.credentials)) {
+        connection.addRequestProperty(PROXY_AUTHORIZATION, this.credentials);
       }
-      return Response.builder()
-          .status(status)
-          .reason(reason)
-          .headers(headers)
-          .request(request)
-          .body(stream, length)
-          .build();
+      return connection;
+    }
+
+    public String getCredentials() {
+      return this.credentials;
+    }
+
+    private String basic(String username, String password) {
+      String token = username + ":" + password;
+      byte[] bytes = token.getBytes(StandardCharsets.ISO_8859_1);
+      String encoded = Base64.getEncoder().encodeToString(bytes);
+      return "Basic " + encoded;
     }
   }
 }
diff --git a/core/src/test/java/feign/client/DefaultClientTest.java b/core/src/test/java/feign/client/DefaultClientTest.java
@@ -13,10 +13,19 @@
  */
 package feign.client;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.core.Is.isA;
 import static org.junit.Assert.assertEquals;
+
+import feign.Client.Proxied;
 import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.net.InetSocketAddress;
 import java.net.ProtocolException;
+import java.net.Proxy;
+import java.net.Proxy.Type;
+import java.net.SocketAddress;
+import java.net.URL;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
 import org.junit.Test;
@@ -32,7 +41,7 @@
  */
 public class DefaultClientTest extends AbstractClientTest {
 
-  Client disableHostnameVerification =
+  protected Client disableHostnameVerification =
       new Client.Default(TrustingSSLSocketFactory.get(), new HostnameVerifier() {
         @Override
         public boolean verify(String s, SSLSession sslSession) {
@@ -103,4 +112,37 @@ public void canOverrideHostnameVerifier() throws IOException, InterruptedExcepti
     api.post("foo");
   }
 
+  private final SocketAddress proxyAddress =
+      new InetSocketAddress("proxy.example.com", 8080);
+
+  /**
+   * Test that the proxy is being used, but don't check the credentials.  Credentials can still
+   * be used, but they must be set using the appropriate system properties and testing that is
+   * not what we are looking to do here.
+   */
+  @Test
+  public void canCreateWithImplicitOrNoCredentials() throws Exception {
+    Proxied proxied = new Proxied(
+        TrustingSSLSocketFactory.get(), null,
+        new Proxy(Type.HTTP, proxyAddress));
+    assertThat(proxied).isNotNull();
+    assertThat(proxied.getCredentials()).isNullOrEmpty();
+
+    /* verify that the proxy */
+    HttpURLConnection connection = proxied.getConnection(new URL("http://www.example.com"));
+    assertThat(connection).isNotNull().isInstanceOf(HttpURLConnection.class);
+  }
+
+  @Test
+  public void canCreateWithExplicitCredentials() throws Exception {
+    Proxied proxied = new Proxied(
+        TrustingSSLSocketFactory.get(), null,
+        new Proxy(Type.HTTP, proxyAddress), "user", "password");
+    assertThat(proxied).isNotNull();
+    assertThat(proxied.getCredentials()).isNotBlank();
+
+    HttpURLConnection connection = proxied.getConnection(new URL("http://www.example.com"));
+    assertThat(connection).isNotNull().isInstanceOf(HttpURLConnection.class);
+  }
+
 }
diff --git a/pom.xml b/pom.xml
@@ -495,8 +495,11 @@
             <exclude>**/.idea/**</exclude>
             <exclude>**/target/**</exclude>
             <exclude>LICENSE</exclude>
+            <exclude>NOTICE</exclude>
+            <exclude>OSSMETADATA</exclude>
             <exclude>**/*.md</exclude>
             <exclude>bnd.bnd</exclude>
+            <exclude>travis/**</exclude>
             <exclude>src/test/resources/**</exclude>
             <exclude>src/main/resources/**</exclude>
           </excludes>