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

Enabling proxy for trackers prevents incoming TCP connections from peers #6001

Closed
grechnik opened this issue Feb 21, 2021 · 3 comments
Closed
Milestone

Comments

@grechnik
Copy link

My usecase is similar to #4904 : I setup proxy to connect to trackers while using direct connections with peers. In this configuration after #4904 is solved, UDP traffic is fine; however, any incoming TCP connection is accepted but immediately closed by the library.

It seems that the fix in #4909 leaves out a check in session_impl::on_accept_connection:

if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none)

At the very least, I think it should be synchronized with session_impl::reopen_listen_sockets
if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none
&& m_settings.get_bool(settings_pack::proxy_peer_connections))

More generally, I'm not sure whether the check in on_accept_connection makes sense at all: if we don't want to deal with the connection, we shouldn't be even listening on the port. Removing the check seems to fix the problem for me.

libtorrent version (or branch): 1.2.12.0, built in qBittorrent v4.3.3

platform/architecture: Windows 10 Build 19042.804, 64-bit

@arvidn
Copy link
Owner

arvidn commented Feb 22, 2021

I think you're right. The case where you don't proxy connection is not taken into account in that check.

I imagine this would fix it:

diff --git a/src/session_impl.cpp b/src/session_impl.cpp
index 892d9b895..7b3ecf553 100644
--- a/src/session_impl.cpp
+++ b/src/session_impl.cpp
@@ -2646,7 +2646,8 @@ namespace {
 
                // don't accept any connections from our local sockets if we're using a
                // proxy
-               if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none)
+               if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none
+                       && m_settings.get_bool(settings_pack::proxy_peer_connections))
                        return;
 
                auto listen = std::find_if(m_listen_sockets.begin(), m_listen_sockets.end()
@@ -5509,7 +5510,8 @@ namespace {
                        return std::uint16_t(sock->tcp_external_port());
                }
 
-               if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none)
+               if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none
+                       && m_settings.get_bool(settings_pack::proxy_peer_connections))
                        return 0;
 
                for (auto const& s : m_listen_sockets)

@arvidn arvidn added this to the 1.2.13 milestone Feb 22, 2021
@arvidn
Copy link
Owner

arvidn commented Feb 22, 2021

could you test this patch? #6002

@grechnik
Copy link
Author

I have tested the patch, it fixes the issue.

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

No branches or pull requests

2 participants