protocol fields serialization fix (#85)

* protocol fields serialization fix

Signed-off-by: iceseer <[email protected]>

* fixup!

Signed-off-by: iceseer <[email protected]>

* issues fixup!

Signed-off-by: iceseer <[email protected]>

* fixup!

Signed-off-by: iceseer <[email protected]>

Author: iceseer

include/libp2p/multi/converters/converter_utils.hpp

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include <gsl/span>
 #include <libp2p/common/types.hpp>
 #include <libp2p/multi/multiaddress_protocol_list.hpp>
 #include <libp2p/outcome/outcome.hpp>
@@ -38,6 +39,14 @@ namespace libp2p::multi::converters {
   auto bytesToMultiaddrString(const common::ByteArray &bytes)
       -> outcome::result<std::string>;
 
+  /**
+   * Converts the given byte sequence representing
+   * a multiaddr to a string containing the multiaddr in a human-readable
+   * format, if the provided byte sequence was a valid multiaddr
+   */
+  auto bytesToMultiaddrString(gsl::span<const uint8_t> bytes)
+      -> outcome::result<std::string>;
+
 }  // namespace libp2p::multi::converters
 
 #endif  // LIBP2P_CONVERTER_UTILS_HPP

include/libp2p/multi/multiaddress.hpp

@@ -13,6 +13,7 @@
 #include <string_view>
 #include <unordered_map>
 
+#include <gsl/span>
 #include <boost/optional.hpp>
 #include <libp2p/common/types.hpp>
 #include <libp2p/multi/multiaddress_protocol_list.hpp>
@@ -53,6 +54,14 @@ namespace libp2p::multi {
      */
     static FactoryResult create(const ByteBuffer &bytes);
 
+    /**
+     * Construct a multiaddress instance from the bytes
+     * @param bytes to be in that multiaddress
+     * @return pointer to Multiaddress, if creation is successful, error
+     * otherwise
+     */
+    static FactoryResult create(gsl::span<const uint8_t> bytes);
+
     /**
      * Encapsulate a multiaddress to this one, such that:
      * '/ip4/192.168.0.1' after encapsulation with '/udp/138' becomes

src/multi/converters/converter_utils.cpp

@@ -112,7 +112,7 @@ namespace libp2p::multi::converters {
     }
   }
 
-  outcome::result<std::string> bytesToMultiaddrString(const ByteArray &bytes) {
+  outcome::result<std::string> bytesToMultiaddrString(gsl::span<const uint8_t> bytes) {
     std::string results;
 
     size_t lastpos = 0;
@@ -121,7 +121,7 @@ namespace libp2p::multi::converters {
     const std::string hex = hex_upper(bytes);
 
     // Process Hex String
-    while (lastpos < bytes.size() * 2) {
+    while (lastpos < (size_t)bytes.size() * 2) {
       gsl::span<const uint8_t, -1> pid_bytes{bytes};
       int protocol_int = UVarint(pid_bytes.subspan(lastpos / 2)).toUInt64();
       Protocol const *protocol =
@@ -192,4 +192,7 @@ namespace libp2p::multi::converters {
     return results;
   }
 
+  outcome::result<std::string> bytesToMultiaddrString(const ByteArray &bytes) {
+    return bytesToMultiaddrString(gsl::span<const uint8_t>(bytes));
+  }
 }  // namespace libp2p::multi::converters

src/multi/multiaddress.cpp

@@ -66,15 +66,18 @@ namespace libp2p::multi {
         ByteBuffer{std::vector<uint8_t>{bytes.begin(), bytes.end()}}};
   }
 
-  Multiaddress::FactoryResult Multiaddress::create(const ByteBuffer &bytes) {
-    // convert bytes address to string and make sure it represents valid address
+  Multiaddress::FactoryResult Multiaddress::create(gsl::span<const uint8_t> bytes) {
     auto conversion_res = converters::bytesToMultiaddrString(bytes);
     if (!conversion_res) {
       return Error::INVALID_INPUT;
     }
 
     std::string s = conversion_res.value();
-    return Multiaddress{std::move(s), ByteBuffer{bytes}};
+    return Multiaddress{std::move(s), ByteBuffer(bytes.begin(), bytes.end())};
+  }
+
+  Multiaddress::FactoryResult Multiaddress::create(const ByteBuffer &bytes) {
+    return create(gsl::span<const uint8_t>(bytes));
   }
 
   Multiaddress::Multiaddress(std::string &&address, ByteBuffer &&bytes)

src/protocol/identify/identify_msg_processor.cpp

@@ -14,6 +14,17 @@
 #include <libp2p/protocol/identify/utils.hpp>
 #include <generated/protocol/identify/protobuf/identify.pb.h>
 
+namespace {
+  inline std::string fromMultiaddrToString(const libp2p::multi::Multiaddress &ma) {
+    auto const &addr = ma.getBytesAddress();
+    return std::string(reinterpret_cast<const char*>(addr.data()), addr.size()); // NOLINT
+  }
+
+  inline libp2p::outcome::result<libp2p::multi::Multiaddress> fromStringToMultiaddr(const std::string &addr) {
+    return libp2p::multi::Multiaddress::create(gsl::span<const uint8_t>(reinterpret_cast<const uint8_t*>(addr.data()), addr.size())); // NOLINT
+  }
+}
+
 namespace libp2p::protocol {
   IdentifyMessageProcessor::IdentifyMessageProcessor(
       Host &host, network::ConnectionManager &conn_manager,
@@ -42,13 +53,13 @@ namespace libp2p::protocol {
     // set an address of the other side, so that it knows, which address we used
     // to connect to it
     if (auto remote_addr = stream->remoteMultiaddr()) {
-      msg.set_observedaddr(std::string{remote_addr.value().getStringAddress()});
+      msg.set_observedaddr(fromMultiaddrToString(remote_addr.value()));
     }
 
     // set addresses we are listening on
     for (const auto &addr :
          host_.getNetwork().getListener().getListenAddresses()) {
-      msg.add_listenaddrs(std::string{addr.getStringAddress()});
+      msg.add_listenaddrs(fromMultiaddrToString(addr));
     }
 
     // set our public key
@@ -267,7 +278,7 @@ namespace libp2p::protocol {
       return;
     }
 
-    auto address_res = multi::Multiaddress::create(address_str);
+    auto address_res = fromStringToMultiaddr(address_str);
     if (!address_res) {
       return log_->error("peer {} has send an invalid observed address",
                          peer_id.toBase58());
@@ -319,7 +330,7 @@ namespace libp2p::protocol {
 
     std::vector<multi::Multiaddress> listen_addresses;
     for (const auto &addr_str : addresses_strings) {
-      auto addr_res = multi::Multiaddress::create(addr_str);
+      auto addr_res = fromStringToMultiaddr(addr_str);
       if (!addr_res) {
         log_->error("peer {} has sent an invalid listen address",
                     peer_id.toBase58());

test/libp2p/protocol/identify_test.cpp

@@ -51,9 +51,9 @@ class IdentifyTest : public testing::Test {
       identify_pb_msg_.add_protocols(proto);
     }
     identify_pb_msg_.set_observedaddr(
-        std::string{remote_multiaddr_.getStringAddress()});
+        std::string{(const char*)remote_multiaddr_.getBytesAddress().data(), remote_multiaddr_.getBytesAddress().size()});
     for (const auto &addr : listen_addresses_) {
-      identify_pb_msg_.add_listenaddrs(std::string{addr.getStringAddress()});
+      identify_pb_msg_.add_listenaddrs(std::string{(const char*)addr.getBytesAddress().data(), addr.getBytesAddress().size()});
     }
     identify_pb_msg_.set_publickey(marshalled_pubkey_.data(),
                                    marshalled_pubkey_.size());
@@ -207,8 +207,7 @@ TEST_F(IdentifyTest, Receive) {
       .WillOnce(ReturnStreamRes(std::static_pointer_cast<Stream>(stream_)));
 
   EXPECT_CALL(*stream_, read(_, 1, _))
-      .WillOnce(ReadPut(gsl::make_span(identify_pb_msg_bytes_.data(), 1)))
-      .WillOnce(ReadPut(gsl::make_span(identify_pb_msg_bytes_.data() + 1, 1)));
+      .WillOnce(ReadPut(gsl::make_span(identify_pb_msg_bytes_.data(), 1)));
   EXPECT_CALL(*stream_, read(_, pb_msg_len_varint_->toUInt64(), _))
       .WillOnce(ReadPut(gsl::make_span(
           identify_pb_msg_bytes_.data() + pb_msg_len_varint_->size(),