You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jclouds.apache.org by an...@apache.org on 2016/02/08 23:50:44 UTC

jclouds git commit: clean up logic and docs for setting proxies [Forced Update!]

Repository: jclouds
Updated Branches:
  refs/heads/merged-pr-914 bc119edc9 -> 3ec2a917a (forced update)


clean up logic and docs for setting proxies


Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo
Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/3ec2a917
Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/3ec2a917
Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/3ec2a917

Branch: refs/heads/merged-pr-914
Commit: 3ec2a917ac425937f8d9c69a4026e68399e816e0
Parents: 6371235
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Fri Feb 5 14:51:52 2016 +0000
Committer: Andrew Phillips <an...@apache.org>
Committed: Mon Feb 8 17:49:53 2016 -0500

----------------------------------------------------------------------
 .gitignore                                      |  1 +
 core/src/main/java/org/jclouds/Constants.java   | 36 +++++++++++-
 .../java/org/jclouds/proxy/ProxyConfig.java     | 13 ++++-
 .../java/org/jclouds/proxy/ProxyForURI.java     | 26 ++++++---
 .../proxy/internal/GuiceProxyConfig.java        | 16 +++++-
 .../java/org/jclouds/proxy/ProxyForURITest.java | 59 +++++++++++++++-----
 project/pom.xml                                 |  1 +
 7 files changed, 125 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jclouds/blob/3ec2a917/.gitignore
----------------------------------------------------------------------
diff --git a/.gitignore b/.gitignore
index 8c20bff..f8836f5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,3 +19,4 @@ atlassian-ide-plugin.xml
 .java-version
 .factorypath
 .apt_generated
+.checkstyle

