make parse_interface_list more robust and improve string tests (#806)

make parse_interface_list more robust and improve string tests

simulation/test_transfer.cpp

@@ -54,6 +54,18 @@ namespace lt = libtorrent;
 
 const int connect_socks = 2;
 
+std::string make_ep_string(char const* address, bool const is_v6
+	, char const* port)
+{
+	std::string ret;
+	if (is_v6) ret += '[';
+	ret += address;
+	if (is_v6) ret += ']';
+	ret += ':';
+	ret += port;
+	return ret;
+}
+
 template <typename Setup, typename HandleAlerts, typename Test>
 void run_test(
 	Setup const& setup
@@ -98,13 +110,13 @@ void run_test(
 	pack.set_int(settings_pack::out_enc_policy, settings_pack::pe_disabled);
 	pack.set_int(settings_pack::allowed_enc_level, settings_pack::pe_plaintext);
 
-	pack.set_str(settings_pack::listen_interfaces, peer0_ip[use_ipv6] + std::string(":6881"));
+	pack.set_str(settings_pack::listen_interfaces, make_ep_string(peer0_ip[use_ipv6], use_ipv6, "6881"));
 
 	// create session
 	std::shared_ptr<lt::session> ses[2];
 	ses[0] = std::make_shared<lt::session>(pack, ios0);
 
-	pack.set_str(settings_pack::listen_interfaces, peer1_ip[use_ipv6] + std::string(":6881"));
+	pack.set_str(settings_pack::listen_interfaces, make_ep_string(peer1_ip[use_ipv6], use_ipv6, "6881"));
 	ses[1] = std::make_shared<lt::session>(pack, ios1);
 
 	setup(*ses[0], *ses[1]);

src/string_util.cpp

@@ -214,44 +214,89 @@ namespace libtorrent
 		out.clear();
 
 		std::string::size_type start = 0;
-		std::string::size_type end = 0;
 
 		while (start < in.size())
 		{
 			// skip leading spaces
-			while (start < in.size()
+			while (start < in.size() && is_space(in[start]))
-				&& is_space(in[start]))
 				++start;
 
-			end = in.find_first_of(',', start);
+			if (start == in.size()) return;
-			if (end == std::string::npos) end = in.size();
 
-			std::string::size_type colon = in.find_last_of(':', end);
-
-			if (colon != std::string::npos && colon > start)
-			{
 			listen_interface_t iface;
+			iface.ssl = false;
 
-				std::string port_string = in.substr(colon + 1, end - colon - 1);
+#if !TORRENT_USE_IPV6
-				iface.ssl = !port_string.empty() && port_string[port_string.size()-1] == 's';
+			bool ipv6 = false;
-				iface.port = atoi(port_string.c_str());
+#endif
+			if (in[start] == '[')
+			{
+#if !TORRENT_USE_IPV6
+				ipv6 = true;
+#endif
+				++start;
+				// IPv6 address
+				while (start < in.size() && in[start] != ']')
+					iface.device += in[start++];
 
-				// skip trailing spaces
+				// skip to the colon
-				std::string::size_type soft_end = colon;
+				while (start < in.size() && in[start] != ':')
-				while (soft_end > start
+					++start;
-					&& is_space(in[soft_end-1]))
+			}
-					--soft_end;
+			else
+			{
+				// consume device name
+				while (start < in.size() && !is_space(in[start]) && in[start] != ':')
+					iface.device += in[start++];
+			}
 
-				// in case this is an IPv6 address, strip off the square brackets
+			// skip spaces
-				// to make it more easily parseable into an ip::address
+			while (start < in.size() && is_space(in[start]))
-				if (in[start] == '[') ++start;
+				++start;
-				if (soft_end > start && in[soft_end-1] == ']') --soft_end;
 
-				iface.device = in.substr(start, soft_end - start);
+			if (start == in.size() || in[start] != ':') return;
+			++start; // skip colon
+
+			// skip spaces
+			while (start < in.size() && is_space(in[start]))
+				++start;
+
+			// consume port
+			std::string port;
+			while (start < in.size() && is_digit(in[start]) && in[start] != ',')
+				port += in[start++];
+
+			if (port.empty()) iface.port = -1;
+			else iface.port = std::atoi(port.c_str());
+
+			// skip spaces
+			while (start < in.size() && is_space(in[start]))
+				++start;
+
+			// consume potential SSL 's'
+			if (start < in.size() && in[start] == 's')
+			{
+				iface.ssl = true;
+				++start;
+			}
+
+			// skip until end or comma
+			while (start < in.size() && in[start] != ',')
+				++start;
+
+			if (iface.port >= 0
+#if !TORRENT_USE_IPV6
+				&& ipv6 == false
+#endif
+				)
+			{
 				out.push_back(iface);
 			}
 
-			start = end + 1;
+			// skip the comma
+			if (start < in.size() && in[start] == ',')
+				++start;
+
 		}
 	}
 

test/Jamfile

@@ -125,7 +125,6 @@ test-suite libtorrent :
 		test_sliding_average.cpp
 		test_socket_io.cpp
 #		test_random.cpp
-		test_utf8.cpp
 		test_gzip.cpp
 		test_bitfield.cpp
 		test_part_file.cpp
@@ -142,7 +141,6 @@ test-suite libtorrent :
 		test_bencoding.cpp
 		test_bdecode.cpp
 		test_http_parser.cpp
-		test_string.cpp
 		test_xml.cpp
 		test_ip_filter.cpp
 		test_hasher.cpp
@@ -158,6 +156,10 @@ test-suite libtorrent :
 		test_linked_list.cpp
 		test_file_progress.cpp ]
 
+	[ run test_string.cpp
+		test_utf8.cpp
+		]
+
 	[ run test_receive_buffer.cpp ]
 	[ run test_alert_manager.cpp ]
 	[ run test_direct_dht.cpp ]

test/test_string.cpp

@@ -39,7 +39,7 @@ POSSIBILITY OF SUCH DAMAGE.
 
 using namespace libtorrent;
 
-TORRENT_TEST(string)
+TORRENT_TEST(maybe_url_encode)
 {
 	// test maybe_url_encode
 	TEST_EQUAL(maybe_url_encode("http://test:test@abc.com/abc<>abc"), "http://test:test@abc.com/abc%3c%3eabc");
@@ -48,10 +48,11 @@ TORRENT_TEST(string)
 	TEST_EQUAL(maybe_url_encode("http://abc.com:8080/foo bar"), "http://abc.com:8080/foo%20bar");
 	TEST_EQUAL(maybe_url_encode("abc"), "abc");
 	TEST_EQUAL(maybe_url_encode("http://abc.com/abc"), "http://abc.com/abc");
+}
 
-	// test to/from hex conversion
+TORRENT_TEST(hex)
-
+{
-	char const* str = "0123456789012345678901234567890123456789";
+	static char const str[] = "0123456789012345678901234567890123456789";
 	char bin[20];
 	TEST_CHECK(aux::from_hex(str, 40, bin));
 	char hex[41];
@@ -61,8 +62,26 @@ TORRENT_TEST(string)
 	TEST_CHECK(aux::to_hex("\x55\x73") == "5573");
 	TEST_CHECK(aux::to_hex("\xaB\xd0") == "abd0");
 
-	// test is_space
+	static char const hex_chars[] = "0123456789abcdefABCDEF";
 
+	for (int i = 1; i < 255; ++i)
+	{
+		bool hex = strchr(hex_chars, i) != NULL;
+		char c = i;
+		TEST_EQUAL(aux::is_hex(&c, 1), hex);
+	}
+
+	TEST_EQUAL(aux::hex_to_int('0'), 0);
+	TEST_EQUAL(aux::hex_to_int('7'), 7);
+	TEST_EQUAL(aux::hex_to_int('a'), 10);
+	TEST_EQUAL(aux::hex_to_int('f'), 15);
+	TEST_EQUAL(aux::hex_to_int('b'), 11);
+	TEST_EQUAL(aux::hex_to_int('t'), -1);
+	TEST_EQUAL(aux::hex_to_int('g'), -1);
+}
+
+TORRENT_TEST(is_space)
+{
 	TEST_CHECK(!is_space('C'));
 	TEST_CHECK(!is_space('\b'));
 	TEST_CHECK(!is_space('8'));
@@ -71,35 +90,38 @@ TORRENT_TEST(string)
 	TEST_CHECK(is_space('\t'));
 	TEST_CHECK(is_space('\n'));
 	TEST_CHECK(is_space('\r'));
+}
 
-	// test to_lower
+TORRENT_TEST(to_lower)
-
+{
 	TEST_CHECK(to_lower('C') == 'c');
 	TEST_CHECK(to_lower('c') == 'c');
 	TEST_CHECK(to_lower('-') == '-');
 	TEST_CHECK(to_lower('&') == '&');
+}
 
-	// test string_equal_no_case
+TORRENT_TEST(string_equal_no_case)
-
+{
 	TEST_CHECK(string_equal_no_case("foobar", "FoobAR"));
 	TEST_CHECK(string_equal_no_case("foobar", "foobar"));
 	TEST_CHECK(!string_equal_no_case("foobar", "foobar "));
 	TEST_CHECK(!string_equal_no_case("foobar", "F00"));
 
-	// test string_begins_no_case
-
 	TEST_CHECK(string_begins_no_case("foobar", "FoobAR --"));
 	TEST_CHECK(!string_begins_no_case("foobar", "F00"));
+}
 
-	// test itoa
+TORRENT_TEST(to_string)
-
+{
 	TEST_CHECK(to_string(345).data() == std::string("345"));
 	TEST_CHECK(to_string(-345).data() == std::string("-345"));
 	TEST_CHECK(to_string(0).data() == std::string("0"));
 	TEST_CHECK(to_string(1000000000).data() == std::string("1000000000"));
+}
 
+TORRENT_TEST(base64)
+{
 	// base64 test vectors from http://www.faqs.org/rfcs/rfc4648.html
-
 	TEST_CHECK(base64encode("") == "");
 	TEST_CHECK(base64encode("f") == "Zg==");
 	TEST_CHECK(base64encode("fo") == "Zm8=");
@@ -107,7 +129,10 @@ TORRENT_TEST(string)
 	TEST_CHECK(base64encode("foob") == "Zm9vYg==");
 	TEST_CHECK(base64encode("fooba") == "Zm9vYmE=");
 	TEST_CHECK(base64encode("foobar") == "Zm9vYmFy");
+}
 
+TORRENT_TEST(base32)
+{
 	// base32 test vectors from http://www.faqs.org/rfcs/rfc4648.html
 
 	TEST_CHECK(base32encode("") == "");
@@ -144,7 +169,10 @@ TORRENT_TEST(string)
 		test += char(i);
 
 	TEST_CHECK(base32decode(base32encode(test)) == test);
+}
 
+TORRENT_TEST(escape_string)
+{
 	// escape_string
 	char const* test_string = "!@#$%^&*()-_=+/,. %?";
 	TEST_EQUAL(escape_string(test_string, int(strlen(test_string)))
@@ -189,40 +217,20 @@ TORRENT_TEST(string)
 	ec.clear();
 
 	TEST_CHECK(unescape_string("123+abc", ec) == "123 abc");
+}
 
-
+TORRENT_TEST(read_until)
-	// read_until
+{
-
 	char const* test_string1 = "abcdesdf sdgf";
 	char const* tmp1 = test_string1;
 	TEST_CHECK(read_until(tmp1, 'd', test_string1 + strlen(test_string1)) == "abc");
 
 	tmp1 = test_string1;
 	TEST_CHECK(read_until(tmp1, '[', test_string1 + strlen(test_string1)) == "abcdesdf sdgf");
-
-	char hex_chars[] = "0123456789abcdefABCDEF";
-
-	for (int i = 1; i < 255; ++i)
-	{
-		bool hex = strchr(hex_chars, i) != NULL;
-		char c = i;
-		TEST_EQUAL(aux::is_hex(&c, 1), hex);
 }
 
-	TEST_EQUAL(aux::hex_to_int('0'), 0);
+TORRENT_TEST(url_has_argument)
-	TEST_EQUAL(aux::hex_to_int('7'), 7);
+{
-	TEST_EQUAL(aux::hex_to_int('a'), 10);
-	TEST_EQUAL(aux::hex_to_int('f'), 15);
-	TEST_EQUAL(aux::hex_to_int('b'), 11);
-	TEST_EQUAL(aux::hex_to_int('t'), -1);
-	TEST_EQUAL(aux::hex_to_int('g'), -1);
-
-	std::string path = "a\\b\\c";
-	convert_path_to_posix(path);
-	TEST_EQUAL(path, "a/b/c");
-
-	// url_has_argument
-
 	TEST_CHECK(url_has_argument("http://127.0.0.1/test", "test") == "");
 	TEST_CHECK(url_has_argument("http://127.0.0.1/test?foo=24", "bar") == "");
 	TEST_CHECK(url_has_argument("http://127.0.0.1/test?foo=24", "foo") == "24");
@@ -231,6 +239,13 @@ TORRENT_TEST(string)
 	TEST_CHECK(url_has_argument("http://127.0.0.1/test?foo=24&bar=23&a=e", "bar") == "23");
 	TEST_CHECK(url_has_argument("http://127.0.0.1/test?foo=24&bar=23&a=e", "a") == "e");
 	TEST_CHECK(url_has_argument("http://127.0.0.1/test?foo=24&bar=23&a=e", "b") == "");
+}
+
+TORRENT_TEST(path)
+{
+	std::string path = "a\\b\\c";
+	convert_path_to_posix(path);
+	TEST_EQUAL(path, "a/b/c");
 
 	// resolve_file_url
 
@@ -245,7 +260,27 @@ TORRENT_TEST(string)
 	TEST_EQUAL(resolve_file_url("file:///c/blah/foo/bar"), "/c/blah/foo/bar");
 	TEST_EQUAL(resolve_file_url("file:///c/b%3fah/foo/bar"), "/c/b?ah/foo/bar");
 #endif
+}
 
+void test_parse_interface(char const* input
+	, std::vector<listen_interface_t> expected
+	, std::string output)
+{
+	std::fprintf(stderr, "parse interface: %s\n", input);
+	std::vector<listen_interface_t> list;
+	parse_listen_interfaces(input, list);
+	TEST_EQUAL(list.size(), expected.size());
+	if (list.size() == expected.size())
+	{
+		TEST_CHECK(std::equal(list.begin(), list.end(), expected.begin()
+			, [&](listen_interface_t const& lhs, listen_interface_t const& rhs)
+			{ return lhs.device == rhs.device && lhs.port == rhs.port && lhs.ssl == rhs.ssl; }));
+	}
+	TEST_EQUAL(print_listen_interfaces(list), output);
+}
+
+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);
@@ -257,37 +292,48 @@ TORRENT_TEST(string)
 	TEST_EQUAL(list[5], "foobar");
 	TEST_EQUAL(list[6], "[::1]");
 
-	std::vector<listen_interface_t> list2;
+#if TORRENT_USE_IPV6
-	parse_listen_interfaces("  a:4,b:35, c : 1000s, d: 351 ,e \t:42,foobar:1337s\n\r,[2001::1]:6881", list2);
+	test_parse_interface("  a:4,b:35, c : 1000s, d: 351 ,e \t:42,foobar:1337s\n\r,[2001::1]:6881"
-	TEST_EQUAL(list2.size(), 7);
+		, {{"a", 4, false}, {"b", 35, false}, {"c", 1000, true}, {"d", 351, false}
-	TEST_EQUAL(list2[0].device, "a");
+			, {"e", 42, false}, {"foobar", 1337, true}, {"2001::1", 6881, false}}
-	TEST_EQUAL(list2[1].device, "b");
+		, "a:4,b:35,c:1000s,d:351,e:42,foobar:1337s,[2001::1]:6881");
-	TEST_EQUAL(list2[2].device, "c");
+#else
-	TEST_EQUAL(list2[3].device, "d");
+	test_parse_interface("  a:4,b:35, c : 1000s, d: 351 ,e \t:42,foobar:1337s\n\r,[2001::1]:6881"
-	TEST_EQUAL(list2[4].device, "e");
+		, {{"a", 4, false}, {"b", 35, false}, {"c", 1000, true}, {"d", 351, false}
-	TEST_EQUAL(list2[5].device, "foobar");
+			, {"e", 42, false}, {"foobar", 1337, true}}
-	TEST_EQUAL(list2[6].device, "2001::1");
+		, "a:4,b:35,c:1000s,d:351,e:42,foobar:1337s");
+#endif
 
-	TEST_EQUAL(list2[0].port, 4);
+	// IPv6 address
-	TEST_EQUAL(list2[1].port, 35);
+#if TORRENT_USE_IPV6
-	TEST_EQUAL(list2[2].port, 1000);
+	test_parse_interface("[2001:ffff::1]:6882s"
-	TEST_EQUAL(list2[3].port, 351);
+		, {{"2001:ffff::1", 6882, true}}
-	TEST_EQUAL(list2[4].port, 42);
+		, "[2001:ffff::1]:6882s");
-	TEST_EQUAL(list2[5].port, 1337);
+#else
-	TEST_EQUAL(list2[6].port, 6881);
+	test_parse_interface("[2001:ffff::1]:6882s", {}, "");
+#endif
 
-	TEST_EQUAL(list2[0].ssl, false);
+	// IPv4 address
-	TEST_EQUAL(list2[1].ssl, false);
+	test_parse_interface("127.0.0.1:6882"
-	TEST_EQUAL(list2[2].ssl, true);
+		, {{"127.0.0.1", 6882, false}}
-	TEST_EQUAL(list2[3].ssl, false);
+		, "127.0.0.1:6882");
-	TEST_EQUAL(list2[4].ssl, false);
-	TEST_EQUAL(list2[5].ssl, true);
-	TEST_EQUAL(list2[6].ssl, false);
 
-	TEST_EQUAL(print_listen_interfaces(list2), "a:4,b:35,c:1000s,d:351,e:42,foobar:1337s,[2001::1]:6881");
+	// maximum padding
+	test_parse_interface("  nic\r\n:\t 12\r s "
+		, {{"nic", 12, true}}
+		, "nic:12s");
 
-	// test string_tokenize
+	// negative tests
+	test_parse_interface("nic:99999999999999999999999", {}, "");
+	test_parse_interface("nic:  -3", {}, "");
+	test_parse_interface("nic:  ", {}, "");
+	test_parse_interface("nic :", {}, "");
+	test_parse_interface("nic ", {}, "");
+	test_parse_interface("nic s", {}, "");
+}
 
+TORRENT_TEST(tokenize)
+{
 	char test_tokenize[] = "a b c \"foo bar\" d\ne f";
 	char* next = test_tokenize;
 	char* ptr = string_tokenize(next, ' ', &next);