From 2ff3ae07d1ac0ae9bfa48229b8e5d0bce1446fde Mon Sep 17 00:00:00 2001 From: Yat Ho Date: Tue, 26 Mar 2024 10:10:06 +0800 Subject: [PATCH] fix: more misc `net.cc` fixes (#6735) * feat: accept ipv6 string in square brackets for `tr_address::from_string()` * test: add test case for ipv6 string in square brackets * fix: include square brackets in host component According to RFC3986: host = IP-literal / IPv4address / reg-name IP-literal = "[" ( IPv6address / IPvFuture ) "]" * fix: set ipv6-only socket before binding UDP socket Will return EINVAL on Linux otherwise * refactor: simplify code using `evutil` when binding TCP socket * fix: do not set SO_REUSEADDR for listening sockets on Windows systems Reason: https://stackoverflow.com/a/14388707/11390656 * fix: do not enclose ipv4 address string in square brackets --- libtransmission/net.cc | 39 +++++++++++++------------ libtransmission/tr-udp.cc | 14 ++------- libtransmission/web-utils.cc | 11 +++++-- tests/libtransmission/announcer-test.cc | 4 +-- tests/libtransmission/net-test.cc | 4 ++- tests/libtransmission/web-utils-test.cc | 8 ++--- 6 files changed, 40 insertions(+), 40 deletions(-) diff --git a/libtransmission/net.cc b/libtransmission/net.cc index b6d4512af..865ad4d9b 100644 --- a/libtransmission/net.cc +++ b/libtransmission/net.cc @@ -320,21 +320,16 @@ tr_socket_t tr_netBindTCPImpl(tr_address const& addr, tr_port port, bool suppres int optval = 1; (void)setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, reinterpret_cast(&optval), sizeof(optval)); - (void)setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, reinterpret_cast(&optval), sizeof(optval)); + (void)evutil_make_listen_socket_reuseable(fd); -#ifdef IPV6_V6ONLY - - if (addr.is_ipv6() && - (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, reinterpret_cast(&optval), sizeof(optval)) == -1) && - (sockerrno != ENOPROTOOPT)) // if the kernel doesn't support it, ignore it + if (addr.is_ipv6() && evutil_make_listen_socket_ipv6only(fd) != 0 && + sockerrno != ENOPROTOOPT) // if the kernel doesn't support it, ignore it { *err_out = sockerrno; tr_net_close_socket(fd); return TR_BAD_SOCKET; } -#endif - auto const [sock, addrlen] = tr_socket_address::to_sockaddr(addr, port); if (bind(fd, reinterpret_cast(&sock), addrlen) == -1) @@ -499,23 +494,29 @@ std::optional tr_address::from_string(std::string_view address_sv) { auto const address_sz = tr_strbuf{ address_sv }; - auto addr = tr_address{}; - - addr.addr.addr4 = {}; - if (evutil_inet_pton(AF_INET, address_sz, &addr.addr.addr4) == 1) + auto ss = sockaddr_storage{}; + auto sslen = int{ sizeof(ss) }; + if (evutil_parse_sockaddr_port(address_sz, reinterpret_cast(&ss), &sslen) != 0) { + return {}; + } + + auto addr = tr_address{}; + switch (ss.ss_family) + { + case AF_INET: + addr.addr.addr4 = reinterpret_cast(&ss)->sin_addr; addr.type = TR_AF_INET; return addr; - } - addr.addr.addr6 = {}; - if (evutil_inet_pton(AF_INET6, address_sz, &addr.addr.addr6) == 1) - { + case AF_INET6: + addr.addr.addr6 = reinterpret_cast(&ss)->sin6_addr; addr.type = TR_AF_INET6; return addr; - } - return {}; + default: + return {}; + } } std::string_view tr_address::display_name(char* out, size_t outlen) const @@ -706,7 +707,7 @@ int tr_address::compare(tr_address const& that) const noexcept // <=> std::string tr_socket_address::display_name(tr_address const& address, tr_port port) noexcept { - return fmt::format("[{:s}]:{:d}", address.display_name(), port.host()); + return fmt::format(address.is_ipv6() ? "[{:s}]:{:d}" : "{:s}:{:d}", address.display_name(), port.host()); } bool tr_socket_address::is_valid_for_peers(tr_peer_from from) const noexcept diff --git a/libtransmission/tr-udp.cc b/libtransmission/tr-udp.cc index 97c9715d5..a1d219033 100644 --- a/libtransmission/tr-udp.cc +++ b/libtransmission/tr-udp.cc @@ -159,8 +159,7 @@ tr_session::tr_udp_core::tr_udp_core(tr_session& session, tr_port udp_port) if (auto sock = socket(PF_INET, SOCK_DGRAM, 0); sock != TR_BAD_SOCKET) { - auto optval = 1; - (void)setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, reinterpret_cast(&optval), sizeof(optval)); + (void)evutil_make_listen_socket_reuseable(sock); auto const addr = session_.bind_address(TR_AF_INET); auto const [ss, sslen] = tr_socket_address::to_sockaddr(addr, udp_port_); @@ -204,8 +203,8 @@ tr_session::tr_udp_core::tr_udp_core(tr_session& session, tr_port udp_port) } else if (auto sock = socket(PF_INET6, SOCK_DGRAM, 0); sock != TR_BAD_SOCKET) { - auto optval = 1; - (void)setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, reinterpret_cast(&optval), sizeof(optval)); + (void)evutil_make_listen_socket_reuseable(sock); + (void)evutil_make_listen_socket_ipv6only(sock); auto const addr = session_.bind_address(TR_AF_INET6); auto const [ss, sslen] = tr_socket_address::to_sockaddr(addr, udp_port_); @@ -240,13 +239,6 @@ tr_session::tr_udp_core::tr_udp_core(tr_session& session, tr_port udp_port) udp6_socket_ = sock; udp6_event_.reset(event_new(session_.event_base(), udp6_socket_, EV_READ | EV_PERSIST, event_callback, &session_)); event_add(udp6_event_.get(), nullptr); - -#ifdef IPV6_V6ONLY - // Since we always open an IPv4 socket on the same port, - // this shouldn't matter. But I'm superstitious. - int one = 1; - (void)setsockopt(sock, IPPROTO_IPV6, IPV6_V6ONLY, reinterpret_cast(&one), sizeof(one)); -#endif } } } diff --git a/libtransmission/web-utils.cc b/libtransmission/web-utils.cc index 1e5c545fc..25c233daa 100644 --- a/libtransmission/web-utils.cc +++ b/libtransmission/web-utils.cc @@ -340,8 +340,13 @@ std::optional tr_urlParse(std::string_view url) auto remain = parsed.authority; if (tr_strv_starts_with(remain, '[')) { - remain.remove_prefix(1); // '[' - parsed.host = tr_strv_sep(&remain, ']'); + pos = remain.find(']'); + if (pos == std::string_view::npos) + { + return std::nullopt; + } + parsed.host = remain.substr(0, pos + 1); + remain.remove_prefix(pos + 1); if (tr_strv_starts_with(remain, ':')) { remain.remove_prefix(1); @@ -389,7 +394,7 @@ std::optional tr_urlParse(std::string_view url) std::optional tr_urlParseTracker(std::string_view url) { auto const parsed = tr_urlParse(url); - return parsed && tr_isValidTrackerScheme(parsed->scheme) ? std::make_optional(*parsed) : std::nullopt; + return parsed && tr_isValidTrackerScheme(parsed->scheme) ? parsed : std::nullopt; } bool tr_urlIsValidTracker(std::string_view url) diff --git a/tests/libtransmission/announcer-test.cc b/tests/libtransmission/announcer-test.cc index 0984ff15e..e22bf4948 100644 --- a/tests/libtransmission/announcer-test.cc +++ b/tests/libtransmission/announcer-test.cc @@ -84,7 +84,7 @@ TEST_F(AnnouncerTest, parseHttpAnnounceResponsePexCompact) if (std::size(response.pex) == 1) { - EXPECT_EQ("[127.0.0.1]:64551"sv, response.pex[0].display_name()); + EXPECT_EQ("127.0.0.1:64551"sv, response.pex[0].display_name()); } } @@ -123,7 +123,7 @@ TEST_F(AnnouncerTest, parseHttpAnnounceResponsePexList) if (std::size(response.pex) == 1) { - EXPECT_EQ("[8.8.4.4]:53"sv, response.pex[0].display_name()); + EXPECT_EQ("8.8.4.4:53"sv, response.pex[0].display_name()); } } diff --git a/tests/libtransmission/net-test.cc b/tests/libtransmission/net-test.cc index 4e208a4fe..0cd8f3f4f 100644 --- a/tests/libtransmission/net-test.cc +++ b/tests/libtransmission/net-test.cc @@ -218,7 +218,9 @@ TEST_F(NetTest, ipCompare) std::tuple{ "8.8.8.8"sv, "8.8.8.8"sv, 0 }, std::tuple{ "8.8.8.8"sv, "2001:0:0eab:dead::a0:abcd:4e"sv, -1 }, std::tuple{ "2001:1890:1112:1::20"sv, "2001:0:0eab:dead::a0:abcd:4e"sv, 1 }, - std::tuple{ "2001:1890:1112:1::20"sv, "2001:1890:1112:1::20"sv, 0 } }; + std::tuple{ "2001:1890:1112:1::20"sv, "[2001:0:0eab:dead::a0:abcd:4e]"sv, 1 }, + std::tuple{ "2001:1890:1112:1::20"sv, "2001:1890:1112:1::20"sv, 0 }, + std::tuple{ "2001:1890:1112:1::20"sv, "[2001:1890:1112:1::20]"sv, 0 } }; for (auto const& [sv1, sv2, res] : IpPairs) { diff --git a/tests/libtransmission/web-utils-test.cc b/tests/libtransmission/web-utils-test.cc index f84848cf9..e163207b8 100644 --- a/tests/libtransmission/web-utils-test.cc +++ b/tests/libtransmission/web-utils-test.cc @@ -126,8 +126,8 @@ TEST_F(WebUtilsTest, urlParse) url = "http://[2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d]:8080/announce"sv; parsed = tr_urlParse(url); EXPECT_EQ("http"sv, parsed->scheme); - EXPECT_EQ("2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d"sv, parsed->sitename); - EXPECT_EQ("2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d"sv, parsed->host); + EXPECT_EQ("[2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d]"sv, parsed->sitename); + EXPECT_EQ("[2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d]"sv, parsed->host); EXPECT_EQ("/announce"sv, parsed->path); EXPECT_EQ(8080, parsed->port); @@ -135,8 +135,8 @@ TEST_F(WebUtilsTest, urlParse) url = "http://[2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d]/announce"sv; parsed = tr_urlParse(url); EXPECT_EQ("http"sv, parsed->scheme); - EXPECT_EQ("2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d"sv, parsed->sitename); - EXPECT_EQ("2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d"sv, parsed->host); + EXPECT_EQ("[2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d]"sv, parsed->sitename); + EXPECT_EQ("[2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d]"sv, parsed->host); EXPECT_EQ("/announce"sv, parsed->path); EXPECT_EQ(80, parsed->port);