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

[Draft] Add announce_port support #21692

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

Conversation

0xThiebaut
Copy link

@0xThiebaut 0xThiebaut commented Oct 27, 2024

Warning

Draft pending next release including arvidn/libtorrent#7771

The announce_port setting permits to overwrite the port passed along to trackers as the &port= parameter. If left as the default, the listening port is used. This setting is only meant for very special cases where a seed's listening port differs from the effectively exposed port (e.g., through external NAT-PMP). See arvidn/libtorrent#7771 for an example use-case.

This PR adds the relevant setting alongside the existing announce_ip setting.
image

The setting subsequently modifies the announced port (&port=1234):

127.0.0.1 - - [27/Oct/2024 09:44:16] "GET /announce?info_hash=%a7%801%f7%3b%a1k%3a%8e%b6%84%dd%cfyw%f1%f3%b8m!&peer_id=-qB5100-YwWihiQ4KGdM&port=1234&uploaded=0&downloaded=0&left=0&corrupt=0&key=CEDD0BC7&event=started&numwant=200&compact=1&no_peer_id=1&supportcrypto=1&redundant=0&ip=1.2.3.4 HTTP/1.1" 404 -

src/base/bittorrent/sessionimpl.cpp Outdated Show resolved Hide resolved
Comment on lines +1986 to +1987
// Port to announce to trackers
settingsPack.set_int(lt::settings_pack::announce_port, announcePort());
Copy link
Member

Choose a reason for hiding this comment

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

You should guard against libtorrent version here and only here:

Suggested change
// Port to announce to trackers
settingsPack.set_int(lt::settings_pack::announce_port, announcePort());
#if LIBTORRENT_VERSION_NUM >= XXXXX // please fill the correct version in
// Port to announce to trackers
settingsPack.set_int(lt::settings_pack::announce_port, announcePort());
#endif

Copy link
Author

Choose a reason for hiding this comment

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

Thanks a lot for the review. I will resolve this item once the merged arvidn/libtorrent#7771 reaches a release.

src/gui/advancedsettings.h Outdated Show resolved Hide resolved
src/gui/advancedsettings.cpp Outdated Show resolved Hide resolved
src/gui/advancedsettings.cpp Show resolved Hide resolved
src/webui/www/private/views/preferences.html Outdated Show resolved Hide resolved
@Chocobo1 Chocobo1 added the WebUI WebUI-related issues/changes label Nov 9, 2024
@0xThiebaut
Copy link
Author

On a side-note, the requires restart statement was copied over from the similar announce_ip setting; I'm however doubting whether this is an accurate statement for both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebUI WebUI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants