-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
base: main
Are you sure you want to change the base?
Conversation
src/lib/connect/index.ts
Outdated
if (isBrowser || opts.unixSocket) { | ||
opts.socks = undefined | ||
} | ||
|
||
if ( | ||
!isBrowser && | ||
!opts.unixSocket && | ||
opts.socks === undefined && | ||
typeof process !== 'undefined' | ||
) { | ||
opts.socks = process.env['MQTTJS_SOCKS_PROXY'] | ||
} |
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.
why? Could you remove the env var here?
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.
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.
@DirtyHairy Would it be possible to test part of this? |
Sure, done 😏 |
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://
andsocks4a://
.