encoding: Block::fromStream properly handles TLV-LENGTH=0

Block::fromStream implementation is also optimized to reduce
memory copying.

refs #4180

Change-Id: I7cc073330b9ec4a9af4863db62e6c108d220066b

src/encoding/block.cpp

@@ -22,7 +22,6 @@
  */
 
 #include "block.hpp"
-#include "block-helpers.hpp"
 #include "buffer-stream.hpp"
 #include "encoding-buffer.hpp"
 #include "tlv.hpp"
@@ -120,7 +119,7 @@ Block::Block(const uint8_t* buf, size_t bufSize)
   // pos now points to TLV-VALUE
 
   if (length > static_cast<uint64_t>(end - pos)) {
-    BOOST_THROW_EXCEPTION(tlv::Error("Not enough data in the buffer to fully parse TLV"));
+    BOOST_THROW_EXCEPTION(Error("Not enough bytes in the buffer to fully parse TLV"));
   }
   size_t typeLengthSize = pos - buf;
   m_size = typeLengthSize + length;
@@ -172,28 +171,28 @@ Block::fromStream(std::istream& is)
 
   uint32_t type = tlv::readType(begin, end);
   uint64_t length = tlv::readVarNumber(begin, end);
-
-  if (length == 0) {
-    // XXX An extra octet is incorrectly consumed from istream (#4180)
-    return Block(type);
+  if (begin != end) {
+    is.putback(*begin);
   }
 
-  if (length > MAX_SIZE_OF_BLOCK_FROM_STREAM) {
-    BOOST_THROW_EXCEPTION(tlv::Error("TLV-LENGTH from stream exceeds limit"));
+  size_t tlSize = tlv::sizeOfVarNumber(type) + tlv::sizeOfVarNumber(length);
+  if (tlSize + length > MAX_SIZE_OF_BLOCK_FROM_STREAM) {
+    BOOST_THROW_EXCEPTION(Error("TLV-LENGTH from stream exceeds limit"));
   }
 
-  // We may still have some problem here, if some exception happens,
-  // we may completely lose all the bytes extracted from the stream.
-
-  char buf[MAX_SIZE_OF_BLOCK_FROM_STREAM];
-  buf[0] = *begin;
-  is.read(buf + 1, length - 1);
-
-  if (length != static_cast<uint64_t>(is.gcount()) + 1) {
-    BOOST_THROW_EXCEPTION(tlv::Error("Not enough data in the buffer to fully parse TLV"));
+  EncodingBuffer eb(tlSize + length, length);
+  uint8_t* valueBuf = eb.buf();
+  is.read(reinterpret_cast<char*>(valueBuf), length);
+  if (length != static_cast<uint64_t>(is.gcount())) {
+    BOOST_THROW_EXCEPTION(Error("Not enough bytes from stream to fully parse TLV"));
   }
 
-  return makeBinaryBlock(type, buf, length);
+  eb.prependVarNumber(length);
+  eb.prependVarNumber(type);
+
+  // TLV-VALUE is directly written into eb.buf(), eb.end() is not incremented, but eb.getBuffer()
+  // has the correct layout.
+  return Block(eb.getBuffer());
 }
 
 std::tuple<bool, Block>

src/encoding/block.hpp

@@ -38,7 +38,8 @@ class const_buffer;
 
 namespace ndn {
 
-/** @brief Class representing a wire element of NDN-TLV packet format
+/** @brief Represents a TLV element of NDN packet format
+ *  @sa https://named-data.net/doc/ndn-tlv/tlv.html
  */
 class Block
 {
@@ -144,6 +145,8 @@ public: // constructor, creation, assignment
   Block(uint32_t type, const Block& value);
 
   /** @brief Parse Block from an input stream
+   *  @throw tlv::Error TLV-LENGTH is zero or exceeds upper bound
+   *  @warning If decoding fails, bytes are still consumed from the input stream.
    */
   static Block
   fromStream(std::istream& is);

tests/unit-tests/encoding/block.t.cpp

@@ -199,21 +199,46 @@ BOOST_AUTO_TEST_CASE(FromStreamWhitespace) // Bug 2728
   b.parse();
 }
 
-BOOST_AUTO_TEST_CASE_EXPECTED_FAILURES(FromStreamZeroLength, 1)
-BOOST_AUTO_TEST_CASE(FromStreamZeroLength) // Bug 2729
+BOOST_AUTO_TEST_CASE(FromStreamZeroLength)
 {
-  const uint8_t BUFFER[] = {0x07, 0x00, // TLV-LENGTH is zero
-                      0x09, 0x01, 0x01};
+  const uint8_t BUFFER[] = {0x70, 0x00,
+                            0x71, 0x03, 0x86, 0x11, 0x24,
+                            0x72, 0x00};
 
   std::stringstream stream;
   stream.write(reinterpret_cast<const char*>(BUFFER), sizeof(BUFFER));
   stream.seekg(0);
 
-  Block b = Block::fromStream(stream);
-  BOOST_CHECK_EQUAL(b.type(), 0x07);
-  BOOST_CHECK_EQUAL(b.value_size(), 0);
+  Block b1 = Block::fromStream(stream);
+  BOOST_CHECK_EQUAL(b1.type(), 0x70);
+  BOOST_CHECK_EQUAL(b1.value_size(), 0);
 
-  BOOST_CHECK_NO_THROW(b = Block::fromStream(stream)); // expected failure Bug 4180
+  Block b2 = Block::fromStream(stream);
+  BOOST_CHECK_EQUAL(b2.type(), 0x71);
+  BOOST_CHECK_EQUAL(b2.value_size(), 3);
+  const uint8_t EXPECTED_VALUE2[] = {0x86, 0x11, 0x24};
+  BOOST_CHECK_EQUAL_COLLECTIONS(b2.value_begin(), b2.value_end(),
+                                EXPECTED_VALUE2, EXPECTED_VALUE2 + sizeof(EXPECTED_VALUE2));
+
+  Block b3 = Block::fromStream(stream);
+  BOOST_CHECK_EQUAL(b3.type(), 0x72);
+  BOOST_CHECK_EQUAL(b3.value_size(), 0);
+
+  BOOST_CHECK_THROW(Block::fromStream(stream), tlv::Error);
+}
+
+BOOST_AUTO_TEST_CASE(FromStreamPacketTooLarge)
+{
+  const uint8_t BUFFER[] = {0x07, 0xfe, 0x00, 0x01, 0x00, 0x00};
+
+  std::stringstream stream;
+  stream.write(reinterpret_cast<const char*>(BUFFER), sizeof(BUFFER));
+  for (int i = 0; i < 0x10000; ++i) {
+    stream.put('\0');
+  }
+  stream.seekg(0);
+
+  BOOST_CHECK_THROW(Block::fromStream(stream), tlv::Error);
 }
 
 BOOST_AUTO_TEST_CASE(FromWireBuffer)