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.

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

Fix issue with homebrew-installed nc taking precedence in $PATH #12366

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix issue with homebrew-installed nc taking precedence in $PATH #12366

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions plugins/shell-proxy/ssh-proxy.py
View file
Open in desktop
Expand Up @@ -21,8 +21,12 @@
raise TypeError('unsupported proxy protocol: "{}"'.format(parsed.scheme))

def make_argv():
yield "nc"
if sys.platform in {'linux', 'cygwin'}:
if sys.platform == 'darwin':
# 'nc' in $PATH may be installed by homebrew, if without path
yield "/usr/bin/nc"
else:
yield "nc"
Comment on lines +24 to +28
Copy link
Contributor

@septs septs Apr 19, 2024 •
edited

Choose a reason for hiding this comment

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

hardcoded paths are bad code.

Suggested change
if sys.platform == 'darwin':
# 'nc' in $PATH may be installed by homebrew, if without path
yield "/usr/bin/nc"
else:
yield "nc"
yield os.environ.get("SHELLPROXY_NETCAT_PROGRAM", "nc")

References

Copy link
Author

Choose a reason for hiding this comment

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

/usr/bin/nc is the BSD version pre-installed in MacOS，If users need to configure the system using SHELLPROXY_NETCAT_PROGRAM, then it might require debugging to identify the issue.

if sys.platform in {'linux', 'cygwin', 'darwin'}:
Copy link
Contributor

Choose a reason for hiding this comment

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

plz split to a new pr, a pull request should only handle one thing.

Copy link
Author

Choose a reason for hiding this comment

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

/usr/bin/nc is the BSD version pre-installed in macos, which need need use the same config as linux and cygwin. Therefore, it's all about addressing the issue of macOS not being ready to use out of the box.

# caveats: the built-in netcat of most linux distributions and cygwin support proxy type
# caveats: macOS built-in netcat command not supported proxy-type
yield "-X" # --proxy-type
Expand Down