Repository: relayrides/pushy
Pull request #751 (2020-02-24 16:59:21)

Also get credentials for HTTP proxies from system properties
As a follow-up to #691, this also gets usernames/passwords for HTTP proxies from system properties. @arunan-interlink your feedback is welcome and appreciated!

arunan-interlink: It appears to make sense after a quick eyeball.
jchambers: Cool. Thanks! We'll roll with it.

Patch:
diff --git a/pushy/src/main/java/com/eatthepath/pushy/apns/proxy/HttpProxyHandlerFactory.java b/pushy/src/main/java/com/eatthepath/pushy/apns/proxy/HttpProxyHandlerFactory.java
index 1ae69e29..3c77a333 100644
--- a/pushy/src/main/java/com/eatthepath/pushy/apns/proxy/HttpProxyHandlerFactory.java
+++ b/pushy/src/main/java/com/eatthepath/pushy/apns/proxy/HttpProxyHandlerFactory.java
@@ -22,19 +22,13 @@
 
 package com.eatthepath.pushy.apns.proxy;
 
-import java.net.InetSocketAddress;
-import java.net.Proxy;
-import java.net.ProxySelector;
-import java.net.SocketAddress;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.util.List;
-
+import io.netty.handler.proxy.HttpProxyHandler;
+import io.netty.handler.proxy.ProxyHandler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import io.netty.handler.proxy.HttpProxyHandler;
-import io.netty.handler.proxy.ProxyHandler;
+import java.net.*;
+import java.util.List;
 
 /**
  * A concrete {@link ProxyHandlerFactory} implementation that creates {@link HttpProxyHandler} instances.
@@ -52,45 +46,52 @@ public class HttpProxyHandlerFactory implements ProxyHandlerFactory {
     private final String username;
     private final String password;
 
+    private static final String PROXY_USERNAME_PROPERTY_KEY = "http.proxyUser";
+    private static final String PROXY_PASSWORD_PROPERTY_KEY = "http.proxyPassword";
+
     /**
-     * Checks the system default proxies to determine if an {@link HttpProxyHandlerFactory}
-     * is required for the given host or not, returning an instance or null for each case.
+     * Constructs an {@code HttpProxyHandlerFactory} that uses the HTTP proxy (if any) specified in Java's standard
+     * <a href="https://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html#Proxies">proxy system
+     * properties</a>.
      *
-     * @param apnsHost the APNs host to check e.g. "api.push.apple.com".
-     * @return an {@link HttpProxyHandlerFactory} if one is required, or {@code null} if not.
-     * @throws URISyntaxException if <code>apnsHost</code> is malformed in some way.
+     * @param apnsHost the APNs host for which to find proxy settings
+     * @return an HTTP proxy factory if a proxy is configured for the given APNs host, or {@code null} if no proxy is
+     * configured for the given host
+     * @throws URISyntaxException if {@code apnsHost} is malformed in some way
      *
-     * @see <a href="https://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html#Proxies">The Java documentation on proxies.</a>
-     * @since 0.14
+     * @see <a href="https://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html#Proxies">Java™ Platform, Standard Edition 7 - Networking Properties - Proxies</a>
+     * @since 0.13.11
      */
