Skip to content

fix nil socks5 proxy case #476

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.

Sign up for GitHub

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

Jump to bottom
Merged
ehsandeep merged 3 commits into from
Feb 8, 2024
Merged

fix nil socks5 proxy case #476

ehsandeep merged 3 commits into from
Feb 8, 2024

Conversation

@dogancanbakir
Copy link
Member

@dogancanbakir dogancanbakir commented Jan 25, 2024

Proposed changes

Closes #475

$ go run . -sa ""   

                       _ ___    
   ___  _______ __ __ (_) _/_ __
  / _ \/ __/ _ \\ \ // / _/ // /
 / .__/_/  \___/_\_\/_/_/ \_, / 
/_/                      /___/

                projectdiscovery.io

[INF] Current proxify version v0.0.12 (latest)
[INF] HTTP Proxy Listening on 127.0.0.1:8888
[INF] Saving proxify logs to proxify_logs.jsonl
- Ctrl+C pressed in Terminal

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@dogancanbakir dogancanbakir self-assigned this Jan 25, 2024
@dogancanbakir dogancanbakir linked an issue Jan 25, 2024 that may be closed by this pull request
httpProxy exits when socks5proxy is set to nil #475
Closed
Mzack9999
Mzack9999 requested changes Jan 31, 2024
View reviewed changes
Copy link
Member

@Mzack9999 Mzack9999 left a 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

@dogancanbakir
Copy link
Member Author

dogancanbakir commented Feb 1, 2024 •
edited
Loading

If the -sa flag was set to an empty string (""), the program would exit because of this line return p.socks5proxy.ListenAndServe("tcp", p.options.ListenAddrSocks5) would no longer wait/listen to anything. However, we have modified the program to wait for all goroutines to finish. So, the intent is not to suppress socks5; instead, keep the program running even if socks5 is not specified.

Mzack9999
Mzack9999 approved these changes Feb 1, 2024
View reviewed changes
Copy link
Member

@Mzack9999 Mzack9999 left a 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 proxy
  • proxify -sa => socks5 on 127.0.0.1:10080
  • proxyfy -sa x.x.x.x:yy => socks5 on x.x.x.x:yy

@ehsandeep ehsandeep merged commit f695365 into dev Feb 8, 2024
@ehsandeep ehsandeep deleted the fix_nil_socks5_case branch February 8, 2024 08:12
@chenrui333 chenrui333 mentioned this pull request Feb 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ehsandeep ehsandeep ehsandeep approved these changes

@Mzack9999 Mzack9999 Mzack9999 approved these changes

Assignees

@dogancanbakir dogancanbakir

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

httpProxy exits when socks5proxy is set to nil

4 participants

@dogancanbakir @ehsandeep @Mzack9999