Fixing TCP Listening Port with Proxy by Masong19hippows · Pull Request #8003 · arvidn/libtorrent

Masong19hippows · 2025-07-27T22:24:27Z

Hello,

I made a PR awhile back about listening on the defined TCP port whenever proxies are enabled. The current behavior is to disable listening when a proxy is set. My proposed changes would add a bool configuration option that would enable listening when a proxy is configured. Default value is false. In the same logic, I also removed the proxy check when enabling natPMP/upnp. This check was meant to disable opening up these ports if the proxy was configured. Instead of coupling this with any proxy settings, I just removed the check all together so that it's only dependent on the "Use UPnP / NAT-PMP port forwarding from my router" flag. I felt this was best because it follows the same logic as decoupling the listening TCP port with the proxy settings - user should decide with one setting.

This was in PR #7726

I felt it was best to close that PR and start anew because I messed up a rebase. During the year of inactivity, there were alot of changes that caused the PR to need a rebase. It got messy because I messed up and I felt like this was a much better option. I think I resolved all of the issues @arvidn showed as well.

Please review when you have a moment.

Masong19hippows · 2025-07-28T00:21:43Z

I'm not sure why the build is failing on github. It doesn't look related to anything I changed. My guess is that I have to change something in the settings pack test with the other bools, but I don't know the magic number to add to the base.

jiang-zhexin · 2025-07-29T01:45:44Z

I really need this feature, this is awesome.

And could add SOCKS5 BIND support? I've implemented it in Go, it might not be too hard.

Masong19hippows · 2025-07-29T01:55:55Z

@jiang-zhexin

You might need to look back at the previous pr for more info where I explained it better. Essentialy the socks bind command is meant to be a temporary port on the server side for secondary connections to an already existing connect. Like, if I contacted a website through a socks proxy, I could then initiate the bind process for that website to send traffic back to me through an open port on the server.

It's not really a bind in the traditional sense, and because of this, I don't think it's possible to accept incoming connections through sock bind.

Masong19hippows · 2025-08-04T02:31:00Z

I think I might've fixed it. I think that means it's good to merge on the ABI stability part then?

arvidn

I haven't thought this through, but it seems to be some overlap with this new setting and proxy_peer_connections. For example, if proxy_peer_connections is set and proxy_accept_incoming is set, that seems like a contradiction.

Did we already discuss the feasibility of just making proxy_peer_connections being false meaning incoming connections are allowed? Or perhaps that's already the case.

arvidn · 2025-08-18T11:10:58Z

 			// man-in-the-middle connections.
 			proxy_send_host_in_connect,

+			// if true, listening on the defined port will be enabled while using proxy


I think this needs some elaboration. "defined" port should probably be described as local listen sockets, allowing incoming connections that are not proxied, circumventing the proxy.

Just updated it. Is that better?

arvidn

did you consider only changing repoen_listen_sockets()?
I think the check right here is the only thing you would need to change: https://github.com/arvidn/libtorrent/blob/RC_2_0/src/session_impl.cpp#L2060-L2072

Right now, if we use a proxy for peers, we ignore all the listen sockets set up, and just create a single one with the proxy flag. That's the flag that causes the behavior you don't want. i.e. not accepting incoming connections directly, only via the proxy.

You could, instead, treat the proxy as addative to the listen sockets configured by listen_interfaces (rather than replacing them). I believe that would also give you the behavior you need, but with a smaller patch and without changing so much existing behavior.

arvidn · 2025-08-18T11:19:46Z

 		if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none
-			&& m_settings.get_bool(settings_pack::proxy_peer_connections))
+			&& m_settings.get_bool(settings_pack::proxy_peer_connections)
+			&& !m_settings.get_bool(settings_pack::proxy_accept_incoming))


this seems to be the crux. Is the point if proxy_accept_incoming to accept incoming peers on the local socket even though outgoing connections must go via the proxy?

Yes that's exactly it. Essentially, the outgoing connection towards clients via the proxy or not shouldn't affect incoming connections, because qbittorrent has no way of accepting incoming connections through the proxy via any proxy protocols.

so, if proxy_peer_connections is false, then proxy_accept_incoming does not have any effect. is that right?

I believe so.

Masong19hippows · 2025-08-18T14:42:38Z

I haven't thought this through, but it seems to be some overlap with this new setting and proxy_peer_connections. For example, if proxy_peer_connections is set and proxy_accept_incoming is set, that seems like a contradiction.

Did we already discuss the feasibility of just making proxy_peer_connections being false meaning incoming connections are allowed? Or perhaps that's already the case.

The problem with that approach is that there is still a use case to use the proxy for peer connections and still listen on the local port. With some external forwarding tools, you can forward incoming connections from the proxy to the local port, but that doesn't work if qbittorrent isn't listening on that port.

Masong19hippows · 2025-08-18T20:15:46Z

did you consider only changing repoen_listen_sockets()? I think the check right here is the only thing you would need to change: https://github.com/arvidn/libtorrent/blob/RC_2_0/src/session_impl.cpp#L2060-L2072

Right now, if we use a proxy for peers, we ignore all the listen sockets set up, and just create a single one with the proxy flag. That's the flag that causes the behavior you don't want. i.e. not accepting incoming connections directly, only via the proxy.

I believe I already did this here, unless I am mindreading what you are saying. My proposed solution is to add an additional check for the new proxy_accept_incoming flag.

