fix semantics of socks5_stream and http_connection to make them not close themselves. Add socks5 and socks4 proxy coverage in test_http_connection

include/libtorrent/http_connection.hpp

@@ -146,7 +146,7 @@ private:
 		, error_code const& e);
 	void on_assign_bandwidth(error_code const& e);
 
-	void callback(error_code e, char* data = 0, int size = 0);
+	void callback(error_code e, char* data = NULL, int size = 0);
 
 	std::vector<char> m_recvbuffer;
 

include/libtorrent/socks5_stream.hpp

@@ -118,7 +118,6 @@ public:
 
 	void close(error_code& ec)
 	{
-		m_hostname.clear();
 		m_dst_name.clear();
 		proxy_base::close(ec);
 	}
@@ -126,7 +125,6 @@ public:
 #ifndef BOOST_NO_EXCEPTIONS
 	void close()
 	{
-		m_hostname.clear();
 		m_dst_name.clear();
 		proxy_base::close();
 	}

simulation/test_http_connection.cpp

@@ -36,6 +36,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "swarm_config.hpp"
 #include "simulator/simulator.hpp"
 #include "simulator/http_server.hpp"
+#include "simulator/socks_server.hpp"
 #include "libtorrent/alert_types.hpp"
 #include "libtorrent/aux_/proxy_settings.hpp"
 #include "libtorrent/http_connection.hpp"
@@ -110,7 +111,7 @@ boost::shared_ptr<http_connection> test_request(io_service& ios
 	, int* handler_called
 	, std::string const& auth = std::string())
 {
-	std::cerr << " ===== TESTING: " << url << " =====" << std::endl;
+	fprintf(stderr, " ===== TESTING: %s =====\n", url.c_str());
 
 	auto h = boost::make_shared<http_connection>(ios
 		, res
@@ -166,7 +167,7 @@ void print_http_header(std::map<std::string, std::string> const& headers)
 	}
 }
 
-void run_test(std::string url, int expect_size, int expect_status
+void run_test(settings_pack::proxy_type_t proxy_type, std::string url, int expect_size, int expect_status
 	, boost::system::error_code expect_error, std::vector<int> expect_counters);
 
 enum expect_counters
@@ -183,24 +184,31 @@ enum expect_counters
 	num_counters
 };
 
