From 3ec2a917ac425937f8d9c69a4026e68399e816e0 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Fri, 5 Feb 2016 14:51:52 +0000 Subject: JCLOUDS-1075: Clean up logic and docs for setting proxies --- .gitignore | 1 + core/src/main/java/org/jclouds/Constants.java | 36 ++++++++++++- .../main/java/org/jclouds/proxy/ProxyConfig.java | 13 ++++- .../main/java/org/jclouds/proxy/ProxyForURI.java | 26 +++++++--- .../jclouds/proxy/internal/GuiceProxyConfig.java | 16 +++++- .../java/org/jclouds/proxy/ProxyForURITest.java | 59 +++++++++++++++++----- project/pom.xml | 1 + 7 files changed, 125 insertions(+), 27 deletions(-) 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 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 @@ /** * Boolean property. *

- * 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 java.net.useSystemProxies 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. + *

+ * Whether or not jclouds is permitted to use the default proxy detected by the JVM + * and configured there using the usual Java settings: + *

+ *

+ * 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 false, + * then jclouds will not use a proxy irrespective of the java.net.* 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. *

@@ -132,6 +162,7 @@ * String property. *

* 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 @@ * String property. *

* 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"; 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 @@ 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 */ 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 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 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 creds = config.getCredentials(); @@ -84,9 +81,22 @@ public PasswordAuthentication getPasswordAuthentication() { 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 proxies = ProxySelector.getDefault().select(endpoint); + return getLast(proxies); } } 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 @@ @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 @@ @Override public Optional getProxy() { - if (host == null) + if (Strings.isNullOrEmpty(host)) return Optional.absent(); Integer port = this.port; if (port == null) { @@ -107,13 +113,19 @@ public boolean useSystem() { 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(); } } 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 @@ private class MyProxyConfig implements ProxyConfig { private boolean useSystem; + private boolean jvmProxyEnabled; private Type type; private Optional proxy; private Optional credentials; - MyProxyConfig(boolean useSystem, Type type, Optional proxy, Optional credentials) { + MyProxyConfig(boolean useSystem, boolean jvmProxyEnabled, Type type, Optional proxy, Optional credentials) { this.useSystem = useSystem; + this.jvmProxyEnabled = jvmProxyEnabled; this.type = type; this.proxy = proxy; this.credentials = credentials; @@ -71,11 +74,16 @@ public Type getType() { public Optional 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 void testDontUseProxyForSockets() throws Exception { @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 void testUseProxyForSockets() throws Exception { @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 void testUseProxyForSocketsSettingShouldntAffectHTTP() throws Exception { @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)); + } + } 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 @@ **/modernizer_exclusions.txt **/.factorypath **/.apt_generated/** + **/.checkstyle .repository/**