Merge pull request #926

c175fbf RouterInfo: buffer refactor (anonimal)
695c911 Tests: create buffer utility class unit-test (anonimal)
5e2897c Core: create buffer utility class (anonimal)

src/core/CMakeLists.txt

@@ -59,6 +59,7 @@ add_library(kovri-core
   router/tunnel/impl.h
   router/tunnel/pool.h
   router/tunnel/transit.h
+  util/buffer.h
   util/byte_stream.h
   util/config.h
   util/exception.h

src/core/router/context.cc

@@ -123,7 +123,7 @@ void RouterContext::Initialize(const boost::program_options::variables_map& map)
           std::make_pair(has_ntcp, has_ssu));  // TODO(anonimal): brittle, see TODO in header
 
       // Update context RI
-      m_RouterInfo.Update(router.GetBuffer(), router.GetBufferLen());
+      m_RouterInfo.Update(router.data(), router.size());
     }
   else  // Keys (and RI should also) exist
     {
@@ -178,7 +178,7 @@ void RouterContext::Initialize(const boost::program_options::variables_map& map)
       router.SetDefaultOptions();
 
       // Update context RI
-      m_RouterInfo.Update(router.GetBuffer(), router.GetBufferLen());
+      m_RouterInfo.Update(router.data(), router.size());
 
       // Test for reachability of context's RI
       if (IsUnreachable())

src/core/router/i2np.cc

@@ -289,7 +289,7 @@ std::shared_ptr<I2NPMessage> CreateDatabaseStoreMsg(
       buf += 32;
     }
     kovri::core::Gzip compressor;
-    compressor.Put(router->GetBuffer(), router->GetBufferLen());
+    compressor.Put(router->data(), router->size());
     auto size = compressor.MaxRetrievable();
     core::OutputByteStream::Write<std::uint16_t>(buf, size);  // size
     buf += 2;

src/core/router/info.cc

@@ -54,7 +54,7 @@ namespace kovri
 namespace core
 {
 
-RouterInfo::RouterInfo() : m_Exception(__func__), m_Buffer(nullptr)  // TODO(anonimal): buffer refactor
+RouterInfo::RouterInfo() : m_Exception(__func__)
 {
 }
 
@@ -103,23 +103,16 @@ RouterInfo::RouterInfo(
 
 RouterInfo::RouterInfo(const std::string& path)
     : m_Exception(__func__),
-      m_Path(path),
-      m_Buffer(std::make_unique<std::uint8_t[]>(Size::MaxBuffer))  // TODO(anonimal): buffer refactor
+      m_Path(path)
 {
   ReadFromFile();
   ReadFromBuffer(false);
 }
 
 RouterInfo::RouterInfo(const std::uint8_t* buf, std::uint16_t len)
     : m_Exception(__func__),
-      m_Buffer(std::make_unique<std::uint8_t[]>(Size::MaxBuffer)),  // TODO(anonimal): buffer refactor
-      m_BufferLen(len)
+      m_Buffer(buf, len)
 {
-  if (!buf)
-    throw std::invalid_argument("RouterInfo: null buffer");
-  if (len < Size::MinBuffer || len > Size::MaxBuffer)
-    throw std::length_error("RouterInfo: invalid buffer length");
-  std::memcpy(m_Buffer.get(), buf, len);
   ReadFromBuffer(true);
   m_IsUpdated = true;
 }
@@ -138,18 +131,11 @@ void RouterInfo::ReadFromFile()
 
       // Get full length of stream
       stream.Seekg(0, std::ios::end);
-      m_BufferLen = stream.Tellg();
-      if (m_BufferLen < Size::MinBuffer || m_BufferLen > Size::MaxBuffer)
-        {
-          LOG(error) << "RouterInfo: buffer length = " << m_BufferLen;
-          throw std::runtime_error(m_Path + " is malformed");
-        }
+      m_Buffer(stream.Tellg());
 
       // Read in complete length of stream
       stream.Seekg(0, std::ios::beg);
-      if (!m_Buffer)
-        m_Buffer = std::make_unique<std::uint8_t[]>(Size::MaxBuffer);
-      stream.Read(m_Buffer.get(), m_BufferLen);
+      stream.Read(m_Buffer.data(), m_Buffer.size());
     }
   catch (...)
     {
@@ -163,27 +149,26 @@ void RouterInfo::ReadFromBuffer(bool verify_signature)
   try
     {
       // Get + verify identity length from existing RI in buffer
-      std::size_t ident_len =
-          m_RouterIdentity.FromBuffer(m_Buffer.get(), m_BufferLen);
+      std::size_t const ident_len =
+          m_RouterIdentity.FromBuffer(m_Buffer.data(), m_Buffer.size());
       if (!ident_len)
         throw std::length_error("null ident length");
 
       // Parse existing RI from buffer
       std::string router_info(
-          reinterpret_cast<char*>(m_Buffer.get()) + ident_len,
-          m_BufferLen - ident_len);
+          reinterpret_cast<const char*>(m_Buffer.data()) + ident_len,
+          m_Buffer.size() - ident_len);
 
       ParseRouterInfo(router_info);
 
       // Verify signature
       if (verify_signature)
         {
           // Note: signature length is guaranteed to be no less than buffer length
-          std::uint16_t len = m_BufferLen - m_RouterIdentity.GetSignatureLen();
+          std::uint16_t const len =
+              m_Buffer.size() - m_RouterIdentity.GetSignatureLen();
           if (!m_RouterIdentity.Verify(
-                  reinterpret_cast<std::uint8_t*>(m_Buffer.get()),
-                  len,
-                  reinterpret_cast<std::uint8_t*>(m_Buffer.get() + len)))
+                  m_Buffer.data(), len, m_Buffer.data() + len))
             {
               LOG(error) << "RouterInfo: signature verification failed";
               m_IsUnreachable = true;
@@ -679,33 +664,27 @@ void RouterInfo::DisableV6()
 
 void RouterInfo::Update(const std::uint8_t* buf, std::uint16_t len)
 {
-  if (len < Size::MinBuffer || len > Size::MaxBuffer)
-    throw std::length_error(
-        "RouterInfo: " + std::string(__func__) + ": invalid buffer length");
-  if (!m_Buffer)
-    m_Buffer = std::make_unique<std::uint8_t[]>(Size::MaxBuffer);
-  m_BufferLen = len;
+  m_Buffer(buf, len);
   m_IsUpdated = true;
   m_IsUnreachable = false;
   m_SupportedTransports = 0;
   m_Caps = 0;
   m_Addresses.clear();
   m_Options.clear();
-  std::memcpy(m_Buffer.get(), buf, len);
   ReadFromBuffer(true);
   // don't delete buffer until saved to file
 }
 
 const std::uint8_t* RouterInfo::LoadBuffer()
 {
-  if (!m_Buffer)
+  if (!m_Buffer.size())
     {
       ReadFromFile();
       LOG(debug) << "RouterInfo: buffer for " << GetIdentHashAbbreviation()
                  << " loaded from file";
     }
 
-  return m_Buffer.get();
+  return m_Buffer.data();
 }
 
 void RouterInfo::CreateBuffer(const PrivateKeys& private_keys)
@@ -715,23 +694,20 @@ void RouterInfo::CreateBuffer(const PrivateKeys& private_keys)
       // Create RI
       core::StringStream router_info;
       CreateRouterInfo(router_info, private_keys);
-      if (router_info.Str().size() > Size::MaxBuffer)
-        throw std::length_error("created RI is too big");
 
       // Create buffer
-      m_BufferLen = router_info.Str().size();
-      if (!m_Buffer)
-        m_Buffer = std::make_unique<std::uint8_t[]>(Size::MaxBuffer);
-      std::memcpy(m_Buffer.get(), router_info.Str().c_str(), m_BufferLen);
+      m_Buffer(
+          reinterpret_cast<const std::uint8_t*>(router_info.Str().c_str()),
+          router_info.Str().size());
 
       // Signature
       // TODO(anonimal): signing should be done when creating RI, not after. Requires other refactoring.
       private_keys.Sign(
-          reinterpret_cast<std::uint8_t*>(m_Buffer.get()),
-          m_BufferLen,
-          reinterpret_cast<std::uint8_t*>(m_Buffer.get()) + m_BufferLen);
+          m_Buffer.data(),
+          m_Buffer.size(),
+          m_Buffer.data() + m_Buffer.size());
 
-      m_BufferLen += private_keys.GetPublic().GetSignatureLen();
+      m_Buffer(m_Buffer.size() + private_keys.GetPublic().GetSignatureLen());
     }
   catch (...)
     {
@@ -916,11 +892,8 @@ void RouterInfo::SaveToFile(const std::string& path)
   if (stream.Fail())
     throw std::runtime_error("RouterInfo: cannot open " + path);
 
-  // TODO(anonimal): buffer should be guaranteed
-  if (!m_Buffer)
-    throw std::length_error("RouterInfo: cannot save file, buffer is empty");
-
-  if (!stream.Write(m_Buffer.get(), m_BufferLen))
+  assert(m_Buffer.size());
+  if (!stream.Write(m_Buffer.data(), m_Buffer.size()))
     throw std::runtime_error("RouterInfo: cannot save " + path);
 }
 

src/core/router/info.h

@@ -49,6 +49,7 @@
 #include "core/router/identity.h"
 #include "core/router/profiling.h"
 
+#include "core/util/buffer.h"
 #include "core/util/exception.h"
 #include "core/util/tag.h"
 #include "core/util/filesystem.h"
@@ -543,21 +544,22 @@ class RouterInfo : public RouterInfoTraits, public RoutingDestination
 
  public:
   /// @return Pointer to RI buffer
-  const std::uint8_t* GetBuffer() const
+  const std::uint8_t* data() const
   {
-    return m_Buffer.get();
+    return m_Buffer.data();
   }
 
   /// @return RI buffer length
-  std::uint16_t GetBufferLen() const noexcept
+  std::uint16_t size() const noexcept
   {
-    return m_BufferLen;
+    return m_Buffer.size();
   }
 
-  /// @brief Deletes RI buffer
-  void DeleteBuffer()
+  /// @brief Clear RI buffer
+  void clear()
   {
-    m_Buffer.reset(nullptr);
+    // TODO(unassigned): we may also want to clear all options
+    m_Buffer.clear();
   }
 
   /// @return RI's router identity
@@ -802,10 +804,9 @@ class RouterInfo : public RouterInfoTraits, public RoutingDestination
 
  private:
   core::Exception m_Exception;
+  core::Buffer<Size::MinBuffer, Size::MaxBuffer> m_Buffer;
   std::string m_Path;
   IdentityEx m_RouterIdentity;
-  std::unique_ptr<std::uint8_t[]> m_Buffer;
-  std::uint16_t m_BufferLen{};
   std::uint64_t m_Timestamp{};
   std::vector<Address> m_Addresses;
   std::map<std::string, std::string> m_Options;

src/core/router/net_db/impl.cc

@@ -339,7 +339,7 @@ bool NetDb::Load()
                         || timestamp < router->GetTimestamp()
                                     + Time::RouterExpiration))
                   {
-                    router->DeleteBuffer();
+                    router->clear();
                     router->GetOptions().clear();  // options are not used for regular routers  // TODO(anonimal): review
                     m_RouterInfos.insert(std::make_pair(router->GetIdentHash(), router));
                     if (router->HasCap(RouterInfo::Cap::Floodfill))
@@ -391,7 +391,7 @@ void NetDb::SaveUpdated() {
       it.second->SaveToFile(f);
       it.second->SetUpdated(false);
       it.second->SetUnreachable(false);
-      it.second->DeleteBuffer();
+      it.second->clear();
       count++;
     } else {
       // RouterInfo expires after N minutes if it uses an introducer
@@ -714,7 +714,7 @@ void NetDb::HandleDatabaseLookupMsg(
       if (router) {
         LOG(debug) << "NetDb: requested RouterInfo " << key << " found";
         router->LoadBuffer();
-        if (router->GetBuffer())
+        if (router->data())
           reply_msg = CreateDatabaseStoreMsg(router);
       }
     }