-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Support http proxies #11043
Support http proxies #11043
Conversation
b5c878d
to
35c380d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments on the code after a more thorough read through. I am setting up my proxy settings now to test the functionality...
packages/core/src/electron-browser/request/electron-browser-request-module.ts
Outdated
Show resolved
Hide resolved
packages/preferences/src/browser/util/preference-tree-generator.ts
Outdated
Show resolved
Hide resolved
Functionally, I believe that this is working more or less as advertised. I'm not an expert on proxies or firewalls, but having set both up naïvely, traffic for Electron appears to be running through the configured proxy and making it through the firewall. @paul-marechal, I know you have some experience setting up proxies / throttlers on Linux, so if you'd be willing to give it a test, I'd appreciate it. |
35c380d
to
7782fa3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks mostly good to me, just a few oddities.
packages/core/src/electron-node/request/electron-backend-request-service.ts
Outdated
Show resolved
Hide resolved
bda5dfe
to
e15cbce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good as first iteration. We can take care of the missing bits in Electron later with IPC/RPC.
What it does
Closes #7797
Closes #10267
For some reason GitHub force-closed my old PR while force-pushing and doesn't allow me to reopen it, so I had to create a new PR. See #10958 for previous discussion.
Adds general proxy support to the whole application (backend+frontend) by using a
RequestService
which comes with automatic proxy support. Other services and downstream applications are encouraged to use that service whenever requests to non-origin or non-localhost web-services are performed.Also patches the
http
andhttps
node.js imports for the extension host. This enables extensions to perform requests through proxies as well.For electron there's currently a large TODO: Automatic OS proxy detection doesn't work yet, since that requires access to the electron-main process (see also #10698).
How to test
Generally, testing this PR requires setting up firewall rules which block all outgoing traffic and configuring a local proxy rule. I used Fiddler Classic (Windows only?) which sets itself up at
http://localhost:8888
. I assume Mac and Linux users have a simpler time configuring such a proxy.Also, these tests should preferably performed using the Electron version. The browser version will automatically use the OS proxy for stuff like fetching vsx search results, whereas the electron version will fail if the proxy is not configured.
java
. Nothing should appear if outbound traffic is blocked.proxy
and set theHttp: Proxy
preference to your local proxy url.proxy-test
extension.Hello World Request
command. It will send a request tohttps://www.google.com
and display the error message/success code.Http: Proxy support
preference.override
,fallback
andon
should all provide the same success result, whileoff
should display an error, since the extension proxy support has been turned off.There are now proxy settings for fetching packages from ovsx registries using the
theia download:plugins
command.plugins
folder.theia download:plugins
should result in an error, since the outgoing download requests are blocked--proxy-url <url>
parameter should enable successful download of these extensions.Review checklist
Reminder for reviewers