-    public static HttpProxyHandlerFactory fromSystemProxies(final String apnsHost)
-            throws URISyntaxException {
+    public static HttpProxyHandlerFactory fromSystemProxies(final String apnsHost) throws URISyntaxException {
 
-        final URI hostUri = new URI("https", apnsHost, null, null);
-        final SocketAddress proxyAddress = getProxyAddressForUri(hostUri, Proxy.Type.HTTP);
+        final SocketAddress proxyAddress = getProxyAddressForUri(new URI("https", apnsHost, null, null));
 
-        return (proxyAddress != null) ? new HttpProxyHandlerFactory(proxyAddress) : null;
+        return (proxyAddress != null)
+                ? new HttpProxyHandlerFactory(proxyAddress, System.getProperty(PROXY_USERNAME_PROPERTY_KEY), System.getProperty(PROXY_PASSWORD_PROPERTY_KEY))
+                : null;
     }
 
     /**
-     * Returns the {@link SocketAddress} for any system-wide {@link Proxy} of a given {@link Proxy.Type} for a given {@link URI}.
+     * Returns the {@link SocketAddress} for any system-wide HTTP {@link Proxy} for a given {@link URI}.
      *
-     * @param uri the URI to find any system proxy for.
-     * @param proxyType the types of proxy to find for the URI.
-     * @return the {@link SocketAddress} of any <code>Proxy</code> found, or <code>null</code> if there were none.
-     * @since 0.14
+     * @param uri the URI for which to find a system-wide proxy
+     * @return the {@link SocketAddress} of the first HTTP {@link Proxy} found, or {@code null} if there were none
+     * @since 0.13.11
      */
-    private static SocketAddress getProxyAddressForUri(final URI uri, final Proxy.Type proxyType) {
+    private static SocketAddress getProxyAddressForUri(final URI uri) {
         final ProxySelector defaultProxySelector = ProxySelector.getDefault();
         final List<Proxy> proxiesForUri = defaultProxySelector.select(uri);
-        log.debug("Proxies for URI \"{}\" were {}", uri, proxiesForUri);
 
-        for (java.net.Proxy proxy : proxiesForUri) {
-            if (proxy.type() == proxyType) {
+        log.debug("Proxies for \"{}\": {}", uri, proxiesForUri);
+
+        for (final java.net.Proxy proxy : proxiesForUri) {
+            if (proxy.type() == Proxy.Type.HTTP) {
                 InetSocketAddress proxyAddress = (InetSocketAddress) proxy.address();
+
                 if (proxyAddress.isUnresolved()) {
                     proxyAddress = new InetSocketAddress(proxyAddress.getHostString(), proxyAddress.getPort());
                 }
+
                 return proxyAddress;
             }
         }
@@ -147,6 +148,16 @@ public class HttpProxyHandlerFactory implements ProxyHandlerFactory {
         return handler;
     }
 
+    // Visible for testing
+    String getUsername() {
+        return this.username;
+    }
+
+    // Visible for testing
+    String getPassword() {
+        return this.password;
+    }
+
     /* (non-Javadoc)
      * @see java.lang.Object#toString()
      */


Test patch:
diff --git a/pushy/src/test/java/com/eatthepath/pushy/apns/proxy/HttpProxyHandlerFactoryTest.java b/pushy/src/test/java/com/eatthepath/pushy/apns/proxy/HttpProxyHandlerFactoryTest.java
index 94ee8a3d..c780bc8d 100644
--- a/pushy/src/test/java/com/eatthepath/pushy/apns/proxy/HttpProxyHandlerFactoryTest.java
+++ b/pushy/src/test/java/com/eatthepath/pushy/apns/proxy/HttpProxyHandlerFactoryTest.java
@@ -1,28 +1,30 @@
 package com.eatthepath.pushy.apns.proxy;
 
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
+import com.eatthepath.pushy.apns.ApnsClientBuilder;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
 
 import java.io.IOException;
-import java.net.InetSocketAddress;
-import java.net.Proxy;
-import java.net.ProxySelector;
-import java.net.SocketAddress;
-import java.net.URI;
-import java.net.URISyntaxException;
+import java.net.*;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Test;
-
-import com.eatthepath.pushy.apns.ApnsClientBuilder;
+import static org.junit.Assert.*;
 
 
 public class HttpProxyHandlerFactoryTest {
 
-    private static class FixedProxiesSelector extends ProxySelector {
+    private static abstract class TestProxySelectorAdapter extends ProxySelector {
+
+        @Override
+        public void connectFailed(final URI uri, final SocketAddress sa, final IOException ioe) {
+            // Not needed for this test.
+        }
+    }
+
+    private static class FixedProxiesSelector extends TestProxySelectorAdapter {
 
         private final List<Proxy> availableProxies;
 
@@ -31,17 +33,12 @@ public class HttpProxyHandlerFactoryTest {
         }
 
         @Override
-        public List<Proxy> select(URI uri) {
+        public List<Proxy> select(final URI uri) {
             return availableProxies;
         }
-
-        @Override
-        public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {
-            // Not needed for this test.
-        }
     }
 
-    private static class SingleHostHttpProxySelector extends ProxySelector {
+    private static class SingleHostHttpProxySelector extends TestProxySelectorAdapter {
 
         private final String proxiedHost;
 
@@ -50,27 +47,17 @@ public class HttpProxyHandlerFactoryTest {
         }
 
         @Override
-        public List<Proxy> select(URI uri) {
-            Proxy proxy;
-
-            if (uri.getHost().equals(proxiedHost)) {
-                proxy = DUMMY_HTTP_PROXY;
-            } else {
-                proxy = Proxy.NO_PROXY;
-            }
-
-            return Arrays.asList(proxy);
-        }
-
-        @Override
-        public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {
-            // Not needed for this test.
+        public List<Proxy> select(final URI uri) {
+            return Collections.singletonList(uri.getHost().equals(proxiedHost) ? DUMMY_HTTP_PROXY : Proxy.NO_PROXY);
         }
     }
 
     private static final Proxy DUMMY_HTTP_PROXY = new Proxy(Proxy.Type.HTTP, new InetSocketAddress("httpproxy", 123));
     private static final Proxy DUMMY_SOCKS_PROXY = new Proxy(Proxy.Type.SOCKS, new InetSocketAddress("socksproxy", 456));
 
+    private static final String USERNAME_PROPERTY_KEY = "http.proxyUser";
+    private static final String PASSWORD_PROPERTY_KEY = "http.proxyPassword";
+
     private static ProxySelector defaultProxySelector;
 
     @BeforeClass
@@ -136,4 +123,53 @@ public class HttpProxyHandlerFactoryTest {
         assertNull(HttpProxyHandlerFactory.fromSystemProxies(ApnsClientBuilder.PRODUCTION_APNS_HOST));
         assertNotNull(HttpProxyHandlerFactory.fromSystemProxies(ApnsClientBuilder.DEVELOPMENT_APNS_HOST));
     }
+
+    @Test
+    public void testProxyWithCredentials() throws URISyntaxException {
+        final String originalUsername = System.getProperty(USERNAME_PROPERTY_KEY);
+        final String originalPassword = System.getProperty(PASSWORD_PROPERTY_KEY);
+
+        try {
+            ProxySelector.setDefault(new SingleHostHttpProxySelector(ApnsClientBuilder.DEVELOPMENT_APNS_HOST));
+
+            {
+                System.clearProperty(USERNAME_PROPERTY_KEY);
+                System.clearProperty(PASSWORD_PROPERTY_KEY);
+
+                final HttpProxyHandlerFactory noCredentialProxyHandlerFactory =
+                        HttpProxyHandlerFactory.fromSystemProxies(ApnsClientBuilder.DEVELOPMENT_APNS_HOST);
+
+                assertNotNull(noCredentialProxyHandlerFactory);
+                assertNull(noCredentialProxyHandlerFactory.getUsername());
+                assertNull(noCredentialProxyHandlerFactory.getPassword());
+            }
+
+            {
+                final String username = "username";
+                final String password = "password";
+
+                System.setProperty(USERNAME_PROPERTY_KEY, username);
+                System.setProperty(PASSWORD_PROPERTY_KEY, password);
+
+                final HttpProxyHandlerFactory credentialedProxyHandlerFactory =
+                        HttpProxyHandlerFactory.fromSystemProxies(ApnsClientBuilder.DEVELOPMENT_APNS_HOST);
+
+                assertNotNull(credentialedProxyHandlerFactory);
+                assertEquals(username, credentialedProxyHandlerFactory.getUsername());
+                assertEquals(password, credentialedProxyHandlerFactory.getPassword());
+            }
+        } finally {
+            if (originalUsername == null) {
+                System.clearProperty(USERNAME_PROPERTY_KEY);
+            } else {
+                System.setProperty(USERNAME_PROPERTY_KEY, originalUsername);
+            }
+
+            if (originalPassword == null) {
+                System.clearProperty(PASSWORD_PROPERTY_KEY);
+            } else {
+                System.setProperty(PASSWORD_PROPERTY_KEY, originalPassword);
+            }
+        }
+    }
 }