-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
Contribution to fix multiple issues with proxy support #4318
Description
Describe the issue
This is not exactly a support question, but your PR bot is extremely aggressive and does not allow submitting a blank report, even one created from the New Issue flow.) On with the issue... 🤞
Hi! 👋
Firstly, thanks for your work on this project! 🙂
Today I used patch-package to patch [email protected] for the project I'm working on.
At first I tried using some of the suggestions here like using https-proxy-agent, but it is severely limited in that it does not normal agent options (I hear it's a regression, and the lib is more or less abandoned). So I went for a deep-dive and I managed to isolate a certain number of flaws. I fixed them using patch-package but I'm willing to contribute them back and open separate PRs for each of these issues (along with tests), when time permits. Would you welcome this?
Issues I believe my changes can fix:
- HTTPS over HTTP Proxy Fails #3384 (this is a one-liner, axios doesn't actually allow HTTPS over HTTP due to a simple logic mixup)
- Requests to https through proxy #3903 (axios doesn't set the proxy protocol in
setProxy, which is a problem when the proxy protocol and upstream protocol don't match) - Axios Proxy Issues - 501 and SSL errors #3459 (might also be related to the same cause as Axios Proxy Issues - 501 and SSL errors #3459 or HTTPS over HTTP Proxy Fails #3384)
- Proxies are not working #4165 (likely a mix of the above, but also possibly related to relative redirects not properly supported due to a bug in follow-redirects
Other changes that I made in my patch:
- Enable re-evaluation of
http_proxy,https_proxyandno_proxy(when not passing an explicitproxyconfig option) when following redirects (http_proxyandhttps_proxycould be configured differently)
The patch is fairly large and would probably be better conveyed as multiple PRs, but I'll share it below. (We have 160 tests in our project confirming the patch solves a number of issues, but I can't share them here; I would contribute with axios test coverage instead.)
Example Code
N/A
Expected behavior, if applicable
N/A
Environment
- Axios Version [0.24.0]
- Adapter [HTTP]
- Browser [(N/A)]
- Browser Version [(N/A)]
- Node.js Version [12.20.0]
- OS: [Windows 10, macOS Catalina]
- Additional Library Versions [[email protected]]
Additional context/Screenshots
N/A