make UPnP port-mapping lease duration configurable, and default to 1 hour leases

ChangeLog

@@ -1,3 +1,4 @@
+	* make UPnP port-mapping lease duration configurable
 	* deprecate the bittyrant choking algorithm
 	* add build option to disable streaming
 

include/libtorrent/aux_/session_impl.hpp

@@ -829,7 +829,6 @@ namespace aux {
 
 			void update_socket_buffer_size();
 			void update_dht_announce_interval();
-			void update_anonymous_mode();
 			void update_download_rate();
 			void update_upload_rate();
 			void update_connections_limit();

include/libtorrent/settings_pack.hpp

@@ -1735,6 +1735,13 @@ namespace aux {
 			// option.
 			send_not_sent_low_watermark,
 
+			// The expiration time of UPnP port-mappings, specified in seconds. 0
+			// means permanent lease. Some routers do not support expiration times
+			// on port-maps (nor correctly returning an error indicating lack of
+			// support). In those cases, set this to 0. Otherwise, don't set it any
+			// lower than 5 minutes.
+			upnp_lease_duration,
+
 			max_int_setting_internal
 		};
 

include/libtorrent/upnp.hpp

@@ -42,6 +42,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/string_util.hpp"
 #include "libtorrent/aux_/portmap.hpp"
 #include "libtorrent/aux_/vector.hpp"
+#include "libtorrent/aux_/session_settings.hpp"
 
 #include <memory>
 #include <functional>
@@ -147,15 +148,13 @@ struct TORRENT_EXTRA_EXPORT upnp final
 	, single_threaded
 {
 	upnp(io_service& ios
-		, std::string const& user_agent
+		, aux::session_settings const& settings
 		, aux::portmap_callback& cb
 		, address_v4 const& listen_address
 		, address_v4 const& netmask
 		, std::string listen_device);
 	~upnp();
 
-	void set_user_agent(std::string const& v) { m_user_agent = v; }
-
 	void start();
 
 	// Attempts to add a port mapping for the specified protocol. Valid protocols are
@@ -288,11 +287,8 @@ private:
 		std::string path;
 		address external_ip;
 
-		// there are routers that's don't support timed
-		// port maps, without returning error 725. It seems
-		// safer to always assume that we have to ask for
-		// permanent leases
-		int lease_duration = 0;
+		// default lease duration for a port map.
+		int lease_duration = 3600;
 
 		// true if the device supports specifying a
 		// specific external port, false if it doesn't
@@ -318,7 +314,7 @@ private:
 
 	aux::vector<global_mapping_t, port_mapping_t> m_mappings;
 
-	std::string m_user_agent;
+	aux::session_settings const& m_settings;
 
 	// the set of devices we've found
 	std::set<rootdevice> m_devices;

src/session_impl.cpp

@@ -6500,25 +6500,6 @@ namespace {
 #endif
 	}
 
-	void session_impl::update_anonymous_mode()
-	{
-		if (!m_settings.get_bool(settings_pack::anonymous_mode))
-		{
-			for (auto& s : m_listen_sockets)
-			{
-				if (!s->upnp_mapper) continue;
-				s->upnp_mapper->set_user_agent(m_settings.get_str(settings_pack::user_agent));
-			}
-			return;
-		}
-
-		for (auto& s : m_listen_sockets)
-		{
-			if (!s->upnp_mapper) continue;
-			s->upnp_mapper->set_user_agent("");
-		}
-	}
-
 #if TORRENT_ABI_VERSION == 1
 	void session_impl::update_local_download_rate()
 	{
@@ -6769,9 +6750,7 @@ namespace {
 		{
 			// the upnp constructor may fail and call the callbacks
 			// into the session_impl.
-			s.upnp_mapper = std::make_shared<upnp>(m_io_service
-				, m_settings.get_bool(settings_pack::anonymous_mode)
-				? "" : m_settings.get_str(settings_pack::user_agent)
+			s.upnp_mapper = std::make_shared<upnp>(m_io_service, m_settings
 				, *this, s.local_endpoint.address().to_v4(), s.netmask.to_v4(), s.device);
 			s.upnp_mapper->start();
 		}

src/settings_pack.cpp

@@ -177,7 +177,7 @@ constexpr int CLOSE_FILE_INTERVAL = 0;
 		SET(enable_incoming_tcp, true, nullptr),
 		SET(ignore_resume_timestamps, false, nullptr),
 		SET(no_recheck_incomplete_resume, false, nullptr),
-		SET(anonymous_mode, false, &session_impl::update_anonymous_mode),
+		SET(anonymous_mode, false, nullptr),
 		SET(report_web_seed_downloads, true, &session_impl::update_report_web_seed_downloads),
 		DEPRECATED_SET(rate_limit_utp, true, &session_impl::update_rate_limit_utp),
 		DEPRECATED_SET(announce_double_nat, false, nullptr),
@@ -349,6 +349,7 @@ constexpr int CLOSE_FILE_INTERVAL = 0;
 		SET(max_web_seed_connections, 3, nullptr),
 		SET(resolver_cache_timeout, 1200, &session_impl::update_resolver_cache_timeout),
 		SET(send_not_sent_low_watermark, 16384, nullptr),
+		SET(upnp_lease_duration, 3600, nullptr),
 	}});
 
 #undef SET

src/upnp.cpp

@@ -77,14 +77,16 @@ namespace upnp_errors
 
 static error_code ignore_error;
 
-upnp::rootdevice::rootdevice() {}
+upnp::rootdevice::rootdevice() = default;
+#if TORRENT_USE_ASSERTS
 upnp::rootdevice::~rootdevice()
 {
 	TORRENT_ASSERT(magic == 1337);
-#if TORRENT_USE_ASSERTS
 	magic = 0;
-#endif
 }
+#else
+upnp::rootdevice::~rootdevice() = default;
+#endif
 
 upnp::rootdevice::rootdevice(rootdevice const&) = default;
 upnp::rootdevice& upnp::rootdevice::operator=(rootdevice const&) = default;
@@ -95,12 +97,12 @@ upnp::rootdevice& upnp::rootdevice::operator=(rootdevice&&) = default;
 // bind to, since the broadcast socket opens one socket per local
 // interface by default
 upnp::upnp(io_service& ios
-	, std::string const& user_agent
+	, aux::session_settings const& settings
 	, aux::portmap_callback& cb
 	, address_v4 const& listen_address
 	, address_v4 const& netmask
 	, std::string listen_device)
-	: m_user_agent(user_agent)
+	: m_settings(settings)
 	, m_callback(cb)
 	, m_io_service(ios)
 	, m_resolver(ios)
@@ -557,6 +559,7 @@ void upnp::on_reply(udp::socket& s, error_code const& ec)
 
 	rootdevice d;
 	d.url = url;
+	d.lease_duration = m_settings.get_int(settings_pack::upnp_lease_duration);
 
 	auto i = m_devices.find(d);
 
@@ -742,7 +745,8 @@ void upnp::create_port_mapping(http_connection& c, rootdevice& d
 		, to_string(d.mapping[i].protocol)
 		, d.mapping[i].local_ep.port()
 		, local_endpoint.c_str()
-		, m_user_agent.c_str()
+		, m_settings.get_bool(settings_pack::anonymous_mode)
+			? "" : m_settings.get_str(settings_pack::user_agent).c_str()
 		, d.lease_duration, soap_action);
 
 	post(d, soap, soap_action);

test/test_upnp.cpp

@@ -231,13 +231,13 @@ void run_upnp_test(char const* root_filename, char const* control_name, int igd_
 
 	sock->open(&incoming_msearch, ios, ec);
 
-	std::string user_agent = "test agent";
+	aux::session_settings sett;
 
 	// pick an appropriate interface to run this test on
 	auto const ipf = pick_upnp_interface();
 
 	upnp_callback cb;
-	auto upnp_handler = std::make_shared<upnp>(ios, user_agent, cb
+	auto upnp_handler = std::make_shared<upnp>(ios, sett, cb
 		, ipf.interface_address.to_v4(), ipf.netmask.to_v4(), ipf.name);
 	upnp_handler->start();
 
@@ -326,9 +326,10 @@ TORRENT_TEST(upnp_max_mappings)
 	lt::io_service ios;
 
 	auto const ipf = pick_upnp_interface();
+	aux::session_settings sett;
 
 	upnp_callback cb;
-	auto upnp_handler = std::make_shared<upnp>(ios, "", cb
+	auto upnp_handler = std::make_shared<upnp>(ios, sett, cb
 		, ipf.interface_address.to_v4(), ipf.netmask.to_v4(), ipf.name);
 
 	for (int i = 0; i < 50; ++i)