promote errors in parsing listen_interfaces to post listen_failed_alert

ChangeLog

@@ -1,3 +1,4 @@
+	* promote errors in parsing listen_interfaces to post listen_failed_alert
 	* fix bug in protocol encryption/obfuscation
 	* fix buffer overflow in SOCKS5 UDP logic
 	* fix issue of rapid calls to file_priority() clobbering each other

include/libtorrent/string_util.hpp

@@ -36,6 +36,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/config.hpp"
 #include "libtorrent/string_view.hpp"
 #include "libtorrent/span.hpp"
+#include "libtorrent/error_code.hpp"
 
 #include <vector>
 #include <string>
@@ -60,6 +61,9 @@ namespace libtorrent {
 			url += '/';
 	}
 
+	// internal
+	TORRENT_EXTRA_EXPORT string_view strip_string(string_view in);
+
 	TORRENT_EXTRA_EXPORT bool is_print(char c);
 	TORRENT_EXTRA_EXPORT bool is_space(char c);
 	TORRENT_EXTRA_EXPORT char to_lower(char c);
@@ -80,16 +84,25 @@ namespace libtorrent {
 		std::string device;
 		int port;
 		bool ssl;
+		friend bool operator==(listen_interface_t const& lhs, listen_interface_t const& rhs)
+		{
+			return lhs.device == rhs.device
+				&& lhs.port == rhs.port
+				&& lhs.ssl == rhs.ssl;
+		}
 	};
 
 	// this parses the string that's used as the listen_interfaces setting.
 	// it is a comma-separated list of IP or device names with ports. For
 	// example: "eth0:6881,eth1:6881" or "127.0.0.1:6881"
 	TORRENT_EXTRA_EXPORT std::vector<listen_interface_t> parse_listen_interfaces(
-		std::string const& in);
+		std::string const& in, std::vector<std::string>& errors);
 
+#if TORRENT_ABI_VERSION == 1 \
+	|| !defined TORRENT_DISABLE_LOGGING
 	TORRENT_EXTRA_EXPORT std::string print_listen_interfaces(
 		std::vector<listen_interface_t> const& in);
+#endif
 
 	// this parses the string that's used as the listen_interfaces setting.
 	// it is a comma-separated list of IP or device names with ports. For

src/session_impl.cpp

