ByteStream: add base input interface + internal vector buffer

Referencing #810

src/client/destination.cc

@@ -502,10 +502,8 @@ void ClientDestination::HandleDataMessage(const std::uint8_t* buf, std::size_t)
   // Get I2NP Data message payload size
   std::uint32_t const size = core::InputByteStream::Read<std::uint32_t>(buf);
 
-  // Create payload stream - TODO(anonimal): remove const_cast, see bytestream TODO
-  core::InputByteStream payload(const_cast<std::uint8_t*>(buf + 4), size);
-
   // Assume I2CP payload - TODO(unassigned): don't assume
+  core::InputByteStream payload(buf + 4, size);
   payload.SkipBytes(4);
   std::uint16_t const source_port = payload.Read<std::uint16_t>();
   std::uint16_t const dest_port = payload.Read<std::uint16_t>();

src/core/router/transports/ssu/packet.cc

@@ -701,7 +701,7 @@ std::unique_ptr<SSUHeader> SSUPacketParser::ParseHeader() {
 std::unique_ptr<SSUPacket> SSUPacketParser::ParsePacket() {
   m_Header = ParseHeader();
   std::unique_ptr<SSUPacket> packet;
-  std::uint8_t* const old_data = m_Data;
+  std::uint8_t* const old_data = m_DataPtr;
   const std::size_t old_length = m_Length;
   switch (m_Header->GetPayloadType()) {
     case SSUPayloadType::SessionRequest:
@@ -758,7 +758,7 @@ std::unique_ptr<SSUSessionCreatedPacket> SSUPacketParser::ParseSessionCreated()
   packet->SetPort(Read<std::uint16_t>());
   packet->SetRelayTag(Read<std::uint32_t>());
   packet->SetSignedOnTime(Read<std::uint32_t>());
-  packet->SetSignature(m_Data, m_Length);
+  packet->SetSignature(m_DataPtr, m_Length);
   return packet;
 }
 
@@ -776,7 +776,7 @@ std::unique_ptr<SSUSessionConfirmedPacket> SSUPacketParser::ParseSessionConfirme
       m_Header->GetSize() + init_length - m_Length
       + identity.GetSignatureLen());
   SkipBytes(padding_size);  // Padding
-  packet->SetSignature(m_Data);
+  packet->SetSignature(m_DataPtr);
   return packet;
 }
 
