-
Notifications
You must be signed in to change notification settings - Fork 238
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
fix nil socks5 proxy case #476
Conversation
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.
If the intent is to suppress socks5 from listening (which happens by default), maybe we should consider reversing the logic and having an empty value by default which skip the socks5 initialization and enable it only with valid "[ip]:port" value. If left empty the OS will listen on arbitrary port on 0.0.0.0
which might be an unwanted behavior
If the |
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.
lgtm - I get your point. My observation was more related to potential usability improvement. Since we allow suppressing socks5, and have a flag to define it's behavior, maybe the following could look more intuitive?
flagSet.DynamicVar(&options.ListenAddrSocks5, "sa", "127.0.0.1:10080", "Listening SOCKS IP and Port address (ip:port)"),
Which will result in:
proxify
=> no socks5 proxyproxify
-sa => socks5 on127.0.0.1:10080
proxyfy
-sa x.x.x.x:yy => socks5 onx.x.x.x:yy
Proposed changes
Closes #475
Checklist