-TORRENT_TEST(http_connection)
+void run_suite(settings_pack::proxy_type_t proxy_type)
 {
 	std::string url_base = "http://10.0.0.2:8080";
 
-	run_test(url_base + "/non-existent", 0, 404, error_code(), { 1, 1 });
+	run_test(proxy_type, url_base + "/test_file", 1337, 200, error_code(), { 1, 1, 1});
-	run_test(url_base + "/test_file", 1337, 200, error_code(), { 1, 1, 1});
+
-	run_test(url_base + "/redirect", 1337, 200, error_code(), { 2, 1, 1, 1 });
+	run_test(proxy_type, url_base + "/non-existent", 0, 404, error_code(), { 1, 1 });
-	run_test(url_base + "/relative/redirect", 1337, 200, error_code(), {2, 1, 1, 0, 1});
+	run_test(proxy_type, url_base + "/redirect", 1337, 200, error_code(), { 2, 1, 1, 1 });
-	run_test(url_base + "/infinite/redirect", 0, 301, error_code(asio::error::eof), {6, 1, 0, 0, 0, 6});
+	run_test(proxy_type, url_base + "/relative/redirect", 1337, 200, error_code(), {2, 1, 1, 0, 1});
-	run_test(url_base + "/chunked_encoding", 1337, 200, error_code(), { 1, 1, 0, 0, 0, 0, 1});
+	run_test(proxy_type, url_base + "/infinite/redirect", 0, 301, error_code(asio::error::eof), {6, 1, 0, 0, 0, 6});
+	run_test(proxy_type, url_base + "/chunked_encoding", 1337, 200, error_code(), { 1, 1, 0, 0, 0, 0, 1});
 
 	// we are on an IPv4 host, we can't connect to IPv6 addresses, make sure that
 	// error is correctly propagated
-	run_test("http://[ff::dead:beef]:8080/test_file", 0, -1, error_code(asio::error::address_family_not_supported)
+	// with socks5 we would be able to do this, assuming the socks server
+	// supported it, but the current socks implementation in libsimulator does
+	// not support IPv6
+	if (proxy_type != settings_pack::socks5)
+	{
+		run_test(proxy_type, "http://[ff::dead:beef]:8080/test_file", 0, -1, error_code(asio::error::address_family_not_supported)
 			, {0,1});
+	}
 
 	// there is no node at 10.0.0.10, this should fail with connection refused
-	run_test("http://10.0.0.10:8080/test_file", 0, -1,
+	run_test(proxy_type, "http://10.0.0.10:8080/test_file", 0, -1,
 		error_code(boost::system::errc::connection_refused, boost::system::system_category())
 		, {0,1});
 
@@ -208,23 +216,20 @@ TORRENT_TEST(http_connection)
 	// connect to and the second one where we'll get the test file response. Make
 	// sure the http_connection correcly tries the second IP if the first one
 	// fails.
-	run_test("http://try-next.com:8080/test_file", 1337, 200, error_code(), { 1, 1, 1});
+	run_test(proxy_type, "http://try-next.com:8080/test_file", 1337, 200, error_code(), { 1, 1, 1});
 
 	// make sure hostname lookup failures are passed through correctly
-	run_test("http://non-existent.com/test_file", 0, -1, asio::error::host_not_found, { 0, 1});
+	run_test(proxy_type, "http://non-existent.com/test_file", 0, -1, asio::error::host_not_found, { 0, 1});
 
 	// make sure we handle gzipped content correctly
-	run_test(url_base + "/test_file.gz", 1337, 200, error_code(), { 1, 1, 0, 0, 0, 0, 0, 1});
+	run_test(proxy_type, url_base + "/test_file.gz", 1337, 200, error_code(), { 1, 1, 0, 0, 0, 0, 0, 1});
 
-//	run_test(url_base + "/password_protected", 1337, 200, error_code(), { 1, 1, 1});
+// TODO: 2 test basic-auth
-
+// TODO: 2 test https
-//#error test all proxies
-//#error test https
-//#error test chunked encoding
 
 }
 
-void run_test(std::string url, int expect_size, int expect_status
+void run_test(settings_pack::proxy_type_t proxy_type, std::string url, int expect_size, int expect_status
 	, boost::system::error_code expect_error, std::vector<int> expect_counters)
 {
 	using sim::asio::ip::address_v4;
@@ -236,11 +241,23 @@ void run_test(std::string url, int expect_size, int expect_status
 
 	sim::asio::io_service web_server(sim, address_v4::from_string("10.0.0.2"));
 	sim::asio::io_service ios(sim, address_v4::from_string("10.0.0.1"));
-	sim::asio::io_service ipv6_host(sim, address_v6::from_string("ff::dead:beef"));
+	sim::asio::io_service proxy_ios(sim, address_v4::from_string("50.50.50.50"));
 	lt::resolver res(ios);
 
 	sim::http_server http(web_server, 8080);
-	sim::http_server http_v6(ipv6_host, 8080);
+	sim::socks_server socks(proxy_ios, 4444, proxy_type == settings_pack::socks4 ? 4 : 5);
+
+	lt::aux::proxy_settings ps;
+	if (proxy_type != settings_pack::none)
+	{
+		ps.hostname = "50.50.50.50";
+		ps.port = 4444;
+		ps.username = "testuser";
+		ps.password = "testpass";
+		ps.type = proxy_type;
+		ps.proxy_hostnames = false;
+		// TODO: 2 also test proxying host names, and verify they are in fact proxied
+	}
 
 	char data_buffer[4000];
 	std::generate(data_buffer, data_buffer + sizeof(data_buffer), &std::rand);
@@ -336,12 +353,6 @@ void run_test(std::string url, int expect_size, int expect_status
 			"\r\n";
 	});
 
-	lt::aux::proxy_settings ps;
-	ps.hostname = "127.0.0.1";
-	ps.username = "testuser";
-	ps.password = "testpass";
-	ps.type = settings_pack::none;
-
 	auto c = test_request(ios, res, url, data_buffer, expect_size
 		, expect_status, expect_error, ps, &counters[connect_handler]
 		, &counters[handler]);
@@ -360,3 +371,67 @@ void run_test(std::string url, int expect_size, int expect_status
 	}
 }
 
+TORRENT_TEST(http_connection)
+{
+	run_suite(settings_pack::none);
+}
+
+TORRENT_TEST(http_connection_socks4)
+{
+	run_suite(settings_pack::socks4);
+}
+
+TORRENT_TEST(http_connection_socks5)
+{
+	run_suite(settings_pack::socks5);
+}
+
+TORRENT_TEST(http_connection_socks_error)
+{
+	// if we set up to user a proxy that does not exist, expect failure!
+	using sim::asio::ip::address_v4;
+	sim_config network_cfg;
+	sim::simulation sim{network_cfg};
+
+	sim::asio::io_service web_server(sim, address_v4::from_string("10.0.0.2"));
+	sim::asio::io_service ios(sim, address_v4::from_string("10.0.0.1"));
+	lt::resolver res(ios);
+
+	sim::http_server http(web_server, 8080);
+
+	lt::aux::proxy_settings ps;
+	ps.hostname = "50.50.50.50";
+	ps.port = 4444;
+	ps.username = "testuser";
+	ps.password = "testpass";
+	ps.type = settings_pack::socks5;
+
+	char data_buffer[4000];
+	std::generate(data_buffer, data_buffer + sizeof(data_buffer), &std::rand);
+
+	http.register_handler("/test_file"
+		, [data_buffer](std::string method, std::string req
+		, std::map<std::string, std::string>& headers)
+	{
+		print_http_header(headers);
+		TEST_CHECK(false && "we're not supposed to get here!");
+		return sim::send_response(200, "OK", 1337).append(data_buffer, 1337);
+	});
+
+	int connect_counter = 0;
+	int handler_counter = 0;
+	auto c = test_request(ios, res, "http://10.0.0.2:8080/test_file"
+		, data_buffer, -1, -1
+		, error_code(boost::system::errc::connection_refused, boost::system::system_category())
+		, ps, &connect_counter, &handler_counter);
+
+	error_code e;
+	sim.run(e);
+
+	if (e) std::cerr << " run failed: " << e.message() << std::endl;
+	TEST_EQUAL(e, error_code());
+}
+
+// TODO: test http proxy
+// TODO: test socks5 with password
+

src/http_connection.cpp

@@ -395,6 +395,9 @@ void http_connection::start(std::string const& hostname, int port
 		}
 		else
 #endif
+
+		// TODO: 3 if hostname is in fact an IP address (v4 or v6), we should go
+		// straight to connecting, regardless of proxy_hostname is enabled or not
 		if (ps && ps->proxy_hostnames
 			&& (ps->type == settings_pack::socks5
 				|| ps->type == settings_pack::socks5_pw))
@@ -450,7 +453,6 @@ void http_connection::on_timeout(boost::weak_ptr<http_connection> p
 		else
 		{
 			c->callback(boost::asio::error::timed_out);
-			c->close(true);
 		}
 		return;
 	}
@@ -519,7 +521,6 @@ void http_connection::on_i2p_resolve(error_code const& e
 	if (e)
 	{
 		callback(e);
-		close();
 		return;
 	}
 	connect_i2p_tracker(destination);
@@ -534,10 +535,7 @@ void http_connection::on_resolve(error_code const& e
 #endif
 	if (e)
 	{
-		boost::shared_ptr<http_connection> me(shared_from_this());
-
 		callback(e);
-		close();
 		return;
 	}
 	TORRENT_ASSERT(!addresses.empty());
@@ -640,9 +638,7 @@ void http_connection::on_connect(error_code const& e)
 	}
 	else
 	{
-		boost::shared_ptr<http_connection> me(shared_from_this());
 		callback(e);
-		close();
 	}
 }
 
@@ -664,7 +660,6 @@ void http_connection::callback(error_code e, char* data, int size)
 			if (ec)
 			{
 				if (m_handler) m_handler(ec, m_parser, data, size, *this);
-				close();
 				return;
 			}
 			size = int(buf.size());
@@ -692,9 +687,7 @@ void http_connection::on_write(error_code const& e)
 
 	if (e)
 	{
-		boost::shared_ptr<http_connection> me(shared_from_this());
 		callback(e);
-		close();
 		return;
 	}
 
@@ -763,7 +756,6 @@ void http_connection::on_read(error_code const& e
 			size = m_parser.get_body().left();
 		}
 		callback(ec, data, size);
-		close();
 		return;
 	}
 
@@ -771,7 +763,6 @@ void http_connection::on_read(error_code const& e
 	{
 		TORRENT_ASSERT(bytes_transferred == 0);
 		callback(e);
-		close();
 		return;
 	}
 
@@ -805,7 +796,6 @@ void http_connection::on_read(error_code const& e
 				{
 					// missing location header
 					callback(error_code(errors::http_missing_location));
-					close();
 					return;
 				}
 
@@ -861,7 +851,6 @@ void http_connection::on_read(error_code const& e
 		// if we've reached the size limit, terminate the connection and
 		// report the error
 		callback(error_code(boost::system::errc::file_too_large, generic_category()));
-		close();
 		return;
 	}
 	int amount_to_read = m_recvbuffer.size() - m_read_pos;

src/socks5_stream.cpp

@@ -141,8 +141,6 @@ namespace libtorrent
 		else
 		{
 			(*h)(socks_error::unsupported_version);
-			error_code ec;
-			close(ec);
 		}
 	}
 
@@ -177,8 +175,6 @@ namespace libtorrent
 		if (version < m_version)
 		{
 			(*h)(socks_error::unsupported_version);
-			error_code ec;
-			close(ec);
 			return;
 		}
 
@@ -191,8 +187,6 @@ namespace libtorrent
 			if (m_user.empty())
 			{
 				(*h)(socks_error::username_required);
-				error_code ec;
-				close(ec);
 				return;
 			}
 
@@ -214,8 +208,6 @@ namespace libtorrent
 		else
 		{
 			(*h)(socks_error::unsupported_authentication_method);
-			error_code ec;
-			close(ec);
 			return;
 		}
 	}
@@ -253,16 +245,12 @@ namespace libtorrent
 		if (version != 1)
 		{
 			(*h)(socks_error::unsupported_authentication_version);
-			error_code ec;
-			close(ec);
 			return;
 		}
 
 		if (status != 0)
 		{
 			(*h)(socks_error::authentication_error);
-			error_code ec;
-			close(ec);
 			return;
 		}
 
@@ -309,8 +297,6 @@ namespace libtorrent
 			if (!m_remote_endpoint.address().is_v4())
 			{
 				(*h)(boost::asio::error::address_family_not_supported);
-				error_code ec;
-				close(ec);
 				return;
 			}
 			m_buffer.resize(m_user.size() + 9);
@@ -326,8 +312,6 @@ namespace libtorrent
 		else
 		{
 			(*h)(socks_error::unsupported_version);
-			error_code ec;
-			close(ec);
 			return;
 		}
 
@@ -376,8 +360,6 @@ namespace libtorrent
 			if (version < m_version)
 			{
 				(*h)(socks_error::unsupported_version);
-				error_code ec;
-				close(ec);
 				return;
 			}
 			if (response != 0)
@@ -394,7 +376,6 @@ namespace libtorrent
 					case 8: ec = boost::asio::error::address_family_not_supported; break;
 				}
 				(*h)(ec);
-				close(ec);
 				return;
 			}
 			p += 1; // reserved
@@ -437,8 +418,6 @@ namespace libtorrent
 			else
 			{
 				(*h)(boost::asio::error::address_family_not_supported);
-				error_code ec;
-				close(ec);
 				return;
 			}
 			m_buffer.resize(m_buffer.size() + extra_bytes);
@@ -455,8 +434,6 @@ namespace libtorrent
 			if (version != 0)
 			{
 				(*h)(socks_error::general_failure);
-				error_code ec;
-				close(ec);
 				return;
 			}
 
@@ -487,16 +464,14 @@ namespace libtorrent
 				return;
 			}
 
-			int code = socks_error::general_failure;
+			error_code ec(socks_error::general_failure, get_socks_category());
 			switch (response)
 			{
-				case 91: code = socks_error::authentication_error; break;
+				case 91: ec = boost::asio::error::connection_refused; break;
-				case 92: code = socks_error::no_identd; break;
+				case 92: ec = socks_error::no_identd; break;
-				case 93: code = socks_error::identd_error; break;
+				case 93: ec = socks_error::identd_error; break;
 			}
-			error_code ec(code, get_socks_category());
 			(*h)(ec);
-			close(ec);
 		}
 	}
 

test/main.cpp

@@ -310,6 +310,10 @@ EXPORT int main(int argc, char const* argv[])
 			}
 		}
 
+		// get proper interleaving of stderr and stdout
+		setbuf(stdout, NULL);
+		setbuf(stderr, NULL);
+
 		_g_test_idx = i;
 		current_test = &t;