From e3dad98fbe0ead8a10db8a1a5b711e80428b3ac7 Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 22 Nov 2015 14:08:32 -0500 Subject: [PATCH] fix an old use of posix_category. make http_connection translate IPs into proper endpoints when connecting over socks (instead of passing the IP on as a hostname string). add test coverage --- include/libtorrent/socks5_stream.hpp | 11 ++++ simulation/test_http_connection.cpp | 95 +++++++++++++++++----------- src/http_connection.cpp | 39 ++++++++---- test/test_transfer.cpp | 2 +- 4 files changed, 94 insertions(+), 53 deletions(-) diff --git a/include/libtorrent/socks5_stream.hpp b/include/libtorrent/socks5_stream.hpp index 4bd943db4..dc94e6c9f 100644 --- a/include/libtorrent/socks5_stream.hpp +++ b/include/libtorrent/socks5_stream.hpp @@ -111,6 +111,17 @@ public: void set_dst_name(std::string const& host) { + // TODO: 3 enable this assert and fix remaining causes of it triggering +/* +#if TORRENT_USE_ASSERTS + error_code ec; + address::from_string(host, ec); + // if this assert trips, set_dst_name() is called wth an IP address rather + // than a hostname. Instead, resolve the IP into an address and pass it to + // async_connect instead + TORRENT_ASSERT(ec); +#endif +*/ m_dst_name = host; if (m_dst_name.size() > 255) m_dst_name.resize(255); diff --git a/simulation/test_http_connection.cpp b/simulation/test_http_connection.cpp index ef0cee790..498697206 100644 --- a/simulation/test_http_connection.cpp +++ b/simulation/test_http_connection.cpp @@ -167,7 +167,7 @@ void print_http_header(std::map const& headers) } } -void run_test(settings_pack::proxy_type_t proxy_type, std::string url, int expect_size, int expect_status +void run_test(lt::aux::proxy_settings ps, std::string url, int expect_size, int expect_status , boost::system::error_condition expect_error, std::vector expect_counters); enum expect_counters @@ -184,58 +184,79 @@ enum expect_counters num_counters }; -void run_suite(settings_pack::proxy_type_t proxy_type) +void run_suite(lt::aux::proxy_settings ps) { std::string url_base = "http://10.0.0.2:8080"; - run_test(proxy_type, url_base + "/test_file", 1337, 200, error_condition(), { 1, 1, 1}); + run_test(ps, url_base + "/test_file", 1337, 200, error_condition(), { 1, 1, 1}); - run_test(proxy_type, url_base + "/non-existent", 0, 404, error_condition(), { 1, 1 }); - run_test(proxy_type, url_base + "/redirect", 1337, 200, error_condition(), { 2, 1, 1, 1 }); - run_test(proxy_type, url_base + "/relative/redirect", 1337, 200, error_condition(), {2, 1, 1, 0, 1}); + run_test(ps, url_base + "/non-existent", 0, 404, error_condition(), { 1, 1 }); + run_test(ps, url_base + "/redirect", 1337, 200, error_condition(), { 2, 1, 1, 1 }); + run_test(ps, url_base + "/relative/redirect", 1337, 200, error_condition(), {2, 1, 1, 0, 1}); - run_test(proxy_type, url_base + "/infinite/redirect", 0, 301 + run_test(ps, url_base + "/infinite/redirect", 0, 301 , error_condition(asio::error::eof, asio::error::get_misc_category()), {6, 1, 0, 0, 0, 6}); - run_test(proxy_type, url_base + "/chunked_encoding", 1337, 200, error_condition(), { 1, 1, 0, 0, 0, 0, 1}); + run_test(ps, url_base + "/chunked_encoding", 1337, 200, error_condition(), { 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 // 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) + if (ps.type != settings_pack::socks5) { - run_test(proxy_type, "http://[ff::dead:beef]:8080/test_file", 0, -1 + run_test(ps, "http://[ff::dead:beef]:8080/test_file", 0, -1 , error_condition(boost::system::errc::address_family_not_supported, generic_category()) , {0,1}); } // there is no node at 10.0.0.10, this should fail with connection refused - run_test(proxy_type, "http://10.0.0.10:8080/test_file", 0, -1, + run_test(ps, "http://10.0.0.10:8080/test_file", 0, -1, error_condition(boost::system::errc::connection_refused, generic_category()) , {0,1}); - // this hostname will resolve to multiple IPs, all but one that we cannot - // 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(proxy_type, "http://try-next.com:8080/test_file", 1337, 200 - , error_condition(), { 1, 1, 1}); + // TODO: 3 add support for "domain name" address type in libsimulator's socks + // proxy. Also, make sure we can assert that raw IPs still are passed on to + // the proxy server as IPs, and not as hostnames + if (ps.proxy_hostnames == false) + { + // this hostname will resolve to multiple IPs, all but one that we cannot + // 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(ps, "http://try-next.com:8080/test_file", 1337, 200 + , error_condition(), { 1, 1, 1}); - // make sure hostname lookup failures are passed through correctly - run_test(proxy_type, "http://non-existent.com/test_file", 0, -1 - , error_condition(asio::error::host_not_found, boost::asio::error::get_netdb_category()), { 0, 1}); + // make sure hostname lookup failures are passed through correctly + run_test(ps, "http://non-existent.com/test_file", 0, -1 + , error_condition(asio::error::host_not_found, boost::asio::error::get_netdb_category()), { 0, 1}); + } // make sure we handle gzipped content correctly - run_test(proxy_type, url_base + "/test_file.gz", 1337, 200, error_condition(), { 1, 1, 0, 0, 0, 0, 0, 1}); + run_test(ps, url_base + "/test_file.gz", 1337, 200, error_condition(), { 1, 1, 0, 0, 0, 0, 0, 1}); // TODO: 2 test basic-auth // TODO: 2 test https } -void run_test(settings_pack::proxy_type_t proxy_type, std::string url, int expect_size, int expect_status +lt::aux::proxy_settings make_proxy_settings(lt::settings_pack::proxy_type_t proxy_type) +{ + lt::aux::proxy_settings ps; + ps.type = proxy_type; + if (proxy_type != settings_pack::none) + { + ps.hostname = "50.50.50.50"; + ps.port = 4444; + ps.username = "testuser"; + ps.password = "testpass"; + ps.proxy_hostnames = false; + } + return ps; +} + +void run_test(lt::aux::proxy_settings ps, std::string url, int expect_size, int expect_status , boost::system::error_condition expect_error, std::vector expect_counters) { using sim::asio::ip::address_v4; @@ -251,19 +272,7 @@ void run_test(settings_pack::proxy_type_t proxy_type, std::string url, int expec lt::resolver res(ios); sim::http_server http(web_server, 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 - } + sim::socks_server socks(proxy_ios, 4444, ps.type == settings_pack::socks4 ? 4 : 5); char data_buffer[4000]; std::generate(data_buffer, data_buffer + sizeof(data_buffer), &std::rand); @@ -379,17 +388,27 @@ void run_test(settings_pack::proxy_type_t proxy_type, std::string url, int expec TORRENT_TEST(http_connection) { - run_suite(settings_pack::none); + lt::aux::proxy_settings ps = make_proxy_settings(settings_pack::none); + run_suite(ps); } TORRENT_TEST(http_connection_socks4) { - run_suite(settings_pack::socks4); + lt::aux::proxy_settings ps = make_proxy_settings(settings_pack::socks4); + run_suite(ps); } TORRENT_TEST(http_connection_socks5) { - run_suite(settings_pack::socks5); + lt::aux::proxy_settings ps = make_proxy_settings(settings_pack::socks5); + run_suite(ps); +} + +TORRENT_TEST(http_connection_socks5_proxy_names) +{ + lt::aux::proxy_settings ps = make_proxy_settings(settings_pack::socks5); + ps.proxy_hostnames = true; + run_suite(ps); } TORRENT_TEST(http_connection_socks_error) diff --git a/src/http_connection.cpp b/src/http_connection.cpp index 11308d558..6d2fc6035 100644 --- a/src/http_connection.cpp +++ b/src/http_connection.cpp @@ -379,6 +379,9 @@ void http_connection::start(std::string const& hostname, int port return; } + m_endpoints.clear(); + m_next_ep = 0; + #if TORRENT_USE_I2P if (is_i2p) { @@ -395,9 +398,6 @@ 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)) @@ -412,8 +412,6 @@ void http_connection::start(std::string const& hostname, int port #if defined TORRENT_ASIO_DEBUGGING add_outstanding_async("http_connection::on_resolve"); #endif - m_endpoints.clear(); - m_next_ep = 0; m_resolver.async_resolve(hostname, m_resolve_flags , boost::bind(&http_connection::on_resolve , me, _1, _2)); @@ -579,19 +577,32 @@ void http_connection::connect() && (m_proxy.type == settings_pack::socks5 || m_proxy.type == settings_pack::socks5_pw)) { - // we're using a socks proxy and we're resolving - // hostnames through it -#ifdef TORRENT_USE_OPENSSL - if (m_ssl) + // test to see if m_hostname really just is an IP (and not a hostname). If it + // is, ec will be represent "success". If so, don't set it as the socks5 + // hostname, just connect to the IP + error_code ec; + address adr = address::from_string(m_hostname, ec); + + if (ec) { - TORRENT_ASSERT(m_sock.get >()); - m_sock.get >()->next_layer().set_dst_name(m_hostname); + // we're using a socks proxy and we're resolving + // hostnames through it +#ifdef TORRENT_USE_OPENSSL + if (m_ssl) + { + TORRENT_ASSERT(m_sock.get >()); + m_sock.get >()->next_layer().set_dst_name(m_hostname); + } + else +#endif + { + TORRENT_ASSERT(m_sock.get()); + m_sock.get()->set_dst_name(m_hostname); + } } else -#endif { - TORRENT_ASSERT(m_sock.get()); - m_sock.get()->set_dst_name(m_hostname); + m_endpoints[0].address(adr); } } diff --git a/test/test_transfer.cpp b/test/test_transfer.cpp index f2bfb423f..e943f74e8 100644 --- a/test/test_transfer.cpp +++ b/test/test_transfer.cpp @@ -104,7 +104,7 @@ struct test_storage : default_storage { std::cerr << "storage written: " << m_written << " limit: " << m_limit << std::endl; error_code ec; - ec = error_code(boost::system::errc::no_space_on_device, get_posix_category()); + ec = error_code(boost::system::errc::no_space_on_device, generic_category()); se.ec = ec; return 0; }