http://git-wip-us.apache.org/repos/asf/jclouds/blob/3ec2a917/core/src/main/java/org/jclouds/Constants.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/Constants.java b/core/src/main/java/org/jclouds/Constants.java
index 115edf1..846a2d6 100644
--- a/core/src/main/java/org/jclouds/Constants.java
+++ b/core/src/main/java/org/jclouds/Constants.java
@@ -100,10 +100,40 @@ public final class Constants {
    /**
     * Boolean property.
     * <p/>
-    * Whether or not to use the proxy setup from the underlying operating system.
+    * Whether or not to attempt to use the proxy setup from the underlying operating system.
+    * Defaults to false.
+    * Only considered if {@link #PROPERTY_PROXY_ENABLE_JVM_PROXY} is false
+    * and {@link #PROPERTY_PROXY_HOST} is not supplied.
+    * Due to how Java's <code>java.net.useSystemProxies</code> is handled,
+    * this may have limited effectiveness.
+    * @deprecated in 2.0.0, replaced by {@link #PROPERTY_PROXY_ENABLE_JVM_PROXY} does what this intended but better
     */
+   @Deprecated
+   // deprecated because:  the impl attempts to set the corresponding JVM system property
+   // but that is documented to have no effect if set after system startup;
+   // see e.g. https://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html
    public static final String PROPERTY_PROXY_SYSTEM = "jclouds.use-system-proxy";
-   
+
+   /**
+    * Boolean property.
+    * <p/>
+    * Whether or not jclouds is permitted to use the default proxy detected by the JVM
+    * and configured there using the usual Java settings:
+    * <ul>
+    * <li> <code>java.net.useSystemProxies</code>
+    * <li> <code>java.net.httpProxyHost</code>
+    * <li> <code>java.net.httpProxyPort</code>
+    * </ul>
+    * <p/>
+    * Defaults to true so that the Java standard way of setting proxies can be used.
+    * However if {@link #PROPERTY_PROXY_HOST} is set that will always take priority
+    * when jclouds looks for a proxy.
+    * If this property is explicitly set <code>false</code>,
+    * then jclouds will not use a proxy irrespective of the <code>java.net.*</code> settings,
+    * unless {@link #PROPERTY_PROXY_HOST} is set or {@link #PROPERTY_PROXY_SYSTEM} is true.
+    */
+   public static final String PROPERTY_PROXY_ENABLE_JVM_PROXY = "jclouds.enable-jvm-proxy";
+
    /**
     * String property.
     * <p/>
@@ -132,6 +162,7 @@ public final class Constants {
     * String property.
     * <p/>
     * Explicitly sets the user name credential for proxy authentication.
+    * This only applies when {@link #PROPERTY_PROXY_HOST} is supplied.
     */
    public static final String PROPERTY_PROXY_USER = "jclouds.proxy-user";
    
@@ -139,6 +170,7 @@ public final class Constants {
     * String property.
     * <p/>
     * Explicitly sets the password credential for proxy authentication.
+    * This only applies when {@link #PROPERTY_PROXY_HOST} is supplied.
     */
    public static final String PROPERTY_PROXY_PASSWORD = "jclouds.proxy-password";
 

http://git-wip-us.apache.org/repos/asf/jclouds/blob/3ec2a917/core/src/main/java/org/jclouds/proxy/ProxyConfig.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/proxy/ProxyConfig.java b/core/src/main/java/org/jclouds/proxy/ProxyConfig.java
index 05f4923..00fde7f 100644
--- a/core/src/main/java/org/jclouds/proxy/ProxyConfig.java
+++ b/core/src/main/java/org/jclouds/proxy/ProxyConfig.java
@@ -33,16 +33,25 @@ import com.google.inject.ImplementedBy;
 public interface ProxyConfig {
 
    /**
+    * @deprecated
     * @see org.jclouds.Constants#PROPERTY_PROXY_SYSTEM
     */
+   @Deprecated
    boolean useSystem();
-   
+
+   /**
+    * @see org.jclouds.Constants#PROPERTY_PROXY_FROM_JVM
+    */
+   boolean isJvmProxyEnabled();
+
    /**
     * @see org.jclouds.Constants#PROPERTY_PROXY_TYPE
     */
    Type getType();
-   
+
    /**
+    * Returns the host and port of the proxy configured here, if there is one
+    *
     * @see org.jclouds.Constants#PROPERTY_PROXY_HOST
     * @see org.jclouds.Constants#PROPERTY_PROXY_PORT
     */

http://git-wip-us.apache.org/repos/asf/jclouds/blob/3ec2a917/core/src/main/java/org/jclouds/proxy/ProxyForURI.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/proxy/ProxyForURI.java b/core/src/main/java/org/jclouds/proxy/ProxyForURI.java
index a84c605..eb1645f 100644
--- a/core/src/main/java/org/jclouds/proxy/ProxyForURI.java
+++ b/core/src/main/java/org/jclouds/proxy/ProxyForURI.java
@@ -65,13 +65,10 @@ public class ProxyForURI implements Function<URI, Proxy> {
    public Proxy apply(URI endpoint) {
       if (!useProxyForSockets && "socket".equals(endpoint.getScheme())) {
          return Proxy.NO_PROXY;
-      } else if (config.useSystem()) {
-         System.setProperty("java.net.useSystemProxies", "true");
-         Iterable<Proxy> proxies = ProxySelector.getDefault().select(endpoint);
-         return getLast(proxies);
-      } else if (config.getProxy().isPresent()) {
+      }
+      if (config.getProxy().isPresent()) {
          SocketAddress addr = new InetSocketAddress(config.getProxy().get().getHostText(), config.getProxy().get()
-               .getPort());
+            .getPort());
          Proxy proxy = new Proxy(config.getType(), addr);
 
          final Optional<Credentials> creds = config.getCredentials();
@@ -84,9 +81,22 @@ public class ProxyForURI implements Function<URI, Proxy> {
             Authenticator.setDefault(authenticator);
          }
          return proxy;
-      } else {
-         return Proxy.NO_PROXY;
       }
+      if (config.isJvmProxyEnabled()) {
+         return getDefaultProxy(endpoint);
+      }
+      if (config.useSystem()) {
+         // see notes on the Constant which initialized the above for deprecation;
+         // in short the following applied after startup is documented to have no effect.
+         System.setProperty("java.net.useSystemProxies", "true");
+         return getDefaultProxy(endpoint);
+      }
+      return Proxy.NO_PROXY;
+   }
+
+   private Proxy getDefaultProxy(URI endpoint) {
+      Iterable<Proxy> proxies = ProxySelector.getDefault().select(endpoint);
+      return getLast(proxies);
    }
 
 }

http://git-wip-us.apache.org/repos/asf/jclouds/blob/3ec2a917/core/src/main/java/org/jclouds/proxy/internal/GuiceProxyConfig.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/proxy/internal/GuiceProxyConfig.java b/core/src/main/java/org/jclouds/proxy/internal/GuiceProxyConfig.java
index c3406b4..609a850 100644
--- a/core/src/main/java/org/jclouds/proxy/internal/GuiceProxyConfig.java
+++ b/core/src/main/java/org/jclouds/proxy/internal/GuiceProxyConfig.java
@@ -17,6 +17,7 @@
 package org.jclouds.proxy.internal;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static org.jclouds.Constants.PROPERTY_PROXY_ENABLE_JVM_PROXY;
 import static org.jclouds.Constants.PROPERTY_PROXY_HOST;
 import static org.jclouds.Constants.PROPERTY_PROXY_PASSWORD;
 import static org.jclouds.Constants.PROPERTY_PROXY_PORT;
@@ -45,10 +46,15 @@ import com.google.inject.Inject;
 @Singleton
 public class GuiceProxyConfig implements ProxyConfig {
 
+   @SuppressWarnings("deprecation")
    @Inject(optional = true)
    @Named(PROPERTY_PROXY_SYSTEM)
+   @Deprecated
    private boolean systemProxies = Boolean.parseBoolean(System.getProperty("java.net.useSystemProxies", "false"));
    @Inject(optional = true)
+   @Named(PROPERTY_PROXY_ENABLE_JVM_PROXY)
+   private boolean jvmProxyEnabled = true;
+   @Inject(optional = true)
    @Named(PROPERTY_PROXY_HOST)
    private String host;
    @Inject(optional = true)
@@ -66,7 +72,7 @@ public class GuiceProxyConfig implements ProxyConfig {
 
    @Override
    public Optional<HostAndPort> getProxy() {
-      if (host == null)
+      if (Strings.isNullOrEmpty(host))
          return Optional.absent();
       Integer port = this.port;
       if (port == null) {
@@ -107,13 +113,19 @@ public class GuiceProxyConfig implements ProxyConfig {
       return systemProxies;
    }
 
+   @Override
+   public boolean isJvmProxyEnabled() {
+      return jvmProxyEnabled;
+   }
+
    /**
     * {@inheritDoc}
     */
    @Override
    public String toString() {
       return Objects.toStringHelper(this).omitNullValues().add("systemProxies", systemProxies ? "true" : null)
-            .add("proxy", getProxy().orNull()).add("user", user).add("type", host != null ? type : null).toString();
+            .add("jvmProxyEnabled", jvmProxyEnabled ? "true" : "false")
+            .add("proxyHostPort", getProxy().orNull()).add("user", user).add("type", host != null ? type : null).toString();
    }
 
 }

http://git-wip-us.apache.org/repos/asf/jclouds/blob/3ec2a917/core/src/test/java/org/jclouds/proxy/ProxyForURITest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/jclouds/proxy/ProxyForURITest.java b/core/src/test/java/org/jclouds/proxy/ProxyForURITest.java
index d0ac45b..261f24b 100644
--- a/core/src/test/java/org/jclouds/proxy/ProxyForURITest.java
+++ b/core/src/test/java/org/jclouds/proxy/ProxyForURITest.java
@@ -17,6 +17,7 @@
 package org.jclouds.proxy;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotEquals;
 import static org.testng.Assert.assertNotNull;
 
 import java.lang.reflect.Field;
@@ -41,12 +42,14 @@ public class ProxyForURITest {
 
    private class MyProxyConfig implements ProxyConfig {
       private boolean useSystem;
+      private boolean jvmProxyEnabled;
       private Type type;
       private Optional<HostAndPort> proxy;
       private Optional<Credentials> credentials;
 
-      MyProxyConfig(boolean useSystem, Type type, Optional<HostAndPort> proxy, Optional<Credentials> credentials) {
+      MyProxyConfig(boolean useSystem, boolean jvmProxyEnabled, Type type, Optional<HostAndPort> proxy, Optional<Credentials> credentials) {
          this.useSystem = useSystem;
+         this.jvmProxyEnabled = jvmProxyEnabled;
          this.type = type;
          this.proxy = proxy;
          this.credentials = credentials;
@@ -71,11 +74,16 @@ public class ProxyForURITest {
       public Optional<Credentials> getCredentials() {
          return credentials;
       }
+
+      @Override
+      public boolean isJvmProxyEnabled() {
+         return jvmProxyEnabled;
+      }
    }
 
    @Test
    public void testDontUseProxyForSockets() throws Exception {
-      ProxyConfig config = new MyProxyConfig(false, Proxy.Type.HTTP, hostAndPort, creds);
+      ProxyConfig config = new MyProxyConfig(false, false, Proxy.Type.HTTP, hostAndPort, creds);
       ProxyForURI proxy = new ProxyForURI(config);
       Field useProxyForSockets = proxy.getClass().getDeclaredField("useProxyForSockets");
       useProxyForSockets.setAccessible(true);
@@ -86,7 +94,7 @@ public class ProxyForURITest {
 
    @Test
    public void testUseProxyForSockets() throws Exception {
-      ProxyConfig config = new MyProxyConfig(false, Proxy.Type.HTTP, hostAndPort, creds);
+      ProxyConfig config = new MyProxyConfig(false, false, Proxy.Type.HTTP, hostAndPort, creds);
       ProxyForURI proxy = new ProxyForURI(config);
       URI uri = new URI("socket://ssh.example.com:22");
       assertEquals(proxy.apply(uri), new Proxy(Proxy.Type.HTTP, new InetSocketAddress("proxy.example.com", 8080)));
@@ -94,7 +102,7 @@ public class ProxyForURITest {
 
    @Test
    public void testUseProxyForSocketsSettingShouldntAffectHTTP() throws Exception {
-      ProxyConfig config = new MyProxyConfig(false, Proxy.Type.HTTP, hostAndPort, creds);
+      ProxyConfig config = new MyProxyConfig(false, false, Proxy.Type.HTTP, hostAndPort, creds);
       ProxyForURI proxy = new ProxyForURI(config);
       Field useProxyForSockets = proxy.getClass().getDeclaredField("useProxyForSockets");
       useProxyForSockets.setAccessible(true);
@@ -105,47 +113,72 @@ public class ProxyForURITest {
 
    @Test
    public void testHTTPDirect() throws URISyntaxException {
-      ProxyConfig config = new MyProxyConfig(false, Proxy.Type.DIRECT, noHostAndPort, noCreds);
+      ProxyConfig config = new MyProxyConfig(false, false, Proxy.Type.DIRECT, noHostAndPort, noCreds);
       URI uri = new URI("http://example.com/file");
       assertEquals(new ProxyForURI(config).apply(uri), Proxy.NO_PROXY);
    }
 
    @Test
    public void testHTTPSDirect() throws URISyntaxException {
-      ProxyConfig config = new MyProxyConfig(false, Proxy.Type.DIRECT, noHostAndPort, noCreds);
+      ProxyConfig config = new MyProxyConfig(false, false, Proxy.Type.DIRECT, noHostAndPort, noCreds);
       URI uri = new URI("https://example.com/file");
       assertEquals(new ProxyForURI(config).apply(uri), Proxy.NO_PROXY);
    }
 
    @Test
    public void testFTPDirect() throws URISyntaxException {
-      ProxyConfig config = new MyProxyConfig(false, Proxy.Type.DIRECT, noHostAndPort, noCreds);
+      ProxyConfig config = new MyProxyConfig(false, false, Proxy.Type.DIRECT, noHostAndPort, noCreds);
       URI uri = new URI("ftp://ftp.example.com/file");
       assertEquals(new ProxyForURI(config).apply(uri), Proxy.NO_PROXY);
    }
 
    @Test
    public void testSocketDirect() throws URISyntaxException {
-      ProxyConfig config = new MyProxyConfig(false, Proxy.Type.DIRECT, noHostAndPort, noCreds);
+      ProxyConfig config = new MyProxyConfig(false, false, Proxy.Type.DIRECT, noHostAndPort, noCreds);
       URI uri = new URI("socket://ssh.example.com:22");
       assertEquals(new ProxyForURI(config).apply(uri), Proxy.NO_PROXY);
    }
 
    @Test
    public void testHTTPThroughHTTPProxy() throws URISyntaxException {
-      ProxyConfig config = new MyProxyConfig(false, Proxy.Type.HTTP, hostAndPort, creds);
+      ProxyConfig config = new MyProxyConfig(false, false, Proxy.Type.HTTP, hostAndPort, creds);
       URI uri = new URI("http://example.com/file");
       assertEquals(new ProxyForURI(config).apply(uri), new Proxy(Proxy.Type.HTTP, new InetSocketAddress(
             "proxy.example.com", 8080)));
    }
 
    @Test
-   public void testHTTPThroughSystemProxy() throws URISyntaxException {
-      ProxyConfig config = new MyProxyConfig(true, Proxy.Type.DIRECT, noHostAndPort, noCreds);
+   public void testThroughJvmProxy() throws URISyntaxException {
+      ProxyConfig config = new MyProxyConfig(false, true, Proxy.Type.HTTP, noHostAndPort, noCreds);
+      URI uri = new URI("http://example.com/file");
+      // could return a proxy, could return NO_PROXY, depends on the tester's environment
+      assertNotNull(new ProxyForURI(config).apply(uri));
+   }
+
+   @Test
+   public void testThroughSystemProxy() throws URISyntaxException {
+      ProxyConfig config = new MyProxyConfig(true, false, Proxy.Type.HTTP, noHostAndPort, noCreds);
       URI uri = new URI("http://example.com/file");
-      // could return a proxy, could return NO_PROXY, depends on the tester's
-      // environment
+      // could return a proxy, could return NO_PROXY, depends on the tester's environment
       assertNotNull(new ProxyForURI(config).apply(uri));
    }
 
+   @Test
+   public void testJcloudsProxyHostsPreferredOverJvmProxy() throws URISyntaxException {
+      ProxyConfig test = new MyProxyConfig(true, true, Proxy.Type.HTTP, hostAndPort, noCreds);
+      ProxyConfig jclouds = new MyProxyConfig(false, false, Proxy.Type.HTTP, hostAndPort, noCreds);
+      ProxyConfig jvm = new MyProxyConfig(false, true, Proxy.Type.HTTP, noHostAndPort, noCreds);
+      URI uri = new URI("http://example.com/file");
+      assertEquals(new ProxyForURI(test).apply(uri), new ProxyForURI(jclouds).apply(uri));
+      assertNotEquals(new ProxyForURI(test).apply(uri), new ProxyForURI(jvm).apply(uri));
+   }
+
+   @Test
+   public void testJvmProxyAlwaysPreferredOverSystem() throws URISyntaxException {
+      ProxyConfig test = new MyProxyConfig(true, true, Proxy.Type.HTTP, noHostAndPort, noCreds);
+      ProxyConfig jvm = new MyProxyConfig(false, true, Proxy.Type.HTTP, noHostAndPort, noCreds);
+      URI uri = new URI("http://example.com/file");
+      assertEquals(new ProxyForURI(test).apply(uri), new ProxyForURI(jvm).apply(uri));
+   }
+
 }

http://git-wip-us.apache.org/repos/asf/jclouds/blob/3ec2a917/project/pom.xml
----------------------------------------------------------------------
diff --git a/project/pom.xml b/project/pom.xml
index 0459c55..95c2a42 100644
--- a/project/pom.xml
+++ b/project/pom.xml
@@ -518,6 +518,7 @@
             <exclude>**/modernizer_exclusions.txt</exclude>
             <exclude>**/.factorypath</exclude>
             <exclude>**/.apt_generated/**</exclude>
+            <exclude>**/.checkstyle</exclude>
 
             <!-- Temporary files generated on CloudBees slaves -->
             <exclude>.repository/**</exclude>