TooTallNate / proxy-agents

Node.js HTTP Proxy Agents Monorepo
https://proxy-agents.n8.io
872 stars 229 forks source link

Use non-compliant URL from 'url' for socks urls #242

Closed indutny-signal closed 9 months ago

indutny-signal commented 10 months ago

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.

vercel[bot] commented 10 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
proxy-agents ✅ Ready (Inspect) Visit Preview Aug 29, 2023 7:14pm
changeset-bot[bot] commented 10 months ago

🦋 Changeset detected

Latest commit: d51474a99bdf0a333404c7a185aa8d7d2746be61

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages | Name | Type | | ----------------- | ----- | | socks-proxy-agent | Patch | | pac-proxy-agent | Patch | | proxy-agent | Patch |

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

TooTallNate commented 9 months ago

Makes sense. Thank you!

indutny-signal commented 9 months ago

Yay, thanks for merging this!

lukekarrys commented 9 months ago

It might be helpful to use a lint rule for this as well to keep it from possibly regressing.

I think no-restricted-globals should do the trick:

{
    "rules": {
        "no-restricted-globals": ["error", { "name": "URL", "message": "Use url.URL instead" }]
    }
}
TooTallNate commented 9 months ago

@lukekarrys would be happy to accept a PR!

lukekarrys commented 9 months ago

would be happy to accept a PR!

Done 😄! #246