-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add support for socks5h proxy scheme #123218
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
Conversation
Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
MihaZupan
left a comment
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.
As long as tests are passing this LGTM
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Co-authored-by: Stephen Toub <stoub@microsoft.com>
Description
HttpClient now accepts
socks5h://proxy URIs. Previously, attempts to use this scheme threwNotSupportedExceptionwith message "Only the 'http', 'https', 'socks4', 'socks4a' and 'socks5' schemes are allowed for proxies."The
socks5hscheme is a cURL convention indicating SOCKS5 with DNS resolution handled by the proxy. Our implementation treats it identically tosocks5since we already forward hostnames to the proxy by default.Changes
socks5htoIsSocksScheme()socks5hcase toEstablishSocksTunnelAsync(), routing to SOCKS5 handlersocks5h://prefix in environment variablessocks5hin supported schemes listSocksProxyTestandHttpEnvironmentProxyTestto coversocks5hschemeExample
Type of change
Testing
Unit tests covering proxy scheme validation and environment variable parsing pass with new
socks5htest cases included.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.