This has 2 different permutations then. If there is a proxy configured and proxy_peer_connections is true, then we would listen on the local port if proxy_accept_incoming is true and follow original logic if false. If there is a proxy configured and proxy_peer_connections is false, then we won't listen on the local port regardless of the proxy_accept_incoming flag. We could remove the proxy_peer_connections check here, but then it would change the predefined behavior, which you said you didn't want to do in the previous pr, and instead want to guard this new behavior around this new flag.

In addition to this check, there is also a check on line 2834 that does the same thing, which I believe to be necessary.

You could, instead, treat the proxy as addative to the listen sockets configured by listen_interfaces (rather than replacing them). I believe that would also give you the behavior you need, but with a smaller patch and without changing so much existing behavior.

Can you please elaborate on what you mean here? I'm not familiar enough with the codebase to understand.

arvidn · 2025-09-08T06:58:00Z

 		if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none
-			&& m_settings.get_bool(settings_pack::proxy_peer_connections))
+			&& m_settings.get_bool(settings_pack::proxy_peer_connections)
+			&& !m_settings.get_bool(settings_pack::proxy_accept_incoming))


so, if proxy_peer_connections is false, then proxy_accept_incoming does not have any effect. is that right?

arvidn · 2025-09-08T07:02:50Z

+		if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none
+			&& m_settings.get_bool(settings_pack::proxy_peer_connections)
+			&& !m_settings.get_bool(settings_pack::proxy_accept_incoming))
+			return 0;


This check and early exit should only be used if sock == nullptr. If sock is set, we already have this check based on whether the listen socket is using a proxy or not.

I think the if statement before this one guarantees that this check will only be ran if sock is null. The if block with if(sock) on line 5529 has a guaranteed return, and so the following if block will only run if sock is null. Regardless, I changed this to have a null check and this should be resolved.

arvidn · 2025-09-08T07:06:26Z

-		if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none
-			&& m_settings.get_bool(settings_pack::proxy_peer_connections))
-			return 0;


This doesn't look right. This should probably mimic the session_impl::listen_port() above. I think you'd want to update the check and early exit on line 5569 as well

I think I fixed this if you want to take a look @arvidn, when you get a chance.

arvidn · 2025-09-08T07:07:20Z

-			&& !(s->flags & listen_socket_t::local_network)
-			&& !(s->flags & listen_socket_t::proxy))


why does this need to change? I can see taking the new option into account, not just removing this condition

I think removing the proxy check for natpmp/upnp makes intuitive sense, because there is already a flag specifically for it. I think it would be confusing for people that use a proxy to also make sure this new flag is checked if they want natpmp/upnp to work, especially when the new flag doesn't describe natpmp/upnp. Please let me know your thoughts, I can add it back in if you'd like.

The default assumption of using a proxy for peer connections is that you don't accept incoming peer connections that circumvent the proxy. If you don't accept incoming connections, clearly you also should not forward the port. There's no listen socket receiving the incoming connection anyway.

Apologies for the delay, I just saw this. I believe I corrected this if you want to take a look.

arvidn · 2025-09-08T07:07:44Z

 		// the gateway
-		if ((s->flags & listen_socket_t::local_network)
-			|| (s->flags & listen_socket_t::proxy))
+		if ((s->flags & listen_socket_t::local_network))


here too. You're removing this check. I would imagine you want to take the new option into account instead

#8003 (comment)

fruworg · 2025-11-07T16:59:32Z

LGTM

erns8 · 2026-06-23T01:29:53Z

Any update ? @arvidn

This comment was marked as resolved.

Sign in to view

Masong19hippows added 7 commits August 4, 2025 08:55

Fixing Listening Port with Proxy

4b41ec8

Fixing diff file error

12b61bb

Fixing extra + from diff file

a2269cd

Fixing ABI

1cb0309

Fixing ABI

ce5cecb

Fixing ABI

96f20c8

removing client file

a8c0154

Masong19hippows force-pushed the ListenOnTCPWithProxy branch from 5363900 to a8c0154 Compare August 4, 2025 13:56

arvidn reviewed Aug 18, 2025

View reviewed changes

Update syntax & update description of proxy_accept_incoming

d235307

arvidn reviewed Sep 8, 2025

View reviewed changes

Masong19hippows added 2 commits October 16, 2025 14:44

Adding checks back in

276cfbe

Correcting comments

6c13071

arvidn self-assigned this Jan 4, 2026

Masong19hippows added 2 commits April 19, 2026 20:40

fix: adding proxy checks for natpmp/upnp

828255f

Merge branch 'RC_2_0' into ListenOnTCPWithProxy

73e0638

		&& !(s->flags & listen_socket_t::local_network)
		&& !(s->flags & listen_socket_t::proxy))

Uh oh!

Conversation

Masong19hippows commented Jul 27, 2025

Uh oh!

Masong19hippows commented Jul 28, 2025

Uh oh!

This comment was marked as resolved.

jiang-zhexin commented Jul 29, 2025

Uh oh!

Masong19hippows commented Jul 29, 2025

Uh oh!

Masong19hippows commented Aug 4, 2025

Uh oh!

This comment was marked as resolved.

arvidn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arvidn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Masong19hippows Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Masong19hippows commented Aug 18, 2025

Uh oh!

Masong19hippows commented Aug 18, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Masong19hippows Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fruworg commented Nov 7, 2025

Uh oh!

erns8 commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Masong19hippows Aug 18, 2025 •

edited

Loading

Masong19hippows Oct 16, 2025 •

edited

Loading