From c299004abf0ef103a05034f84787dff9ac9030ea Mon Sep 17 00:00:00 2001 From: arvidn Date: Tue, 18 Feb 2020 14:48:29 +0100 Subject: [PATCH] undeprecate settings_pack::dht_upload_rate_limit. Make sure it doesn't overflow if set too high --- ChangeLog | 2 ++ include/libtorrent/aux_/session_impl.hpp | 2 +- include/libtorrent/settings_pack.hpp | 6 +--- src/session.cpp | 3 ++ src/session_impl.cpp | 13 +++++--- src/settings_pack.cpp | 2 +- test/test_session.cpp | 39 +++++++++++++++++++++++- 7 files changed, 55 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index d5712bb21..18c6131e2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,5 @@ + * undeprecate settings_pack::dht_upload_rate_limit + 1.2.4 release * fix binding TCP and UDP sockets to the same port, when specifying port 0 diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 72f7c2ca1..72ed1fa42 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/include/libtorrent/aux_/session_impl.hpp @@ -783,13 +783,13 @@ namespace aux { #if TORRENT_ABI_VERSION == 1 void update_ssl_listen(); - void update_dht_upload_rate_limit(); void update_local_download_rate(); void update_local_upload_rate(); void update_rate_limit_utp(); void update_ignore_rate_limits_on_local_network(); #endif + void update_dht_upload_rate_limit(); void update_proxy(); void update_i2p_bridge(); void update_peer_tos(); diff --git a/include/libtorrent/settings_pack.hpp b/include/libtorrent/settings_pack.hpp index 13e98504e..9672e45fc 100644 --- a/include/libtorrent/settings_pack.hpp +++ b/include/libtorrent/settings_pack.hpp @@ -1362,15 +1362,11 @@ namespace aux { deprecated_local_download_rate_limit, #endif -#if TORRENT_ABI_VERSION == 1 // ``dht_upload_rate_limit`` sets the rate limit on the DHT. This is // specified in bytes per second. For busy boxes // with lots of torrents that requires more DHT traffic, this should // be raised. - dht_upload_rate_limit TORRENT_DEPRECATED_ENUM, -#else - deprecated_dht_upload_rate_limit, -#endif + dht_upload_rate_limit, // ``unchoke_slots_limit`` is the max number of unchoked peers in the // session. The number of unchoke slots may be ignored depending on diff --git a/src/session.cpp b/src/session.cpp index c6d1c4a08..e3fb1bc11 100644 --- a/src/session.cpp +++ b/src/session.cpp @@ -337,6 +337,9 @@ namespace { #endif #ifndef TORRENT_DISABLE_DHT + if (params.settings.has_val(settings_pack::dht_upload_rate_limit)) + params.dht_settings.upload_rate_limit = params.settings.get_int(settings_pack::dht_upload_rate_limit); + m_impl->set_dht_settings(std::move(params.dht_settings)); m_impl->set_dht_state(std::move(params.dht_state)); diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 662d839a0..077b2e687 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -5817,6 +5817,9 @@ namespace aux { void session_impl::set_dht_settings(dht::dht_settings const& settings) { static_cast(m_dht_settings) = settings; + if (m_dht_settings.upload_rate_limit > std::numeric_limits::max() / 3) + m_dht_settings.upload_rate_limit = std::numeric_limits::max() / 3; + m_settings.set_int(settings_pack::dht_upload_rate_limit, m_dht_settings.upload_rate_limit); } void session_impl::set_dht_state(dht::dht_state&& state) @@ -6333,15 +6336,17 @@ namespace aux { || m_settings.get_int(settings_pack::unchoke_slots_limit) < 0; } -#if TORRENT_ABI_VERSION == 1 void session_impl::update_dht_upload_rate_limit() { #ifndef TORRENT_DISABLE_DHT - m_dht_settings.upload_rate_limit - = m_settings.get_int(settings_pack::dht_upload_rate_limit); + m_dht_settings.upload_rate_limit = m_settings.get_int(settings_pack::dht_upload_rate_limit); + if (m_dht_settings.upload_rate_limit > std::numeric_limits::max() / 3) + { + m_settings.set_int(settings_pack::dht_upload_rate_limit, std::numeric_limits::max() / 3); + m_dht_settings.upload_rate_limit = std::numeric_limits::max() / 3; + } #endif } -#endif void session_impl::update_disk_threads() { diff --git a/src/settings_pack.cpp b/src/settings_pack.cpp index bcab64a98..ee1f5932d 100644 --- a/src/settings_pack.cpp +++ b/src/settings_pack.cpp @@ -297,7 +297,7 @@ constexpr int CLOSE_FILE_INTERVAL = 0; SET(download_rate_limit, 0, &session_impl::update_download_rate), DEPRECATED_SET(local_upload_rate_limit, 0, &session_impl::update_local_upload_rate), DEPRECATED_SET(local_download_rate_limit, 0, &session_impl::update_local_download_rate), - DEPRECATED_SET(dht_upload_rate_limit, 4000, &session_impl::update_dht_upload_rate_limit), + SET(dht_upload_rate_limit, 8000, &session_impl::update_dht_upload_rate_limit), SET(unchoke_slots_limit, 8, &session_impl::update_unchoke_limit), DEPRECATED_SET(half_open_limit, 0, nullptr), SET(connections_limit, 200, &session_impl::update_connections_limit), diff --git a/test/test_session.cpp b/test/test_session.cpp index 8a64796e0..13f87c010 100644 --- a/test/test_session.cpp +++ b/test/test_session.cpp @@ -431,7 +431,7 @@ TORRENT_TEST(save_state_peer_id) auto const count_dht_inits = [](session& ses) { int count = 0; - int num = 120; // this number is adjusted per version, an estimate + int num = 200; // this number is adjusted per version, an estimate time_point const end_time = clock_type::now() + seconds(15); while (true) { @@ -498,6 +498,43 @@ TORRENT_TEST(init_dht_empty_bootstrap) TEST_EQUAL(count, 1); } +TORRENT_TEST(dht_upload_rate_overflow_pack) +{ + settings_pack p = settings(); + // make sure this doesn't cause an overflow + p.set_int(settings_pack::dht_upload_rate_limit, std::numeric_limits::max()); + p.set_int(settings_pack::alert_mask, alert_category_t(std::uint32_t(p.get_int(settings_pack::alert_mask))) + | alert::dht_log_notification); + p.set_bool(settings_pack::enable_dht, true); + lt::session s(p); + + p = s.get_settings(); + TEST_EQUAL(p.get_int(settings_pack::dht_upload_rate_limit), std::numeric_limits::max() / 3); + + int const count = count_dht_inits(s); + TEST_EQUAL(count, 1); +} + +TORRENT_TEST(dht_upload_rate_overflow) +{ + settings_pack p = settings(); + p.set_bool(settings_pack::enable_dht, true); + p.set_int(settings_pack::alert_mask, alert_category_t(std::uint32_t(p.get_int(settings_pack::alert_mask))) + | alert::dht_log_notification); + lt::session s(p); + + // make sure this doesn't cause an overflow + dht::dht_settings sett; + sett.upload_rate_limit = std::numeric_limits::max(); + s.set_dht_settings(sett); + + p = s.get_settings(); + TEST_EQUAL(p.get_int(settings_pack::dht_upload_rate_limit), std::numeric_limits::max() / 3); + + int const count = count_dht_inits(s); + TEST_EQUAL(count, 1); +} + #endif // TORRENT_DISABLE_DHT TORRENT_TEST(reopen_network_sockets)