diff --git a/ChangeLog b/ChangeLog index f7abad56e..1f5527901 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 1.2.3 release + * fix erroneous event=completed tracker announce when checking files * promote errors in parsing listen_interfaces to post listen_failed_alert * fix bug in protocol encryption/obfuscation * fix buffer overflow in SOCKS5 UDP logic diff --git a/include/libtorrent/announce_entry.hpp b/include/libtorrent/announce_entry.hpp index e39a17a12..448250b68 100644 --- a/include/libtorrent/announce_entry.hpp +++ b/include/libtorrent/announce_entry.hpp @@ -59,7 +59,7 @@ namespace libtorrent { #endif // internal - explicit announce_endpoint(aux::listen_socket_handle const& s); + announce_endpoint(aux::listen_socket_handle const& s, bool completed); // if this tracker has returned an error or warning message // that message is stored here diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 40c3abd0d..444e1dc1b 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -1713,6 +1713,10 @@ namespace libtorrent { // this is set to true while waiting for an async_set_file_priority bool m_outstanding_file_priority:1; + // set to true if we've sent an event=completed to any tracker. This will + // prevent us from sending it again to anyone + bool m_complete_sent:1; + #if TORRENT_USE_ASSERTS // set to true when torrent is start()ed. It may only be started once bool m_was_started = false; diff --git a/simulation/setup_swarm.hpp b/simulation/setup_swarm.hpp index 876d6ae07..8d0869133 100644 --- a/simulation/setup_swarm.hpp +++ b/simulation/setup_swarm.hpp @@ -38,7 +38,7 @@ POSSIBILITY OF SUCH DAMAGE. #ifndef TORRENT_SETUP_SWARM_HPP_INCLUDED #define TORRENT_SETUP_SWARM_HPP_INCLUDED -enum class swarm_test { download, upload }; +enum class swarm_test { download, upload, upload_no_auto_stop }; void setup_swarm(int num_nodes , swarm_test type diff --git a/simulation/test_tracker.cpp b/simulation/test_tracker.cpp index 9d8be0b6e..517f2b855 100644 --- a/simulation/test_tracker.cpp +++ b/simulation/test_tracker.cpp @@ -151,7 +151,10 @@ void test_interval(int interval) } } -TORRENT_TEST(event_completed) +template +std::vector test_event(swarm_test const type + , AddTorrent add_torrent + , OnAlert on_alert) { using sim::asio::ip::address_v4; sim::default_config network_cfg; @@ -162,19 +165,16 @@ TORRENT_TEST(event_completed) sim::http_server http(web_server, 8080); // the request strings of all announces - std::vector> announces; + std::vector announces; const int interval = 500; - lt::time_point start = lt::clock_type::now(); http.register_handler("/announce" , [&](std::string method, std::string req , std::map&) { TEST_EQUAL(method, "GET"); - int const timestamp = int(chrono::duration_cast( - lt::clock_type::now() - start).count()); - announces.push_back({timestamp, req}); + announces.push_back(req); char response[500]; int const size = std::snprintf(response, sizeof(response), "d8:intervali%de5:peers0:e", interval); @@ -184,78 +184,96 @@ TORRENT_TEST(event_completed) lt::settings_pack default_settings = settings(); lt::add_torrent_params default_add_torrent; - int completion = -1; - - setup_swarm(2, swarm_test::download, sim, default_settings, default_add_torrent + setup_swarm(2, type, sim, default_settings, default_add_torrent // add session , [](lt::settings_pack&) { } // add torrent - , [](lt::add_torrent_params& params) { - params.trackers.push_back("http://2.2.2.2:8080/announce"); - } + , add_torrent // on alert - , [&](lt::alert const*, lt::session&) {} + , on_alert // terminate , [&](int const ticks, lt::session& ses) -> bool { - if (completion == -1 && is_seed(ses)) - { - completion = int(chrono::duration_cast( - lt::clock_type::now() - start).count()); - } - return ticks > duration; }); - // the first announce should be event=started, the second should be - // event=completed, then all but the last should have no event and the last - // be event=stopped. - for (int i = 0; i < int(announces.size()); ++i) - { - std::string const& str = announces[i].second; - int timestamp = announces[i].first; + // this is some basic sanity checking of the announces that should always be + // true. + // the first announce should be event=started then no other announce should + // have event=started. + // only the last announce should have event=stopped. + TEST_CHECK(announces.size() > 2); - const bool has_start = str.find("&event=started") - != std::string::npos; - const bool has_completed = str.find("&event=completed") - != std::string::npos; - const bool has_stopped = str.find("&event=stopped") - != std::string::npos; + if (announces.size() <= 2) return {}; + TEST_CHECK(announces.front().find("&event=started") != std::string::npos); + for (auto const& a : span(announces).subspan(1)) + TEST_CHECK(a.find("&event=started") == std::string::npos); - // we there can only be one event - const bool has_event = str.find("&event=") != std::string::npos; + TEST_CHECK(announces.back().find("&event=stopped") != std::string::npos); + for (auto const& a : span(announces).first(announces.size() - 1)) + TEST_CHECK(a.find("&event=stopped") == std::string::npos); + return announces; +} - std::printf("- %s\n", str.c_str()); - - // there is exactly 0 or 1 events. - TEST_EQUAL(int(has_start) + int(has_completed) + int(has_stopped) - , int(has_event)); - - switch (i) - { - case 0: - TEST_CHECK(has_start); - break; - case 1: - { - // the announce should have come approximately the same time we - // completed - TEST_CHECK(std::abs(completion - timestamp) <= 1); - TEST_CHECK(has_completed); - break; - } - default: - if (i == int(announces.size()) - 1) - { - TEST_CHECK(has_stopped); - } - else - { - TEST_CHECK(!has_event); - } - break; +TORRENT_TEST(event_completed_downloading) +{ + auto const announces = test_event(swarm_test::download + , [](lt::add_torrent_params& params) { + params.trackers.push_back("http://2.2.2.2:8080/announce"); } - } + , [&](lt::alert const*, lt::session&) {} + ); + + // make sure there's exactly one event=completed + TEST_CHECK(std::count_if(announces.begin(), announces.end(), [](std::string const& s) + { return s.find("&event=completed") != std::string::npos; }) == 1); +} + +TORRENT_TEST(event_completed_downloading_replace_trackers) +{ + auto const announces = test_event(swarm_test::download + , [](lt::add_torrent_params& params) {} + , [&](lt::alert const* a, lt::session&) { + if (auto const* at = alert_cast(a)) + at->handle.replace_trackers({announce_entry{"http://2.2.2.2:8080/announce"}}); + } + ); + + // make sure there's exactly one event=completed + TEST_CHECK(std::count_if(announces.begin(), announces.end(), [](std::string const& s) + { return s.find("&event=completed") != std::string::npos; }) == 1); +} + +TORRENT_TEST(event_completed_seeding) +{ + auto const announces = test_event(swarm_test::upload_no_auto_stop + , [](lt::add_torrent_params& params) { + params.trackers.push_back("http://2.2.2.2:8080/announce"); + } + , [&](lt::alert const*, lt::session&) {} + ); + + // make sure there are no event=completed, since we added the torrent as a + // seed + TEST_CHECK(std::count_if(announces.begin(), announces.end(), [](std::string const& s) + { return s.find("&event=completed") != std::string::npos; }) == 0); +} + + +TORRENT_TEST(event_completed_seeding_replace_trackers) +{ + auto const announces = test_event(swarm_test::upload_no_auto_stop + , [](lt::add_torrent_params& params) {} + , [&](lt::alert const* a, lt::session&) { + if (auto const* at = alert_cast(a)) + at->handle.replace_trackers({announce_entry{"http://2.2.2.2:8080/announce"}}); + } + ); + + // make sure there are no event=completed, since we added the torrent as a + // seed + TEST_CHECK(std::count_if(announces.begin(), announces.end(), [](std::string const& s) + { return s.find("&event=completed") != std::string::npos; }) == 0); } TORRENT_TEST(announce_interval_440) diff --git a/src/announce_entry.cpp b/src/announce_entry.cpp index 772e52160..50e9ca79d 100644 --- a/src/announce_entry.cpp +++ b/src/announce_entry.cpp @@ -47,13 +47,13 @@ namespace libtorrent { minutes32 constexpr tracker_retry_delay_max{60}; } - announce_endpoint::announce_endpoint(aux::listen_socket_handle const& s) + announce_endpoint::announce_endpoint(aux::listen_socket_handle const& s, bool const completed) : local_endpoint(s ? s.get_local_endpoint() : tcp::endpoint()) , socket(s) , fails(0) , updating(false) , start_sent(false) - , complete_sent(false) + , complete_sent(completed) , triggered_manually(false) {} diff --git a/src/torrent.cpp b/src/torrent.cpp index a4e0f77ab..9ef3c876c 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -228,6 +228,7 @@ bool is_downloading_state(int const st) , m_progress_ppm(0) , m_torrent_initialized(false) , m_outstanding_file_priority(false) + , m_complete_sent(false) { // we cannot log in the constructor, because it relies on shared_from_this // being initialized, which happens after the constructor returns. @@ -2925,7 +2926,7 @@ bool is_downloading_state(int const st) return; } - ae.endpoints.emplace_back(s); + ae.endpoints.emplace_back(s, bool(m_complete_sent)); std::swap(ae.endpoints[valid_endpoints], ae.endpoints.back()); valid_endpoints++; }); @@ -2986,7 +2987,12 @@ bool is_downloading_state(int const st) if (req.event == tracker_request::none) { if (!aep.start_sent) req.event = tracker_request::started; - else if (!aep.complete_sent && is_seed()) req.event = tracker_request::completed; + else if (!m_complete_sent + && !aep.complete_sent + && is_seed()) + { + req.event = tracker_request::completed; + } } req.triggered_manually = aep.triggered_manually; @@ -3218,6 +3224,10 @@ bool is_downloading_state(int const st) tcp::endpoint local_endpoint; if (ae) { +#if TORRENT_ABI_VERSION == 1 + if (!ae->complete_sent && r.event == tracker_request::completed) + ae->complete_sent = true; +#endif announce_endpoint* aep = ae->find_endpoint(r.outgoing_socket); if (aep) { @@ -3228,7 +3238,13 @@ bool is_downloading_state(int const st) if (!aep->start_sent && r.event == tracker_request::started) aep->start_sent = true; if (!aep->complete_sent && r.event == tracker_request::completed) + { aep->complete_sent = true; + // we successfully reported event=completed to one tracker. Don't + // send it to any other ones from now on (there may be other + // announces outstanding right now though) + m_complete_sent = true; + } ae->verified = true; aep->next_announce = now + interval; aep->min_announce = now + resp.min_interval; @@ -5246,8 +5262,11 @@ bool is_downloading_state(int const st) { t.endpoints.clear(); if (t.source == 0) t.source = announce_entry::source_client; +#if TORRENT_ABI_VERSION == 1 + t.complete_sent = m_complete_sent; +#endif for (auto& aep : t.endpoints) - aep.complete_sent = is_seed(); + aep.complete_sent = m_complete_sent; } if (settings().get_bool(settings_pack::prefer_udp_trackers)) @@ -7554,9 +7573,18 @@ bool is_downloading_state(int const st) } else { + // we just added this torrent as a seed, or force-rechecked it, and we + // have all of it. Assume that we sent the event=completed when we + // finished downloading it, and don't send any more. + m_complete_sent = true; for (auto& t : m_trackers) + { +#if TORRENT_ABI_VERSION == 1 + t.complete_sent = true; +#endif for (auto& aep : t.endpoints) aep.complete_sent = true; + } if (m_state != torrent_status::finished && m_state != torrent_status::seeding) diff --git a/test/test_primitives.cpp b/test/test_primitives.cpp index 69aaa51e3..8295e83f9 100644 --- a/test/test_primitives.cpp +++ b/test/test_primitives.cpp @@ -47,7 +47,7 @@ TORRENT_TEST(retry_interval) // make sure the retry interval keeps growing // on failing announces announce_entry ae("dummy"); - ae.endpoints.emplace_back(aux::listen_socket_handle()); + ae.endpoints.emplace_back(aux::listen_socket_handle(), false); int last = 0; auto const tracker_backoff = 250; for (int i = 0; i < 10; ++i)