@@ -5192,8 +5192,9 @@ namespace aux {
 		// this function maps the previous functionality of just setting the ssl
 		// listen port in order to enable the ssl listen sockets, to the new
 		// mechanism where SSL sockets are specified in listen_interfaces.
+		std::vector<std::string> ignore;
 		auto current_ifaces = parse_listen_interfaces(
-			m_settings.get_str(settings_pack::listen_interfaces));
+			m_settings.get_str(settings_pack::listen_interfaces), ignore);
 		// these are the current interfaces we have, first remove all the SSL
 		// interfaces
 		current_ifaces.erase(std::remove_if(current_ifaces.begin(), current_ifaces.end()
@@ -5227,16 +5228,18 @@ namespace aux {
 		INVARIANT_CHECK;
 
 		std::string const net_interfaces = m_settings.get_str(settings_pack::listen_interfaces);
-		m_listen_interfaces = parse_listen_interfaces(net_interfaces);
+		std::vector<std::string> err;
+		m_listen_interfaces = parse_listen_interfaces(net_interfaces, err);
+
+		for (auto const& e : err)
+		{
+			m_alerts.emplace_alert<listen_failed_alert>(e, lt::address{}, 0
+				, operation_t::parse_address, errors::invalid_port, lt::socket_type_t::tcp);
+		}
 
 #ifndef TORRENT_DISABLE_LOGGING
 		if (should_log())
 		{
-			if (!net_interfaces.empty() && m_listen_interfaces.empty())
-			{
-				session_log("ERROR: failed to parse listen_interfaces setting: %s"
-					, net_interfaces.c_str());
-			}
 			session_log("update listen interfaces: %s", net_interfaces.c_str());
 			session_log("parsed listen interfaces count: %d, ifaces: %s"
 				, int(m_listen_interfaces.size())

src/string_util.cpp

@@ -149,6 +149,8 @@ namespace libtorrent {
 		return tmp;
 	}
 
+#if TORRENT_ABI_VERSION == 1 \
+	|| !defined TORRENT_DISABLE_LOGGING
 	std::string print_listen_interfaces(std::vector<listen_interface_t> const& in)
 	{
 		std::string ret;
@@ -176,92 +178,101 @@ namespace libtorrent {
 
 		return ret;
 	}
+#endif
+
+	string_view strip_string(string_view in)
+	{
+		while (!in.empty() && is_space(in.front()))
+			in.remove_prefix(1);
+
+		while (!in.empty() && is_space(in.back()))
+			in.remove_suffix(1);
+		return in;
+	}
 
 	// this parses the string that's used as the listen_interfaces setting.
 	// it is a comma-separated list of IP or device names with ports. For
 	// example: "eth0:6881,eth1:6881" or "127.0.0.1:6881"
-	std::vector<listen_interface_t> parse_listen_interfaces(std::string const& in)
+	std::vector<listen_interface_t> parse_listen_interfaces(std::string const& in
+		, std::vector<std::string>& err)
 	{
 		std::vector<listen_interface_t> out;
 
-		std::string::size_type start = 0;
+		string_view rest = in;
-
+		while (!rest.empty())
-		while (start < in.size())
 		{
-			// skip leading spaces
+			string_view element;
-			while (start < in.size() && is_space(in[start]))
+			std::tie(element, rest) = split_string(rest, ',');
-				++start;
 
-			if (start == in.size()) return out;
+			element = strip_string(element);
+			if (element.size() > 1 && element.front() == '"' && element.back() == '"')
+				element = element.substr(1, element.size() - 2);
+			if (element.empty()) continue;
 
 			listen_interface_t iface;
 			iface.ssl = false;
 
-			if (in[start] == '[')
+			string_view port;
+			if (element.front() == '[')
 			{
-				++start;
+				auto const pos = find_first_of(element, ']', 0);
-				// IPv6 address
+				if (pos == string_view::npos
-				while (start < in.size() && in[start] != ']')
+					|| pos+1 >= element.size()
-					iface.device += in[start++];
+					|| element[pos+1] != ':')
+				{
+					err.emplace_back(element);
+					continue;
+				}
 
-				// skip to the colon
+				iface.device = strip_string(element.substr(1, pos - 1)).to_string();
-				while (start < in.size() && in[start] != ':')
+
-					++start;
+				port = strip_string(element.substr(pos + 2));
 			}
 			else
 			{
 				// consume device name
-				while (start < in.size() && !is_space(in[start]) && in[start] != ':')
+				auto const pos = find_first_of(element, ':', 0);
-					iface.device += in[start++];
+				iface.device = strip_string(element.substr(0, pos)).to_string();
+				if (pos == string_view::npos)
+				{
+					err.emplace_back(element);
+					continue;
+				}
+				port = strip_string(element.substr(pos + 1));
 			}
 
-			// skip spaces
-			while (start < in.size() && is_space(in[start]))
-				++start;
-
-			if (start == in.size() || in[start] != ':') return out;
-			++start; // skip colon
-
-			// skip spaces
-			while (start < in.size() && is_space(in[start]))
-				++start;
-
 			// consume port
-			std::string port;
+			std::string port_str;
-			while (start < in.size() && is_digit(in[start]) && in[start] != ',')
+			for (std::size_t i = 0; i < port.size() && is_digit(port[i]); ++i)
-				port += in[start++];
+				port_str += port[i];
 
-			if (port.empty() || port.size() > 5)
+			if (port_str.empty() || port_str.size() > 5)
 			{
-				iface.port = -1;
+				err.emplace_back(element);
-			}
+				continue;
-			else
-			{
-				iface.port = std::atoi(port.c_str());
-				if (iface.port < 0 || iface.port > 65535) iface.port = -1;
 			}
 
-			// skip spaces
+			iface.port = std::atoi(port_str.c_str());
-			while (start < in.size() && is_space(in[start]))
+			if (iface.port < 0 || iface.port > 65535)
-				++start;
+			{
+				err.emplace_back(element);
+				continue;
+			}
+
+			port.remove_prefix(port_str.size());
+			port = strip_string(port);
 
 			// consume potential SSL 's'
-			if (start < in.size() && in[start] == 's')
+			for (auto const c : port)
 			{
-				iface.ssl = true;
+				switch (c)
-				++start;
+				{
+					case 's': iface.ssl = true; break;
+				}
 			}
 
-			// skip until end or comma
+			TORRENT_ASSERT(iface.port >= 0);
-			while (start < in.size() && in[start] != ',')
+			out.emplace_back(iface);
-				++start;
-
-			if (iface.port >= 0) out.push_back(iface);
-
-			// skip the comma
-			if (start < in.size() && in[start] == ',')
-				++start;
-
 		}
 
 		return out;

test/test_string.cpp

@@ -346,19 +346,23 @@ TORRENT_TEST(path)
 namespace {
 
 void test_parse_interface(char const* input
-	, std::vector<listen_interface_t> expected
+	, std::vector<listen_interface_t> const expected
-	, std::string output)
+	, std::vector<std::string> const expected_e
+	, string_view const output)
 {
 	std::printf("parse interface: %s\n", input);
-	auto const list = parse_listen_interfaces(input);
+	std::vector<std::string> err;
+	auto const list = parse_listen_interfaces(input, err);
 	TEST_EQUAL(list.size(), expected.size());
-	if (list.size() == expected.size())
+	TEST_CHECK(list == expected);
-	{
+	TEST_CHECK(err == expected_e);
-		TEST_CHECK(std::equal(list.begin(), list.end(), expected.begin()
+#if TORRENT_ABI_VERSION == 1 \
-			, [&](listen_interface_t const& lhs, listen_interface_t const& rhs)
+	|| !defined TORRENT_DISABLE_LOGGING
-			{ return lhs.device == rhs.device && lhs.port == rhs.port && lhs.ssl == rhs.ssl; }));
-	}
 	TEST_EQUAL(print_listen_interfaces(list), output);
+	std::cout << "RESULT: " << print_listen_interfaces(list) << '\n';
+#endif
+	for (auto const& e : err)
+		std::cout << "ERR: \"" << e << "\"\n";
 }
 
 } // anonymous namespace
@@ -367,46 +371,68 @@ TORRENT_TEST(parse_list)
 {
 	std::vector<std::string> list;
 	parse_comma_separated_string("  a,b, c, d ,e \t,foobar\n\r,[::1]", list);
-	TEST_EQUAL(list.size(), 7);
+	TEST_CHECK((list == std::vector<std::string>{"a", "b", "c", "d", "e", "foobar", "[::1]"}));
-	TEST_EQUAL(list[0], "a");
+}
-	TEST_EQUAL(list[1], "b");
-	TEST_EQUAL(list[2], "c");
-	TEST_EQUAL(list[3], "d");
-	TEST_EQUAL(list[4], "e");
-	TEST_EQUAL(list[5], "foobar");
-	TEST_EQUAL(list[6], "[::1]");
 
+TORRENT_TEST(parse_interface)
+{
 	test_parse_interface("  a:4,b:35, c : 1000s, d: 351 ,e \t:42,foobar:1337s\n\r,[2001::1]:6881"
 		, {{"a", 4, false}, {"b", 35, false}, {"c", 1000, true}, {"d", 351, false}
 			, {"e", 42, false}, {"foobar", 1337, true}, {"2001::1", 6881, false}}
+		, {}
 		, "a:4,b:35,c:1000s,d:351,e:42,foobar:1337s,[2001::1]:6881");
 
 	// IPv6 address
 	test_parse_interface("[2001:ffff::1]:6882s"
 		, {{"2001:ffff::1", 6882, true}}
+		, {}
 		, "[2001:ffff::1]:6882s");
 
 	// IPv4 address
 	test_parse_interface("127.0.0.1:6882"
 		, {{"127.0.0.1", 6882, false}}
+		, {}
 		, "127.0.0.1:6882");
 
 	// maximum padding
 	test_parse_interface("  nic\r\n:\t 12\r s "
 		, {{"nic", 12, true}}
+		, {}
 		, "nic:12s");
 
 	// negative tests
-	test_parse_interface("nic:99999999999999999999999", {}, "");
+	test_parse_interface("nic:99999999999999999999999", {}, {"nic:99999999999999999999999"}, "");
-	test_parse_interface("nic:  -3", {}, "");
+	test_parse_interface("nic:  -3", {}, {"nic:  -3"}, "");
-	test_parse_interface("nic:  ", {}, "");
+	test_parse_interface("nic:  ", {}, {"nic:"}, "");
-	test_parse_interface("nic :", {}, "");
+	test_parse_interface("nic :", {}, {"nic :"}, "");
-	test_parse_interface("nic ", {}, "");
+	test_parse_interface("nic ", {}, {"nic"}, "");
-	test_parse_interface("nic s", {}, "");
+	test_parse_interface("nic s", {}, {"nic s"}, "");
 
 	// parse interface with port 0
-	test_parse_interface("127.0.0.1:0"
+	test_parse_interface("127.0.0.1:0", {{"127.0.0.1", 0, false}}, {}, "127.0.0.1:0");
-		, {{"127.0.0.1", 0, false}}, "127.0.0.1:0");
+
+	// IPv6 error
+	test_parse_interface("[aaaa::1", {}, {"[aaaa::1"}, "");
+	test_parse_interface("[aaaa::1]", {}, {"[aaaa::1]"}, "");
+	test_parse_interface("[aaaa::1]:", {}, {"[aaaa::1]:"}, "");
+	test_parse_interface("[aaaa::1]:s", {}, {"[aaaa::1]:s"}, "");
+	test_parse_interface("[aaaa::1] :6881", {}, {"[aaaa::1] :6881"}, "");
+	test_parse_interface("[aaaa::1]:6881", {{"aaaa::1", 6881, false}}, {}, "[aaaa::1]:6881");
+
+	// unterminated [
+	test_parse_interface("[aaaa::1,foobar:0", {{"foobar", 0, false}}, {"[aaaa::1"}, "foobar:0");
+
+	// multiple errors
+	test_parse_interface("foo:,bar", {}, {"foo:", "bar"}, "");
+
+	// quoted elements
+	test_parse_interface("\"abc,.\",bar", {}, {"abc,.", "bar"}, "");
+
+	// silent error
+	test_parse_interface("\"", {}, {"\""}, "");
+
+	// multiple errors and one correct
+	test_parse_interface("foo,bar,0.0.0.0:6881", {{"0.0.0.0", 6881, false}}, {"foo", "bar"}, "0.0.0.0:6881");
 }
 
 TORRENT_TEST(split_string)
@@ -508,3 +534,15 @@ TORRENT_TEST(find_first_of)
 	TEST_EQUAL(find_first_of(test, "61", 3), 6);
 	TEST_EQUAL(find_first_of(test, "61", 4), 6);
 }
+
+TORRENT_TEST(strip_string)
+{
+	TEST_EQUAL(strip_string("   ab"), "ab");
+	TEST_EQUAL(strip_string("   ab    "), "ab");
+	TEST_EQUAL(strip_string("       "), "");
+	TEST_EQUAL(strip_string(""), "");
+	TEST_EQUAL(strip_string("a     b"), "a     b");
+	TEST_EQUAL(strip_string("   a     b   "), "a     b");
+	TEST_EQUAL(strip_string(" \t \t ab\t\t\t"), "ab");
+}
+