Repository navigation
Conversation
socks5h: is the conventional scheme for a SOCKS5 proxy that resolves target host names (curl, socks-proxy-agent). It is common in HTTP_PROXY/HTTPS_PROXY/ALL_PROXY, but ProxyAgent, EnvHttpProxyAgent and Socks5ProxyAgent rejected it. Undici never resolves target host names locally: they are always sent to the proxy as domain names (ATYP 0x03). So socks5: and socks: already have socks5h: semantics, and socks5h: can be accepted as an alias with no behavior difference. Refs nodejs#6082.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This relates to...
Refs #6082. Optional follow-up to #6083 (independent of it, can be merged on its own or not at all).
Rationale
socks5h://is the conventional scheme for a SOCKS5 proxy that resolves target host names (curl,socks-proxy-agent, most proxy tooling). Users commonly have it inHTTP_PROXY/HTTPS_PROXY/ALL_PROXY, andProxyAgent/EnvHttpProxyAgent/Socks5ProxyAgentcurrently reject it.Undici already implements
socks5hsemantics for every SOCKS URL: target host names are never resolved locally.parseAddressinlib/core/socks5-utils.jssends IP literals as ATYP 0x01/0x04 and everything else as ATYP 0x03 (domain name), so the proxy does the DNS lookup.socks5:andsocks:therefore already behave likesocks5h:, and acceptingsocks5h:as an alias changes nothing on the wire.Changes
Features
socks5h:proxy URLs inProxyAgent,EnvHttpProxyAgent(viaProxyAgent) andSocks5ProxyAgent.isSocks5Protocol()helper inlib/core/socks5-utils.js, with a comment explaining that all three schemes behave likesocks5h:.ProxyAgentandSocks5ProxyAgentlistsocks5h:and state that host names are always resolved by the proxy.Socks5ProxyAgentconstructor acceptssocks5h://; end-to-end requests throughSocks5ProxyAgent,ProxyAgentandEnvHttpProxyAgentwith asocks5h://proxy, asserting the proxy receives the target as a domain name (localhost).Bug Fixes
N/A
Breaking Changes and Deprecations
None.
If #6083 lands first,
socks5h:also needs adding to itssupportedProxyProtocolslist; I will rebase this PR accordingly.Status