@@ -926,11 +926,11 @@ void SSUPacketBuilder::WriteSessionCreated(SSUSessionCreatedPacket* packet) {
 
 void SSUPacketBuilder::WriteSessionConfirmed(
     SSUSessionConfirmedPacket* packet) {
-  std::uint8_t* const begin = m_Data;
+  std::uint8_t* const begin = m_DataPtr;
   Write<std::uint8_t>(0x01);  // 1 byte info, with 1 fragment
   const std::size_t identity_size = packet->GetRemoteRouterIdentity().GetFullLen();
   Write<std::uint16_t>(identity_size);
-  std::uint8_t* const identity = m_Data;
+  std::uint8_t* const identity = m_DataPtr;
   SkipBytes(identity_size);
   packet->GetRemoteRouterIdentity().ToBuffer(identity, identity_size);
 
@@ -939,8 +939,8 @@ void SSUPacketBuilder::WriteSessionConfirmed(
   // message
   const std::size_t signature_size = packet->GetRemoteRouterIdentity().GetSignatureLen();
   const std::size_t padding_size = GetPaddingSize(
-      packet->GetHeader()->GetSize() + m_Data - begin + signature_size);
-  std::uint8_t* const padding = m_Data;
+      packet->GetHeader()->GetSize() + m_DataPtr - begin + signature_size);
+  std::uint8_t* const padding = m_DataPtr;
   SkipBytes(padding_size);
   kovri::core::RandBytes(padding, padding_size);
   WriteData(packet->GetSignature(), signature_size);

src/core/util/byte_stream.cc

@@ -46,8 +46,23 @@ namespace kovri
 {
 namespace core
 {
+
+// Input
+ByteStream::ByteStream(const std::uint8_t* data, std::size_t len)
+    : m_DataPtr(const_cast<std::uint8_t*>(data)), m_Size(len), m_Length(len), m_Counter{}
+      // TODO(anonimal): remove const cast!
+{
+  assert(data || len);
+
+  if (!data)
+    throw std::invalid_argument("ByteStream: null data");
+  if (!len)
+    throw std::length_error("ByteStream: null length");
+}
+
+// Output
 ByteStream::ByteStream(std::uint8_t* data, std::size_t len)
-    : m_Data(data), m_Size(len), m_Length(len), m_Counter{}
+    : m_DataPtr(data), m_Size(len), m_Length(len), m_Counter{}
 {
   assert(data || len);
 
@@ -57,9 +72,6 @@ ByteStream::ByteStream(std::uint8_t* data, std::size_t len)
     throw std::length_error("ByteStream: null length");
 }
 
-// TODO(anonimal): consider refactoring ByteStream use-cases with a vector in mind
-// TODO(anonimal): though make_unique will always value/zero-initialize upon construction,
-//   assigning a raw pointer will not guarantee that the raw pointer points to initialized memory.
 ByteStream::ByteStream(std::size_t len)
     : m_Size(len), m_Length(len), m_Counter{}
 {
@@ -68,8 +80,8 @@ ByteStream::ByteStream(std::size_t len)
   if (!len)
     throw std::length_error("ByteStream: null length");
 
-  auto data = std::make_unique<std::uint8_t[]>(len);
-  m_Data = data.get();
+  m_Data.reserve(len);
+  m_DataPtr = m_Data.data();
 }
 
 void ByteStream::Advance(std::size_t len)
@@ -79,21 +91,21 @@ void ByteStream::Advance(std::size_t len)
   if (!len || len > m_Length)
     throw std::length_error("ByteStream: invalid length advancement");
 
-  m_Data += len;
+  m_DataPtr += len;
   m_Counter += len;
   m_Length -= len;
 }
 
 // Input
 
-InputByteStream::InputByteStream(std::uint8_t* data, std::size_t len)
+InputByteStream::InputByteStream(const std::uint8_t* data, std::size_t len)
     : ByteStream(data, len)
 {
 }
 
 std::uint8_t* InputByteStream::ReadBytes(std::size_t len)
 {
-  std::uint8_t* ptr = m_Data;
+  std::uint8_t* ptr = m_DataPtr;
   Advance(len);
   return ptr;
 }
@@ -124,7 +136,7 @@ void OutputByteStream::WriteData(
   if (!len)
     throw std::length_error("OutputByteStream: null length");
 
-  std::uint8_t* ptr = m_Data;
+  std::uint8_t* ptr = m_DataPtr;
   Advance(len);
 
   if (!data)

src/core/util/byte_stream.h

@@ -46,16 +46,19 @@ namespace kovri
 {
 namespace core
 {
-// TODO(anonimal): our interfaces should use const pointer - but doing so will break
-//   our current SSU implementation. Finish the SSU rewrite and use const correctness!
 /// @class ByteStream
 /// @brief Base class for I/O byte streaming
 class ByteStream
 {
  public:
-  /// @brief Create stream from/to data buffer
-  /// @param data Buffer to use
-  /// @param len Length to use
+  /// @brief For read-only bytestream
+  /// @param data Buffer to read from
+  /// @param len Total length of buffer
+  explicit ByteStream(const std::uint8_t* data, std::size_t len);
+
+  /// @brief For write-only bytestream
+  /// @param data Buffer to write to
+  /// @param len Total length of buffer
   explicit ByteStream(std::uint8_t* data, std::size_t len);
 
   /// @brief Create stream from/to internal data buffer
@@ -70,7 +73,7 @@ class ByteStream
   /// @return Pointer to the first byte
   const std::uint8_t* Data() const noexcept
   {
-    return m_Data - m_Counter;
+    return m_DataPtr - m_Counter;
   }
 
   /// @brief Total size of stream given at initialization
@@ -84,7 +87,7 @@ class ByteStream
   /// @return Pointer to current byte position
   const std::uint8_t* Tellp() const noexcept
   {
-    return m_Data;
+    return m_DataPtr;
   }
 
   /// @brief Remaining length of the stream after advancement
@@ -94,14 +97,17 @@ class ByteStream
   }
 
  protected:
-  std::uint8_t* m_Data;
+  std::uint8_t* m_DataPtr;  ///< Pointer to existing buffer, external or internal
   std::size_t m_Size, m_Length;
   std::size_t m_Counter;  ///< Counter for amount of incremented data
 
   /// @brief Advances the internal data pointer by the given amount
   /// @param len The amount by which to advance the data pointer
   /// @throw std::length_error if amount exceeds the remaining data length
   void Advance(std::size_t len);
+
+ private:
+  std::vector<std::uint8_t> m_Data;  ///< Internal buffer if no buffer supplied
 };
 
 /// @class InputByteStream
@@ -112,7 +118,7 @@ class InputByteStream : public ByteStream
   /// @brief Constructs the byte stream from a given array of bytes
   /// @param data Pointer to the array of bytes
   /// @param len Length of the array of bytes
-  explicit InputByteStream(std::uint8_t* data, std::size_t len);
+  explicit InputByteStream(const std::uint8_t* data, std::size_t len);
 
   virtual ~InputByteStream() = default;
 
@@ -157,12 +163,13 @@ class InputByteStream : public ByteStream
 class OutputByteStream : public ByteStream
 {
  public:
-  /// @brief Constructs the byte stream from a given array of bytes
+  /// @brief Constructs the byte stream into a given array of bytes
   /// @param data Pointer to the array of bytes
   /// @param len Length of the array of bytes
   explicit OutputByteStream(std::uint8_t* data, std::size_t len);
 
-  /// @brief Constructs the byte stream with a given number of bytes
+  /// @brief Constructs the byte stream using an internal buffer
+  ///   with a given number of bytes
   /// @param len Length of bytes to construct
   explicit OutputByteStream(std::size_t len);
 

tests/unit_tests/core/util/byte_stream.cc

@@ -80,6 +80,12 @@ BOOST_AUTO_TEST_CASE(InputByteStream)
   BOOST_CHECK_EQUAL(input.Read<std::uint8_t>(), m_IPv4Array.at(0));
   BOOST_CHECK_EQUAL(input.ReadBytes(3), &m_IPv4Array.at(1));
   BOOST_CHECK_THROW(input.SkipBytes(1), std::length_error);
+
+  BOOST_CHECK_EQUAL_COLLECTIONS(
+      input.Data(),
+      input.Data() + input.Size(),
+      m_IPv4Array.data(),
+      m_IPv4Array.data() + m_IPv4Array.size());
 }
 
 BOOST_AUTO_TEST_CASE(OutputByteStream)