-
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 #10958
Support http proxies #10958
Conversation
6374722
to
68bb515
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.
Just a quick glance through the code so far. I'll take a closer look and test it after the release.
} | ||
|
||
override async resolveProxy(url: string): Promise<string | undefined> { | ||
// TODO: Implement IPC to the backend to access the Electron proxy resolver |
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.
Will this be possible once #10698 (or something like it) is in place?
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.
Yes, that's what #10698 was initially meant for.
packages/preferences/src/browser/abstract-resource-preference-provider.ts
Outdated
Show resolved
Hide resolved
68bb515
to
6d8d109
Compare
8db8e4b
to
366d5ed
Compare
366d5ed
to
8258070
Compare
What it does
Closes #7797
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