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 <[email protected]> Authored: Fri Feb 5 14:51:52 2016 +0000 Committer: Andrew Phillips <[email protected]> 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>
