Skip to content
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

feat: add intrinsic support for SOCKS proxies #1966

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

DirtyHairy
Copy link

This PR adds support for establishing TCP and TLS connections through a SOCKS proxy. Proxy support is enabled with a new socks configuration parameter that takes a curl-style SOCKS URL, or by setting an environment variable (MQTTJS_SOCKS_PROXY).

Supported schemes (and protocols) are socks5:// , socks5h:// , socks4:// and socks4a:// .

@DirtyHairy DirtyHairy changed the title Add intrinsic support for SOCKS proxies feat: add intrinsic support for SOCKS proxies Feb 5, 2025
Comment on lines 107 to 118
if (isBrowser || opts.unixSocket) {
opts.socks = undefined
}

if (
!isBrowser &&
!opts.unixSocket &&
opts.socks === undefined &&
typeof process !== 'undefined'
) {
opts.socks = process.env['MQTTJS_SOCKS_PROXY']
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? Could you remove the env var here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the environment variable in order to provide an easy way to route unmodified applications that use mqtt.js through a proxy. In or case the culprit is Node-RED.

It would be definitely preferable if these had support for using a proxy themselves, but configuring a proxy is one of those networking options where it is not uncommon to offer configuration through the environment.

src/lib/client.ts Outdated Show resolved Hide resolved
src/lib/client.ts Outdated Show resolved Hide resolved
src/lib/connect/socks.ts Outdated Show resolved Hide resolved
@robertsLando robertsLando requested a review from mcollina February 6, 2025 10:40
Co-authored-by: Daniel Lando <daniel.sorridi@gmail.com>
src/lib/connect/socks.ts Outdated Show resolved Hide resolved
@robertsLando
Copy link
Member

@DirtyHairy Would it be possible to test part of this?

@DirtyHairy
Copy link
Author

Sure, done 😏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants