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

Make wrap_cmd work as a simple run when using unix sockets #570

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

Conversation

dylex
Copy link

@dylex dylex commented Feb 16, 2024

If unix-target is specified, don't use rebind (fixes a crash when accessing target_port). If unix-listen is specified, there is no port argument to skip.

Currently if you specify a wrap_cmd along with a unix_listen socket or a unix_target, things break because of the lack of port. Admittedly there is little reason to use wrap_cmd in this case because you could just run it separately, but it's still a convenient to just run the command.

If unix-target is specified, don't use rebind (fixes a crash when
accessing target_port).  If unix-listen is specified, there is no port
argument to skip.

Currently if you specify a wrap_cmd along with a unix_listen socket or a
unix_target, things break because of the lack of port.  Admittedly there
is little reason to use wrap_cmd in this case because you could just run
it separately, but it's still a convenient to just run the command.
@CendioOssman
Copy link
Member

Maybe I'm missing something, but if I understand you correctly, websockify crashes if you try to combine wrapping with a unix socket target?

That seems like a bug we should fix in that case, instead of disabling wrapping. That will just confuse the user, as websockify will not do what they asked.

@dylex
Copy link
Author

dylex commented Feb 23, 2024

Yes, currently it crashes if you specify --unix-target and a wrap command, and you get strange errors with --unix-listen and a wrap command.

I realize now neither of these are supported usages, similar to specifying both source and target port with a wrap command. I have to admit that I hadn't understood what the wrap command (and rebind) did until I looked at rebind itself, and just assumed it was used to spawn and monitor the program (which it also does). If the intention is to only use wrap commands when using rebind, then these cases should probably be caught with clearer errors. (I don't think rebind could be made to work very easily with unix sockets only as they need to exist beforehand, and can't work at all when proxying from tcp to unix socket or vice-versa.)

However, I think it's also useful to specify a command to run (and potentially respawn) when running without rebind, with explicit ports or sockets. This is a step towards that, though it should also then disable rebind when both target and listen TCP ports are specified.

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