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

only trigger default listen sockets if the listen_interfaces setting … #4166

Closed
wants to merge 1 commit into from

Conversation

ssiloti
Copy link
Collaborator

@ssiloti ssiloti commented Dec 9, 2019

…is empty

The old code would add default listen sockets when listen_interfces was
non-empty but only contained invalid interface specifications.

In the future it may be possible to support operation with no listen sockets
at all, but this will require additional work since the announce code currently
requires a listen socket to function.

…is empty

The old code would add default listen sockets when listen_interfces was
non-empty but only contained invalid interface specifications.

In the future it may be possible to support operation with no listen sockets
at all, but this will require additional work since the announce code currently
requires a listen socket to function.
{
// if no listen interfaces are specified, create sockets to use
// any interface
m_listen_interfaces = parse_listen_interfaces("0.0.0.0:0,[::]:0");
Copy link
Owner

Choose a reason for hiding this comment

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

I think the duplex::only_outgoing flag in the previous code is relevant.

Copy link
Owner

Choose a reason for hiding this comment

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

I seem to recall that the old behavior tried to mimic, as closely as possible, not having any listen sockets. But I think the name is a bit misleading as it is. Since we use these for outgoing connections also. And I think it makes sense to preserve this attempt to not accept incoming connections for an empty string.

But doesn't this still raise the question of what happens when a torrent attempts to announce when there are literally no sockets to use?

@arvidn
Copy link
Owner

arvidn commented Dec 9, 2019

as far as I can tell, opening the "default listen sockets" is as close as we can get to not opening any at all, since we use these sockets for outgoing connections as well. But the existing code sets the duplex to outgoing only, which (I believe) effectively makes them not listen for incoming connections.

Hence, I think the current code is more correct; if you don't specify any listen interfaces, you won't receive incoming connections. If you have invalid syntax for the listen interfaces, you also won't receiving incoming connections, but at least you'll get errors now.

@arvidn arvidn mentioned this pull request Dec 9, 2019
@ssiloti ssiloti closed this Dec 9, 2019
@ssiloti ssiloti deleted the default-listen-on-empty-only branch December 9, 2019 17:41
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