Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix transport bug #3159

Closed
wants to merge 5 commits into from
Closed

Fix transport bug #3159

wants to merge 5 commits into from

Conversation

witjs
Copy link

@witjs witjs commented Jul 31, 2020

fix a bug

If the environment variable https_proxy’s protocol is HTTP

Then I make a request with URL that starts with HTTPS

The request will use the wrong transport

Transport should be nothing to do with proxy

var isHttpsProxy = isHttpsRequest && (proxy ? isHttps.test(proxy.protocol) : true);

proxy.protocol should always be undefined

Copy link
Collaborator

@chinesedfan chinesedfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR. Please add corresponding tests. And in fact, dist files will updated automatically before each release.

@Niek
Copy link

Niek commented Dec 22, 2020

Unfortunately this MR does not fix the issue. This issue (HTTPS requests over HTTP proxies are always failing) has been present since at least 3 years. There are at least 2 packages out there that fix this:
https://www.npmjs.com/package/axios-https-proxy-fix (works)
https://www.npmjs.com/package/axios-proxy-fix (doesn't work for me)

This is the required change in code: Sitronik@36c1f46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants