Merge pull request #9138

cd7ccfb Disable/fix ports with I2P (Lee Clagett)

5 files changed, 52 insertions, 85 deletions

diff --git a/src/net/i2p_address.cpp b/src/net/i2p_address.cpp
index a151e420b..d5ca8544f 100644
--- a/src/net/i2p_address.cpp
+++ b/src/net/i2p_address.cpp
@@ -71,7 +71,7 @@ namespace net
         struct i2p_serialized
         {
             std::string host;
-            std::uint16_t port;
+            std::uint16_t port; //! Leave for compatability with older clients
 
             BEGIN_KV_SERIALIZE_MAP()
                 KV_SERIALIZE(host)
@@ -80,8 +80,7 @@ namespace net
         };
     }
 
-    i2p_address::i2p_address(const boost::string_ref host, const std::uint16_t port) noexcept
-      : port_(port)
+    i2p_address::i2p_address(const boost::string_ref host) noexcept
     {
         // this is a private constructor, throw if moved to public
         assert(host.size() < sizeof(host_));
@@ -97,27 +96,19 @@ namespace net
     }
 
     i2p_address::i2p_address() noexcept
-      : port_(0)
     {
         static_assert(sizeof(unknown_host) <= sizeof(host_), "bad buffer size");
         std::memcpy(host_, unknown_host, sizeof(unknown_host));
         std::memset(host_ + sizeof(unknown_host), 0, sizeof(host_) - sizeof(unknown_host));
     }
 
-    expect<i2p_address> i2p_address::make(const boost::string_ref address, const std::uint16_t default_port)
+    expect<i2p_address> i2p_address::make(const boost::string_ref address)
     {
         boost::string_ref host = address.substr(0, address.rfind(':'));
-        const boost::string_ref port =
-            address.substr(host.size() + (host.size() == address.size() ? 0 : 1));
-
         MONERO_CHECK(host_check(host));
 
-        std::uint16_t porti = default_port;
-        if (!port.empty() && !epee::string_tools::get_xtype_from_string(porti, std::string{port}))
-            return {net::error::invalid_port};
-
         static_assert(b32_length + sizeof(tld) == sizeof(i2p_address::host_), "bad internal host size");
-        return i2p_address{host, porti};
+        return i2p_address{host};
     }
 
     bool i2p_address::_load(epee::serialization::portable_storage& src, epee::serialization::section* hparent)
@@ -127,23 +118,21 @@ namespace net
         {
             std::memcpy(host_, in.host.data(), in.host.size());
             std::memset(host_ + in.host.size(), 0, sizeof(host_) - in.host.size());
-            port_ = in.port;
             return true;
         }
         static_assert(sizeof(unknown_host) <= sizeof(host_), "bad buffer size");
         std::memcpy(host_, unknown_host, sizeof(unknown_host)); // include null terminator
-        port_ = 0;
         return false;
     }
 
     bool i2p_address::store(epee::serialization::portable_storage& dest, epee::serialization::section* hparent) const
     {
-        const i2p_serialized out{std::string{host_}, port_};
+        // Set port to 1 for backwards compatability; zero is invalid port
+        const i2p_serialized out{std::string{host_}, 1};
         return out.store(dest, hparent);
     }
 
     i2p_address::i2p_address(const i2p_address& rhs) noexcept
-      : port_(rhs.port_)
     {
         std::memcpy(host_, rhs.host_, sizeof(host_));
     }
@@ -152,7 +141,6 @@ namespace net
     {
         if (this != std::addressof(rhs))
         {
-            port_ = rhs.port_;
             std::memcpy(host_, rhs.host_, sizeof(host_));
         }
         return *this;
@@ -166,13 +154,12 @@ namespace net
 
     bool i2p_address::equal(const i2p_address& rhs) const noexcept
     {
-        return port_ == rhs.port_ && is_same_host(rhs);
+        return is_same_host(rhs);
     }
 
     bool i2p_address::less(const i2p_address& rhs) const noexcept
     {
-        int res = std::strcmp(host_str(), rhs.host_str());
-        return res < 0 || (res == 0 && port() < rhs.port());
+        return std::strcmp(host_str(), rhs.host_str()) < 0;
     }
 
     bool i2p_address::is_same_host(const i2p_address& rhs) const noexcept
@@ -182,20 +169,6 @@ namespace net
 
     std::string i2p_address::str() const
     {
-        const std::size_t host_length = std::strlen(host_str());
-        const std::size_t port_length =
-            port_ == 0 ? 0 : std::numeric_limits<std::uint16_t>::digits10 + 2;
-
-        std::string out{};
-        out.reserve(host_length + port_length);
-        out.assign(host_str(), host_length);
-
-        if (port_ != 0)
-        {
-            out.push_back(':');
-            namespace karma = boost::spirit::karma;
-            karma::generate(std::back_inserter(out), karma::ushort_, port());
-        }
-        return out;
+        return host_str();
     }
 }
diff --git a/src/net/i2p_address.h b/src/net/i2p_address.h
index 4d342e67d..f52ccbb51 100644
--- a/src/net/i2p_address.h
+++ b/src/net/i2p_address.h
@@ -50,11 +50,10 @@ namespace net
     //! b32 i2p address; internal format not condensed/decoded.
     class i2p_address
     {
-        std::uint16_t port_;
         char host_[61]; // null-terminated
 
         //! Keep in private, `host.size()` has no runtime check
-        i2p_address(boost::string_ref host, std::uint16_t port) noexcept;
+        i2p_address(boost::string_ref host) noexcept;
 
     public:
         //! \return Size of internal buffer for host.
@@ -74,7 +73,7 @@ namespace net
             with `default_port` being used if port is not specified in
             `address`.
         */
-        static expect<i2p_address> make(boost::string_ref address, std::uint16_t default_port = 0);
+        static expect<i2p_address> make(boost::string_ref address);
 
         //! Load from epee p2p format, and \return false if not valid tor address
         bool _load(epee::serialization::portable_storage& src, epee::serialization::section* hparent);
@@ -103,8 +102,8 @@ namespace net
         //! \return Null-terminated `x.b32.i2p` value or `unknown_str()`.
         const char* host_str() const noexcept { return host_; }
 
-        //! \return Port value or `0` if unspecified.
-        std::uint16_t port() const noexcept { return port_; }
+        //! \return `1` to work with I2P socks which considers `0` error.
+        std::uint16_t port() const noexcept { return 1; }
 
         static constexpr bool is_loopback() noexcept { return false; }
         static constexpr bool is_local() noexcept { return false; }
diff --git a/src/net/parse.cpp b/src/net/parse.cpp
index 71d0d7b26..a6d665082 100644
--- a/src/net/parse.cpp
+++ b/src/net/parse.cpp
@@ -81,7 +81,7 @@ namespace net
         if (host_str_ref.ends_with(".onion"))
             return tor_address::make(address, default_port);
         if (host_str_ref.ends_with(".i2p"))
-            return i2p_address::make(address, default_port);
+            return i2p_address::make(address);
 
         boost::system::error_code ec;
         boost::asio::ip::address_v6 v6 = boost::asio::ip::address_v6::from_string(host_str, ec);
diff --git a/src/p2p/net_peerlist_boost_serialization.h b/src/p2p/net_peerlist_boost_serialization.h
index 7f4aecce3..0b7a54860 100644
--- a/src/p2p/net_peerlist_boost_serialization.h
+++ b/src/p2p/net_peerlist_boost_serialization.h
@@ -196,7 +196,7 @@ namespace boost
       if (std::strcmp(host, net::i2p_address::unknown_str()) == 0)
         na = net::i2p_address::unknown();
       else
-        na = MONERO_UNWRAP(net::i2p_address::make(host, port));
+        na = MONERO_UNWRAP(net::i2p_address::make(host));
     }
 
     template <class Archive, class ver_type>
diff --git a/tests/unit_tests/net.cpp b/tests/unit_tests/net.cpp
index 2b565f7d0..c9099b99b 100644
--- a/tests/unit_tests/net.cpp
+++ b/tests/unit_tests/net.cpp
@@ -472,8 +472,6 @@ TEST(i2p_address, invalid)
     EXPECT_TRUE(net::i2p_address::make(".b32.i2p:").has_error());
     EXPECT_TRUE(net::i2p_address::make(b32_i2p + 1).has_error());
     EXPECT_TRUE(net::i2p_address::make(boost::string_ref{b32_i2p, sizeof(b32_i2p) - 2}).has_error());
-    EXPECT_TRUE(net::i2p_address::make(std::string{b32_i2p} + ":65536").has_error());
-    EXPECT_TRUE(net::i2p_address::make(std::string{b32_i2p} + ":-1").has_error());
 
     std::string i2p{b32_i2p};
     i2p.at(10) = 1;
@@ -487,7 +485,7 @@ TEST(i2p_address, unblockable_types)
     ASSERT_NE(nullptr, i2p.host_str());
     EXPECT_STREQ("<unknown i2p host>", i2p.host_str());
     EXPECT_STREQ("<unknown i2p host>", i2p.str().c_str());
-    EXPECT_EQ(0u, i2p.port());
+    EXPECT_EQ(1u, i2p.port());
     EXPECT_TRUE(i2p.is_unknown());
     EXPECT_FALSE(i2p.is_local());
     EXPECT_FALSE(i2p.is_loopback());
@@ -498,7 +496,7 @@ TEST(i2p_address, unblockable_types)
     ASSERT_NE(nullptr, i2p.host_str());
     EXPECT_STREQ("<unknown i2p host>", i2p.host_str());
     EXPECT_STREQ("<unknown i2p host>", i2p.str().c_str());
-    EXPECT_EQ(0u, i2p.port());
+    EXPECT_EQ(1u, i2p.port());
     EXPECT_TRUE(i2p.is_unknown());
     EXPECT_FALSE(i2p.is_local());
     EXPECT_FALSE(i2p.is_loopback());
@@ -513,14 +511,14 @@ TEST(i2p_address, valid)
     const auto address1 = net::i2p_address::make(b32_i2p);
 
     ASSERT_TRUE(address1.has_value());
-    EXPECT_EQ(0u, address1->port());
+    EXPECT_EQ(1u, address1->port());
     EXPECT_STREQ(b32_i2p, address1->host_str());
     EXPECT_STREQ(b32_i2p, address1->str().c_str());
     EXPECT_TRUE(address1->is_blockable());
 
     net::i2p_address address2{*address1};
 
-    EXPECT_EQ(0u, address2.port());
+    EXPECT_EQ(1u, address2.port());
     EXPECT_STREQ(b32_i2p, address2.host_str());
     EXPECT_STREQ(b32_i2p, address2.str().c_str());
     EXPECT_TRUE(address2.is_blockable());
@@ -537,9 +535,9 @@ TEST(i2p_address, valid)
 
     address2 = MONERO_UNWRAP(net::i2p_address::make(std::string{b32_i2p_2} + ":6545"));
 
-    EXPECT_EQ(6545, address2.port());
+    EXPECT_EQ(1u, address2.port());
     EXPECT_STREQ(b32_i2p_2, address2.host_str());
-    EXPECT_EQ(std::string{b32_i2p_2} + ":6545", address2.str().c_str());
+    EXPECT_EQ(std::string{b32_i2p_2}, address2.str().c_str());
     EXPECT_TRUE(address2.is_blockable());
     EXPECT_FALSE(address2.equal(*address1));
     EXPECT_FALSE(address1->equal(address2));
@@ -552,22 +550,22 @@ TEST(i2p_address, valid)
     EXPECT_FALSE(address2.less(*address1));
     EXPECT_TRUE(address1->less(address2));
 
-    net::i2p_address address3 = MONERO_UNWRAP(net::i2p_address::make(std::string{b32_i2p} + ":", 65535));
+    net::i2p_address address3 = MONERO_UNWRAP(net::i2p_address::make(std::string{b32_i2p} + ":65535"));
 
-    EXPECT_EQ(65535, address3.port());
+    EXPECT_EQ(1u, address3.port());
     EXPECT_STREQ(b32_i2p, address3.host_str());
-    EXPECT_EQ(std::string{b32_i2p} + ":65535", address3.str().c_str());
+    EXPECT_EQ(std::string{b32_i2p}, address3.str().c_str());
     EXPECT_TRUE(address3.is_blockable());
-    EXPECT_FALSE(address3.equal(*address1));
-    EXPECT_FALSE(address1->equal(address3));
-    EXPECT_FALSE(address3 == *address1);
-    EXPECT_FALSE(*address1 == address3);
-    EXPECT_TRUE(address3 != *address1);
-    EXPECT_TRUE(*address1 != address3);
+    EXPECT_TRUE(address3.equal(*address1));
+    EXPECT_TRUE(address1->equal(address3));
+    EXPECT_TRUE(address3 == *address1);
+    EXPECT_TRUE(*address1 == address3);
+    EXPECT_FALSE(address3 != *address1);
+    EXPECT_FALSE(*address1 != address3);
     EXPECT_TRUE(address3.is_same_host(*address1));
     EXPECT_TRUE(address1->is_same_host(address3));
     EXPECT_FALSE(address3.less(*address1));
-    EXPECT_TRUE(address1->less(address3));
+    EXPECT_FALSE(address1->less(address3));
 
     EXPECT_FALSE(address3.equal(address2));
     EXPECT_FALSE(address2.equal(address3));
@@ -583,8 +581,8 @@ TEST(i2p_address, valid)
 
 TEST(i2p_address, generic_network_address)
 {
-    const epee::net_utils::network_address i2p1{MONERO_UNWRAP(net::i2p_address::make(b32_i2p, 8080))};
-    const epee::net_utils::network_address i2p2{MONERO_UNWRAP(net::i2p_address::make(b32_i2p, 8080))};
+    const epee::net_utils::network_address i2p1{MONERO_UNWRAP(net::i2p_address::make(b32_i2p))};
+    const epee::net_utils::network_address i2p2{MONERO_UNWRAP(net::i2p_address::make(b32_i2p))};
     const epee::net_utils::network_address ip{epee::net_utils::ipv4_network_address{100, 200}};
 
     EXPECT_EQ(i2p1, i2p2);
@@ -592,7 +590,7 @@ TEST(i2p_address, generic_network_address)
     EXPECT_LT(ip, i2p1);
 
     EXPECT_STREQ(b32_i2p, i2p1.host_str().c_str());
-    EXPECT_EQ(std::string{b32_i2p} + ":8080", i2p1.str());
+    EXPECT_STREQ(b32_i2p, i2p1.str().c_str());
     EXPECT_EQ(epee::net_utils::address_type::i2p, i2p1.get_type_id());
     EXPECT_EQ(epee::net_utils::address_type::i2p, i2p2.get_type_id());
     EXPECT_EQ(epee::net_utils::address_type::ipv4, ip.get_type_id());
@@ -620,11 +618,11 @@ TEST(i2p_address, epee_serializev_b32)
 {
     epee::byte_slice buffer{};
     {
-        test_command_i2p command{MONERO_UNWRAP(net::i2p_address::make(b32_i2p, 10))};
+        test_command_i2p command{MONERO_UNWRAP(net::i2p_address::make(b32_i2p))};
         EXPECT_FALSE(command.i2p.is_unknown());
         EXPECT_NE(net::i2p_address{}, command.i2p);
         EXPECT_STREQ(b32_i2p, command.i2p.host_str());
-        EXPECT_EQ(10u, command.i2p.port());
+        EXPECT_EQ(1u, command.i2p.port());
 
         epee::serialization::portable_storage stg{};
         EXPECT_TRUE(command.store(stg));
@@ -636,7 +634,7 @@ TEST(i2p_address, epee_serializev_b32)
         EXPECT_TRUE(command.i2p.is_unknown());
         EXPECT_EQ(net::i2p_address{}, command.i2p);
         EXPECT_STREQ(net::i2p_address::unknown_str(), command.i2p.host_str());
-        EXPECT_EQ(0u, command.i2p.port());
+        EXPECT_EQ(1u, command.i2p.port());
 
         epee::serialization::portable_storage stg{};
         EXPECT_TRUE(stg.load_from_binary(epee::to_span(buffer)));
@@ -645,7 +643,7 @@ TEST(i2p_address, epee_serializev_b32)
     EXPECT_FALSE(command.i2p.is_unknown());
     EXPECT_NE(net::i2p_address{}, command.i2p);
     EXPECT_STREQ(b32_i2p, command.i2p.host_str());
-    EXPECT_EQ(10u, command.i2p.port());
+    EXPECT_EQ(1u, command.i2p.port());
 
     // make sure that exceeding max buffer doesn't destroy i2p_address::_load
     {
@@ -664,7 +662,7 @@ TEST(i2p_address, epee_serializev_b32)
     EXPECT_TRUE(command.i2p.is_unknown());
     EXPECT_EQ(net::i2p_address{}, command.i2p);
     EXPECT_STRNE(b32_i2p, command.i2p.host_str());
-    EXPECT_EQ(0u, command.i2p.port());
+    EXPECT_EQ(1u, command.i2p.port());
 }
 
 TEST(i2p_address, epee_serialize_unknown)
@@ -675,7 +673,7 @@ TEST(i2p_address, epee_serialize_unknown)
         EXPECT_TRUE(command.i2p.is_unknown());
         EXPECT_EQ(net::i2p_address{}, command.i2p);
         EXPECT_STREQ(net::i2p_address::unknown_str(), command.i2p.host_str());
-        EXPECT_EQ(0u, command.i2p.port());
+        EXPECT_EQ(1u, command.i2p.port());
 
         epee::serialization::portable_storage stg{};
         EXPECT_TRUE(command.store(stg));
@@ -687,7 +685,7 @@ TEST(i2p_address, epee_serialize_unknown)
         EXPECT_TRUE(command.i2p.is_unknown());
         EXPECT_EQ(net::i2p_address{}, command.i2p);
         EXPECT_STRNE(b32_i2p, command.i2p.host_str());
-        EXPECT_EQ(0u, command.i2p.port());
+        EXPECT_EQ(1u, command.i2p.port());
 
         epee::serialization::portable_storage stg{};
         EXPECT_TRUE(stg.load_from_binary(epee::to_span(buffer)));
@@ -696,7 +694,7 @@ TEST(i2p_address, epee_serialize_unknown)
     EXPECT_TRUE(command.i2p.is_unknown());
     EXPECT_EQ(net::i2p_address{}, command.i2p);
     EXPECT_STREQ(net::i2p_address::unknown_str(), command.i2p.host_str());
-    EXPECT_EQ(0u, command.i2p.port());
+    EXPECT_EQ(1u, command.i2p.port());
 
     // make sure that exceeding max buffer doesn't destroy i2p_address::_load
     {
@@ -715,18 +713,18 @@ TEST(i2p_address, epee_serialize_unknown)
     EXPECT_TRUE(command.i2p.is_unknown());
     EXPECT_EQ(net::i2p_address{}, command.i2p);
     EXPECT_STRNE(b32_i2p, command.i2p.host_str());
-    EXPECT_EQ(0u, command.i2p.port());
+    EXPECT_EQ(1u, command.i2p.port());
 }
 
 TEST(i2p_address, boost_serialize_b32)
 {
     std::string buffer{};
     {
-        const net::i2p_address i2p = MONERO_UNWRAP(net::i2p_address::make(b32_i2p, 10));
+        const net::i2p_address i2p = MONERO_UNWRAP(net::i2p_address::make(b32_i2p));
         EXPECT_FALSE(i2p.is_unknown());
         EXPECT_NE(net::i2p_address{}, i2p);
         EXPECT_STREQ(b32_i2p, i2p.host_str());
-        EXPECT_EQ(10u, i2p.port());
+        EXPECT_EQ(1u, i2p.port());
 
         std::ostringstream stream{};
         {
@@ -741,7 +739,7 @@ TEST(i2p_address, boost_serialize_b32)
         EXPECT_TRUE(i2p.is_unknown());
         EXPECT_EQ(net::i2p_address{}, i2p);
         EXPECT_STREQ(net::i2p_address::unknown_str(), i2p.host_str());
-        EXPECT_EQ(0u, i2p.port());
+        EXPECT_EQ(1u, i2p.port());
 
         std::istringstream stream{buffer};
         boost::archive::portable_binary_iarchive archive{stream};
@@ -750,7 +748,7 @@ TEST(i2p_address, boost_serialize_b32)
     EXPECT_FALSE(i2p.is_unknown());
     EXPECT_NE(net::i2p_address{}, i2p);
     EXPECT_STREQ(b32_i2p, i2p.host_str());
-    EXPECT_EQ(10u, i2p.port());
+    EXPECT_EQ(1u, i2p.port());
 }
 
 TEST(i2p_address, boost_serialize_unknown)
@@ -761,7 +759,7 @@ TEST(i2p_address, boost_serialize_unknown)
         EXPECT_TRUE(i2p.is_unknown());
         EXPECT_EQ(net::i2p_address::unknown(), i2p);
         EXPECT_STREQ(net::i2p_address::unknown_str(), i2p.host_str());
-        EXPECT_EQ(0u, i2p.port());
+        EXPECT_EQ(1u, i2p.port());
 
         std::ostringstream stream{};
         {
@@ -776,7 +774,7 @@ TEST(i2p_address, boost_serialize_unknown)
         EXPECT_TRUE(i2p.is_unknown());
         EXPECT_EQ(net::i2p_address{}, i2p);
         EXPECT_STREQ(net::i2p_address::unknown_str(), i2p.host_str());
-        EXPECT_EQ(0u, i2p.port());
+        EXPECT_EQ(1u, i2p.port());
 
         std::istringstream stream{buffer};
         boost::archive::portable_binary_iarchive archive{stream};
@@ -785,7 +783,7 @@ TEST(i2p_address, boost_serialize_unknown)
     EXPECT_TRUE(i2p.is_unknown());
     EXPECT_EQ(net::i2p_address::unknown(), i2p);
     EXPECT_STREQ(net::i2p_address::unknown_str(), i2p.host_str());
-    EXPECT_EQ(0u, i2p.port());
+    EXPECT_EQ(1u, i2p.port());
 }
 
 TEST(get_network_address, i2p)
@@ -801,16 +799,13 @@ TEST(get_network_address, i2p)
     ASSERT_TRUE(bool(address));
     EXPECT_EQ(epee::net_utils::address_type::i2p, address->get_type_id());
     EXPECT_STREQ(b32_i2p, address->host_str().c_str());
-    EXPECT_EQ(std::string{b32_i2p} + ":1000", address->str());
+    EXPECT_EQ(std::string{b32_i2p}, address->str());
 
     address = net::get_network_address(std::string{b32_i2p} + ":2000", 1000);
     ASSERT_TRUE(bool(address));
     EXPECT_EQ(epee::net_utils::address_type::i2p, address->get_type_id());
     EXPECT_STREQ(b32_i2p, address->host_str().c_str());
-    EXPECT_EQ(std::string{b32_i2p} + ":2000", address->str());
-
-    address = net::get_network_address(std::string{b32_i2p} + ":65536", 1000);
-    EXPECT_EQ(net::error::invalid_port, address);
+    EXPECT_EQ(std::string{b32_i2p}, address->str());
 }
 
 TEST(get_network_address, ipv4)