Honor system proxy settings #510
Conversation
if plugin proxy properties are not set
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
Thanks for this contribution @drewhamilton! |
|
I have read the CLA Document and I hereby sign the CLA |
|
@eyalbe4 Thanks for taking a look. After thinking about it more, I think this doesn't make sense without also handling the "http.nonProxyHosts" property, so I will add that handling as well. Then I'll add some tests. |
|
I added handling for the "http.nonProxyHosts" property. It felt a bit silly to write so much string-parsing logic for the wildcard matching; let me know if you're aware of a better way. The |
|
I'll have a look at the code soon @drewhamilton. |
|
Anything else I can do for this? |
|
@drewhamilton - |
|
@eyalbe4 I don't personally need this anymore, though as far as I know it's still a valid approach. @paristote Would this be helpful for your CI/Gradle setup? |
|
Thanks for coming back to this PR, @eyalbe4 , @drewhamilton. Yes this is still useful for us, without it we need extra configuration to set the proxy settings for this particular plugin. |
| 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)); |
There was a problem hiding this comment.
The above line are duplicated many times in the code. I suggest we move them into a method and reuse it.
| if (!pattern.startsWith("*") && !string.startsWith(patternParts[0])) { | ||
| // Shortcut loop if string doesn't start with an exact match: | ||
| return false; | ||
| } else { |
There was a problem hiding this comment.
This "else" clause is redundant, because of the above "return".
|
I'm not using Artifactory anymore, but feel free to make whatever refactors you like, @eyalbe4 and @paristote 🙂 |
This partially addresses #149. If no
proxyHostis specified by the Gradle plugin consumer, the Gradle plugin tries using the JVM "http(s).proxyHost" property. Similarly, if noproxyPortis specified by the Gradle plugin consumer, it tries using the JVM "http(s).proxyPort" property.Whether the "http" or "https" property is used depends on the
contextUrl: "http" if thecontextUrlstarts with "http://", "https" otherwise.This does not address the part of #149 that mentions non-proxy-hosts. This may be a blocker for this PR, since now this plugin could try using a proxy when
contextUrlis actually part of non-proxy-hosts. Please let me know if you think non-proxy-hosts handling must be part of this PR.I didn't write tests yet since I'd like confirmation about whether the above behavior choices are correct. Once these are confirmed I'm happy to add tests.