From 41b5c7d9e930d7f5b53b0ff84c9a6d90e9402676 Mon Sep 17 00:00:00 2001 From: Drew Hamilton Date: Tue, 1 Jun 2021 16:58:43 +0200 Subject: [PATCH 1/5] Add fallback to JVM system proxy values if plugin proxy properties are not set --- .../plugin/artifactory/task/DeployTask.java | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java index 8463368f8..6b67a9bf4 100644 --- a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java +++ b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java @@ -178,7 +178,7 @@ private void deployArtifacts(ArtifactoryClientConfiguration accRoot, Map Date: Tue, 1 Jun 2021 17:02:02 +0200 Subject: [PATCH 2/5] Fix `isHttps` logic --- .../org/jfrog/gradle/plugin/artifactory/task/DeployTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java index 6b67a9bf4..944ffeb81 100644 --- a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java +++ b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java @@ -206,7 +206,7 @@ private void configureProxy(String contextUrl, ArtifactoryClientConfiguration cl String proxyHost = proxy.getHost(); Integer proxyPort = proxy.getPort(); - boolean isHttps = contextUrl.startsWith("http") && !contextUrl.startsWith("https"); + boolean isHttps = !contextUrl.startsWith("http://"); // If no proxyHost is explicitly set, check for the JVM system proxyHost property: if (StringUtils.isBlank(proxyHost)) { From 24b92bf0836dd04c67d01be9b429107697bf76f2 Mon Sep 17 00:00:00 2001 From: Drew Hamilton Date: Wed, 2 Jun 2021 11:05:43 +0200 Subject: [PATCH 3/5] Implement handling for http.nonProxyHosts property --- .../plugin/artifactory/task/DeployTask.java | 65 +++++++++++++++++-- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java index 944ffeb81..10d9c215e 100644 --- a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java +++ b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java @@ -210,15 +210,30 @@ private void configureProxy(String contextUrl, ArtifactoryClientConfiguration cl // If no proxyHost is explicitly set, check for the JVM system proxyHost property: if (StringUtils.isBlank(proxyHost)) { - String systemPropertyName = isHttps ? "https.proxyHost" : "http.proxyHost"; - proxyHost = System.getProperty(systemPropertyName); + // Note: "http.nonProxyHosts" is used for both http and https, despite its prefix + String systemNonProxyHostsString = System.getProperty("http.nonProxyHosts"); + String[] systemNonProxyHosts = StringUtils.split(systemNonProxyHostsString, '|'); + + String contextUrlHost = getHost(contextUrl); + boolean isContextUrlOnNonProxyList = false; + for (String nonProxyHost : systemNonProxyHosts) { + if (nonProxyHost.contains("*") && matchesWildcardPattern(contextUrlHost, nonProxyHost)) { + isContextUrlOnNonProxyList = true; + } + } + + if (!isContextUrlOnNonProxyList) { + String systemPropertyName = isHttps ? "https.proxyHost" : "http.proxyHost"; + proxyHost = System.getProperty(systemPropertyName); + } } + // If no proxyPort is explicitly set, check for the JVM system proxyPort property: if (proxyPort == null) { String systemPropertyName = isHttps ? "https.proxyPort" : "http.proxyPort"; - String systemProxyPortValue = System.getProperty(systemPropertyName); - if (StringUtils.isNotBlank(systemProxyPortValue)) { - proxyPort = Integer.valueOf(systemProxyPortValue); + String systemProxyPort = System.getProperty(systemPropertyName); + if (StringUtils.isNotBlank(systemProxyPort)) { + proxyPort = Integer.valueOf(systemProxyPort); } } @@ -235,6 +250,46 @@ private void configureProxy(String contextUrl, ArtifactoryClientConfiguration cl } } + private static String getHost(String url) { + String afterProtocol = url.contains("://") ? StringUtils.substringAfter(url, "://") : url; + return StringUtils.substringBefore(afterProtocol, "/"); + } + + /** + * Checks whether a string matches a pattern with * wildcards. * is the only supported wildcard character. + * + * @param string The string to check. + * @param pattern The wildcard pattern to match with the string. + * @return True if the string matches the wildcard pattern, or if the pattern contains no wildcards and is equal to + * the string. False otherwise. + */ + private static boolean matchesWildcardPattern(@Nonnull String string, @Nonnull String pattern) { + if (!pattern.contains("*")) { + // Shortcut wildcard matching if the pattern has no wildcard: + return string.equals(pattern); + } + + String[] patternParts = StringUtils.split(pattern, "*"); + + if (!pattern.startsWith("*") && !string.startsWith(patternParts[0])) { + // Shortcut loop if string doesn't start with an exact match: + return false; + } else if (!pattern.endsWith("*") && !string.endsWith(patternParts[patternParts.length - 1])) { + // Shortcut loop if string doesn't end with an exact match: + return false; + } else { + String remainingString = string; + for (String patternPart : patternParts) { + if (remainingString.contains(patternPart)) { + remainingString = StringUtils.substringAfter(remainingString, patternPart); + } else { + return false; + } + } + return true; + } + } + private void configConnectionTimeout(ArtifactoryClientConfiguration clientConf, ArtifactoryManager artifactoryManager) { if (clientConf.getTimeout() != null) { artifactoryManager.setConnectionTimeout(clientConf.getTimeout()); From 76b3c4a8f61e972b1eb1f3fa7f9a76c88af18ded Mon Sep 17 00:00:00 2001 From: Drew Hamilton Date: Wed, 2 Jun 2021 14:11:25 +0200 Subject: [PATCH 4/5] Implement DeployTaskProxyTest --- .../plugin/artifactory/task/DeployTask.java | 28 +- .../artifactory/task/DeployTaskProxyTest.java | 242 ++++++++++++++++++ 2 files changed, 255 insertions(+), 15 deletions(-) create mode 100644 build-info-extractor-gradle/src/test/java/org/jfrog/gradle/plugin/artifactory/task/DeployTaskProxyTest.java diff --git a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java index 10d9c215e..24e338251 100644 --- a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java +++ b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java @@ -1,5 +1,6 @@ package org.jfrog.gradle.plugin.artifactory.task; +import com.google.common.annotations.VisibleForTesting; import org.apache.commons.lang.StringUtils; import org.gradle.api.DefaultTask; import org.gradle.api.Project; @@ -30,6 +31,7 @@ import java.io.IOException; import java.util.*; import java.util.concurrent.*; +import java.util.stream.Stream; /** * @author Ruben Perez @@ -201,7 +203,8 @@ private void deployArtifacts(ArtifactoryClientConfiguration accRoot, Map it.startsWith(".") ? it.substring(1) : it) + .toArray(String[]::new); if (!pattern.startsWith("*") && !string.startsWith(patternParts[0])) { // Shortcut loop if string doesn't start with an exact match: return false; - } else if (!pattern.endsWith("*") && !string.endsWith(patternParts[patternParts.length - 1])) { - // Shortcut loop if string doesn't end with an exact match: - return false; } else { String remainingString = string; for (String patternPart : patternParts) { diff --git a/build-info-extractor-gradle/src/test/java/org/jfrog/gradle/plugin/artifactory/task/DeployTaskProxyTest.java b/build-info-extractor-gradle/src/test/java/org/jfrog/gradle/plugin/artifactory/task/DeployTaskProxyTest.java new file mode 100644 index 000000000..0ca99de4f --- /dev/null +++ b/build-info-extractor-gradle/src/test/java/org/jfrog/gradle/plugin/artifactory/task/DeployTaskProxyTest.java @@ -0,0 +1,242 @@ +package org.jfrog.gradle.plugin.artifactory.task; + +import org.jfrog.build.api.util.Log; +import org.jfrog.build.api.util.NullLog; +import org.jfrog.build.client.ProxyConfiguration; +import org.jfrog.build.extractor.clientConfiguration.ArtifactoryClientConfiguration; +import org.jfrog.build.extractor.clientConfiguration.client.artifactory.ArtifactoryManager; +import org.testng.annotations.*; + +import javax.annotation.Nullable; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; + +@Test +public class DeployTaskProxyTest { + + private static final String HTTPS_PROXY_HOST_SETTING = "https.proxyHost"; + private static final String HTTPS_PROXY_PORT_SETTING = "https.proxyPort"; + private static final String HTTP_PROXY_HOST_SETTING = "http.proxyHost"; + private static final String HTTP_PROXY_PORT_SETTING = "http.proxyPort"; + private static final String HTTP_NON_PROXY_HOSTS_SETTING = "http.nonProxyHosts"; + + private final Log log = new NullLog(); + + private String systemHttpsProxyHost = null; + private String systemHttpsProxyPort = null; + private String systemHttpProxyHost = null; + private String systemHttpProxyPort = null; + private String systemHttpNonProxyHosts = null; + + @BeforeMethod + public void copySystemProperties() { + systemHttpsProxyHost = System.getProperty(HTTPS_PROXY_HOST_SETTING); + systemHttpsProxyPort = System.getProperty(HTTPS_PROXY_PORT_SETTING); + systemHttpProxyHost = System.getProperty(HTTP_PROXY_HOST_SETTING); + systemHttpProxyPort = System.getProperty(HTTP_PROXY_PORT_SETTING); + systemHttpNonProxyHosts = System.getProperty(HTTP_NON_PROXY_HOSTS_SETTING); + } + + @AfterMethod + public void restoreSystemProperties() { + setOrClearSystemProperty(HTTPS_PROXY_HOST_SETTING, systemHttpsProxyHost); + setOrClearSystemProperty(HTTPS_PROXY_PORT_SETTING, systemHttpsProxyPort); + setOrClearSystemProperty(HTTP_PROXY_HOST_SETTING, systemHttpProxyHost); + setOrClearSystemProperty(HTTP_PROXY_PORT_SETTING, systemHttpProxyPort); + setOrClearSystemProperty(HTTP_NON_PROXY_HOSTS_SETTING, systemHttpNonProxyHosts); + } + + private static void setOrClearSystemProperty(String key, @Nullable String value) { + if (value == null) + System.clearProperty(key); + else + System.setProperty(key, value); + } + + @Test + public void givenExplicitProxyHostAndPort_usesExplicitProxyHostAndPort() { + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + clientConfig.proxy.setHost("proxy-host"); + clientConfig.proxy.setPort(9999); + + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("", clientConfig, artifactoryManager); + + ProxyConfiguration result = artifactoryManager.getProxyConfiguration(); + assertEquals(result.host, "proxy-host"); + assertEquals(result.port, 9999); + } + + @Test + public void givenExplicitProxyHostButNoPort_doesNotUseProxy() { + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + clientConfig.proxy.setHost("proxy-host"); + + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("", clientConfig, artifactoryManager); + + assertNull(artifactoryManager.getProxyConfiguration()); + } + + @Test + public void givenExplicitProxyPortButNoHost_doesNotUseProxy() { + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + clientConfig.proxy.setPort(9999); + + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("", clientConfig, artifactoryManager); + + assertNull(artifactoryManager.getProxyConfiguration()); + } + + @Test + public void givenExplicitAndSystemProxyHostAndPort_usesExplicitProxyHostAndPort() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "wrong-proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(4444)); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + clientConfig.proxy.setHost("proxy-host"); + clientConfig.proxy.setPort(9999); + + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("", clientConfig, artifactoryManager); + + ProxyConfiguration result = artifactoryManager.getProxyConfiguration(); + assertEquals(result.host, "proxy-host"); + assertEquals(result.port, 9999); + } + + @Test + public void givenSystemProxyHostAndPort_usesSystemProxyHostAndPort() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(9999)); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("", clientConfig, artifactoryManager); + + ProxyConfiguration result = artifactoryManager.getProxyConfiguration(); + assertEquals(result.host, "proxy-host"); + assertEquals(result.port, 9999); + } + + @Test + public void givenSystemProxyHostAndPort_whenContextUrlIsHttp_usesSystemHttpProxyHostAndPort() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "https-proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(1111)); + System.setProperty(HTTP_PROXY_HOST_SETTING, "http-proxy-host"); + System.setProperty(HTTP_PROXY_PORT_SETTING, String.valueOf(2222)); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("http://example.com/artifacts", clientConfig, artifactoryManager); + + ProxyConfiguration result = artifactoryManager.getProxyConfiguration(); + assertEquals(result.host, "http-proxy-host"); + assertEquals(result.port, 2222); + } + + @Test + public void givenSystemProxyHostAndPort_whenContextUrlIsHttps_usesSystemHttpsProxyHostAndPort() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "https-proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(1111)); + System.setProperty(HTTP_PROXY_HOST_SETTING, "http-proxy-host"); + System.setProperty(HTTP_PROXY_PORT_SETTING, String.valueOf(2222)); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("https://example.com/artifacts", clientConfig, artifactoryManager); + + ProxyConfiguration result = artifactoryManager.getProxyConfiguration(); + assertEquals(result.host, "https-proxy-host"); + assertEquals(result.port, 1111); + } + + @Test + public void givenSystemProxySettings_whenContextUrlMatchesNonProxyHostsExactly_doesNotUseProxy() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "https-proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(1111)); + System.setProperty(HTTP_PROXY_HOST_SETTING, "http-proxy-host"); + System.setProperty(HTTP_PROXY_PORT_SETTING, String.valueOf(2222)); + System.setProperty(HTTP_NON_PROXY_HOSTS_SETTING, "example.com|other"); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("https://example.com/artifacts", clientConfig, artifactoryManager); + + assertNull(artifactoryManager.getProxyConfiguration()); + } + + @Test + public void givenSystemProxySettings_whenContextUrlMatchesNonProxyHostsWithDot_doesNotUseProxy() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "https-proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(1111)); + System.setProperty(HTTP_PROXY_HOST_SETTING, "http-proxy-host"); + System.setProperty(HTTP_PROXY_PORT_SETTING, String.valueOf(2222)); + System.setProperty(HTTP_NON_PROXY_HOSTS_SETTING, ".example.com|other"); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("https://example.com/artifacts", clientConfig, artifactoryManager); + + assertNull(artifactoryManager.getProxyConfiguration()); + } + + @Test + public void givenSystemProxySettings_whenContextUrlMatchesNonProxyHostsByLeadingWildcard_doesNotUseProxy() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "https-proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(1111)); + System.setProperty(HTTP_PROXY_HOST_SETTING, "http-proxy-host"); + System.setProperty(HTTP_PROXY_PORT_SETTING, String.valueOf(2222)); + System.setProperty(HTTP_NON_PROXY_HOSTS_SETTING, "other|*.example.com"); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("https://example.com/artifacts", clientConfig, artifactoryManager); + + assertNull(artifactoryManager.getProxyConfiguration()); + } + + @Test + public void givenSystemProxySettings_whenContextUrlMatchesNonProxyHostsByCentralWildcard_doesNotUseProxy() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "https-proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(1111)); + System.setProperty(HTTP_PROXY_HOST_SETTING, "http-proxy-host"); + System.setProperty(HTTP_PROXY_PORT_SETTING, String.valueOf(2222)); + System.setProperty(HTTP_NON_PROXY_HOSTS_SETTING, "other|ex*ple.com"); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("https://example.com/artifacts", clientConfig, artifactoryManager); + + assertNull(artifactoryManager.getProxyConfiguration()); + } + + @Test + public void givenSystemProxySettings_whenContextUrlMatchesNonProxyHostsWithSlash_doesNotUseProxy() { + System.setProperty(HTTPS_PROXY_HOST_SETTING, "https-proxy-host"); + System.setProperty(HTTPS_PROXY_PORT_SETTING, String.valueOf(1111)); + System.setProperty(HTTP_PROXY_HOST_SETTING, "http-proxy-host"); + System.setProperty(HTTP_PROXY_PORT_SETTING, String.valueOf(2222)); + System.setProperty(HTTP_NON_PROXY_HOSTS_SETTING, "other|example.com/artifacts"); + + ArtifactoryClientConfiguration clientConfig = new ArtifactoryClientConfiguration(log); + ArtifactoryManager artifactoryManager = new ArtifactoryManager("", log); + + DeployTask.configureProxy("https://example.com/artifacts", clientConfig, artifactoryManager); + + assertNull(artifactoryManager.getProxyConfiguration()); + } +} From aaabcf29cd2c31bc24288bf7a26e03fd5678a91b Mon Sep 17 00:00:00 2001 From: Drew Hamilton Date: Wed, 2 Jun 2021 14:12:33 +0200 Subject: [PATCH 5/5] Remove unused import --- .../org/jfrog/gradle/plugin/artifactory/task/DeployTask.java | 1 - 1 file changed, 1 deletion(-) diff --git a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java index 24e338251..e1970670d 100644 --- a/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java +++ b/build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/task/DeployTask.java @@ -31,7 +31,6 @@ import java.io.IOException; import java.util.*; import java.util.concurrent.*; -import java.util.stream.Stream; /** * @author Ruben Perez