You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
See #1749. If the requested URI is HTTPS, attempt to use an HTTPS proxy, if configured, and fallback to the HTTP proxy if one isn't found. This change should be backwards-compatible except in the case where someone is using a broken HTTPS proxy (in which case I would argue that that isn't node-gyp's problem anyway).
If desired, I can remove the url require and substring the uri to detect if it's HTTPS.
No tests were added because there aren't any existing proxy tests.
I think I'm OK with this but I think it's going to have to be semver-major because it introduces potentially new behaviour in existing environments. Would you mind squashing the commits into one and changing the prefix to lib: please?
I'd like to get some tests in for this. Have a look in test/test-download.js, you'll see that you can interact directly with the download function and even pass it environment variables. So it should be straightfoward to come up with some tests that trigger each of these variables. The trick is going to be confirming that it's connecting via a proxy. There's a test/simple-proxy.js that's currently only used by docker.sh (I'm thinking that needs to be removed since it's not really used). That could be repurposed for this. There's also a self-signed certificate in test/fixtures/ that could be used for https proxy tests.
Let me know if you have trouble with this. I can help out as time allows if you're struggling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
npm install && npm testpassesDescription of change
See #1749. If the requested URI is HTTPS, attempt to use an HTTPS proxy, if configured, and fallback to the HTTP proxy if one isn't found. This change should be backwards-compatible except in the case where someone is using a broken HTTPS proxy (in which case I would argue that that isn't node-gyp's problem anyway).
If desired, I can remove the
urlrequire and substring the uri to detect if it's HTTPS.No tests were added because there aren't any existing proxy tests.