-
Notifications
You must be signed in to change notification settings - Fork 246
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
Use non-compliant URL from 'url' for socks urls #242
Use non-compliant URL from 'url' for socks urls #242
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: d51474a The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
`new URL('socks://host:port')` gives different results in browser and Node.js because `socks` is not among the "special schemes" specified by WHATWG and thus the hostname is treated as an opaque value (without parsing the port). Node.js implementation, however, parses the `port`. Since `proxy-agent` can be used in Electron, unless `URL` is imported directly from the Node's `url` module - it is going to use the Browser's version of it which won't work with socks urls.
3d33494
to
d51474a
Compare
Makes sense. Thank you! |
Yay, thanks for merging this! |
It might be helpful to use a lint rule for this as well to keep it from possibly regressing. I think {
"rules": {
"no-restricted-globals": ["error", { "name": "URL", "message": "Use url.URL instead" }]
}
} |
@lukekarrys would be happy to accept a PR! |
Done 😄! #246 |
new URL('socks://host:port')
gives different results in browser and Node.js becausesocks
is not among the "special schemes" specified by WHATWG and thus the hostname is treated as an opaque value (without parsing the port). Node.js implementation, however, parses theport
.Since
proxy-agent
can be used in Electron, unlessURL
is imported directly from the Node'surl
module - it is going to use the Browser's version of it which won't work with socks urls.