encoding: treat TLV-TYPE zero as invalid

 * Use tlv::Invalid to indicate an invalid Block, instead of
   UINT32_MAX (which is a valid type), thus fixing bug #4726
 * Introduce Block::isValid as a replacement for Block::empty
   and soft-deprecate the latter
 * Improve test coverage of Block and tlv::readType

Refs: #4726, #4895
Change-Id: I1cd3336fcbfe83555f3111738da67041dfae64f3
diff --git a/ndn-cxx/encoding/block.cpp b/ndn-cxx/encoding/block.cpp
index 5e51baf..cd90b1f 100644
--- a/ndn-cxx/encoding/block.cpp
+++ b/ndn-cxx/encoding/block.cpp
@@ -288,8 +288,8 @@
 size_t
 Block::size() const
 {
-  if (empty()) {
-    NDN_THROW(Error("Block size cannot be determined (undefined block size)"));
+  if (!isValid()) {
+    NDN_THROW(Error("Cannot determine size of invalid block"));
   }
 
   return m_size;
@@ -487,7 +487,7 @@
 {
   auto oldFmt = os.flags(std::ios_base::dec);
 
-  if (block.empty()) {
+  if (!block.isValid()) {
     os << "[invalid]";
   }
   else if (!block.m_elements.empty()) {
diff --git a/ndn-cxx/encoding/block.hpp b/ndn-cxx/encoding/block.hpp
index 9b81bad..0cd6867 100644
--- a/ndn-cxx/encoding/block.hpp
+++ b/ndn-cxx/encoding/block.hpp
@@ -53,8 +53,8 @@
   };
 
 public: // construction, assignment
-  /** @brief Create an empty Block
-   *  @sa empty()
+  /** @brief Create an invalid Block
+   *  @post `isValid() == false`
    */
   Block();
 
@@ -136,7 +136,6 @@
 
   /** @brief Create a zero-length Block with the specified TLV-TYPE
    *  @param type TLV-TYPE
-   *  @post empty() == false
    */
   explicit
   Block(uint32_t type);
@@ -149,7 +148,7 @@
 
   /** @brief Create a Block with the specified TLV-TYPE and TLV-VALUE
    *  @param type TLV-TYPE
-   *  @param value a Block to be nested as TLV-VALUE, must not be empty
+   *  @param value a Block to be nested as TLV-VALUE, must be valid
    */
   Block(uint32_t type, const Block& value);
 
@@ -164,7 +163,7 @@
    *  @param buffer a Buffer containing a TLV element at offset @p offset
    *  @param offset begin position of the TLV element within @p buffer
    *  @note This function does not throw upon decoding failure.
-   *  @return true and the parsed Block if parsing succeeds; otherwise false and an empty Block
+   *  @return `true` and the parsed Block if parsing succeeds; otherwise `false` and an invalid Block
    */
   static std::tuple<bool, Block>
   fromBuffer(ConstBufferPtr buffer, size_t offset);
@@ -174,33 +173,49 @@
    *  @param bufSize size of the raw buffer; may be greater than the actual size of the TLV element
    *  @note This function does not throw upon decoding failure.
    *  @note This overload copies the TLV element octets into an internal wire buffer.
-   *  @return true and the parsed Block if parsing succeeds; otherwise false and an empty Block
+   *  @return `true` and the parsed Block if parsing succeeds; otherwise `false` and an invalid Block
    */
   static std::tuple<bool, Block>
   fromBuffer(const uint8_t* buf, size_t bufSize);
 
 public: // wire format
+  /** @brief Check if the Block is valid
+   *
+   *  A Block is valid unless it has an invalid TLV-TYPE or is default-constructed.
+   *  In particular, a Block with zero-length TLV-VALUE *is* valid.
+   */
+  bool
+  isValid() const noexcept
+  {
+    return m_type != tlv::Invalid;
+  }
+
   /** @brief Check if the Block is empty
    *
-   *  A Block is considered empty only if it is default-constructed. A Block with a zero-length
-   *  TLV-VALUE is not considered empty.
+   *  A Block is considered empty *iff* it is not valid, i.e., isValid() returns false.
+   *
+   *  @warning Note that an empty Block is *not* the same as a valid but zero-length Block.
+   *  @deprecated Use Block::isValid()
    */
   bool
   empty() const noexcept
   {
-    return m_type == std::numeric_limits<uint32_t>::max();
+    return !isValid();
   }
 
-  /** @brief Reset wire buffer of the element
-   *  @post empty() == true
+  /** @brief Reset the Block to a default-constructed state
+   *
+   *  Equivalent to `*this = Block()`.
+   *
+   *  @post `isValid() == false`
    *  @sa resetWire()
    */
   void
   reset() noexcept;
 
   /** @brief Reset wire buffer but keep TLV-TYPE and sub-elements (if any)
-   *  @post hasWire() == false
-   *  @post hasValue() == false
+   *  @post `hasWire() == false`
+   *  @post `hasValue() == false`
    *  @sa reset()
    */
   void
@@ -218,26 +233,26 @@
   }
 
   /** @brief Get begin iterator of encoded wire
-   *  @pre hasWire() == true
+   *  @pre `hasWire() == true`
    */
   Buffer::const_iterator
   begin() const;
 
   /** @brief Get end iterator of encoded wire
-   *  @pre hasWire() == true
+   *  @pre `hasWire() == true`
    */
   Buffer::const_iterator
   end() const;
 
   /** @brief Return a raw pointer to the beginning of the encoded wire
-   *  @pre hasWire() == true
+   *  @pre `hasWire() == true`
    *  @sa value()
    */
   const uint8_t*
   wire() const;
 
   /** @brief Return the size of the encoded wire, i.e. of the whole TLV
-   *  @pre empty() == false
+   *  @pre `isValid() == true`
    *  @sa value_size()
    */
   size_t
@@ -253,7 +268,7 @@
 
 public: // type and value
   /** @brief Return the TLV-TYPE of the Block
-   *  @pre empty() == false
+   *  @note This will return tlv::Invalid if isValid() is false.
    */
   uint32_t
   type() const
@@ -275,7 +290,7 @@
   }
 
   /** @brief Get begin iterator of TLV-VALUE
-   *  @pre hasValue() == true
+   *  @pre `hasValue() == true`
    */
   Buffer::const_iterator
   value_begin() const
@@ -284,7 +299,7 @@
   }
 
   /** @brief Get end iterator of TLV-VALUE
-   *  @pre hasValue() == true
+   *  @pre `hasValue() == true`
    */
   Buffer::const_iterator
   value_end() const
@@ -341,7 +356,7 @@
 
   /** @brief Remove all sub-elements of the specified TLV-TYPE
    *  @pre parse() has been executed
-   *  @post find(type) == elements_end()
+   *  @post `find(type) == elements_end()`
    */
   void
   remove(uint32_t type);
@@ -428,7 +443,7 @@
 protected:
   /** @brief Underlying buffer storing TLV-VALUE and possibly TLV-TYPE and TLV-LENGTH fields
    *
-   *  If m_buffer is nullptr, this is an empty or zero-length Block with TLV-TYPE given in m_type.
+   *  If m_buffer is nullptr, this is an invalid or zero-length Block with TLV-TYPE given in m_type.
    *  Otherwise,
    *  - [m_valueBegin, m_valueEnd) point to the TLV-VALUE inside m_buffer.
    *  - If m_begin != m_end, [m_begin, m_end) point to Type-Length-Value of this Block in m_buffer.
@@ -441,11 +456,11 @@
   Buffer::const_iterator m_valueBegin; ///< @sa m_buffer
   Buffer::const_iterator m_valueEnd; ///< @sa m_buffer
 
-  uint32_t m_type = std::numeric_limits<uint32_t>::max(); ///< TLV-TYPE
+  uint32_t m_type = tlv::Invalid; ///< TLV-TYPE
 
   /** @brief Total size including Type-Length-Value
    *
-   *  This field is valid only if empty() is false.
+   *  This field is meaningful only if isValid() is true.
    */
   size_t m_size = 0;
 
diff --git a/ndn-cxx/encoding/tlv.hpp b/ndn-cxx/encoding/tlv.hpp
index 1f1ffbf..06a431a 100644
--- a/ndn-cxx/encoding/tlv.hpp
+++ b/ndn-cxx/encoding/tlv.hpp
@@ -60,7 +60,8 @@
 /** @brief TLV-TYPE numbers defined in NDN Packet Format v0.3
  *  @sa https://named-data.net/doc/NDN-packet-spec/current/types.html
  */
-enum {
+enum : uint32_t {
+  Invalid                         = 0,
   Interest                        = 5,
   Data                            = 6,
   Name                            = 7,
@@ -97,7 +98,7 @@
 /** @brief TLV-TYPE numbers defined in NDN Packet Format v0.2 but not in v0.3
  *  @sa https://named-data.net/doc/NDN-packet-spec/0.2.1/types.html
  */
-enum {
+enum : uint32_t {
   Selectors                 = 9,
   MinSuffixComponents       = 13,
   MaxSuffixComponents       = 14,
@@ -113,7 +114,7 @@
 /** @brief TLV-TYPE numbers for typed name components.
  *  @sa https://redmine.named-data.net/projects/ndn-tlv/wiki/NameComponentType
  */
-enum {
+enum : uint32_t {
   KeywordNameComponent     = 32,
   SegmentNameComponent     = 33,
   ByteOffsetNameComponent  = 34,
@@ -194,14 +195,14 @@
  * @brief Read TLV-TYPE.
  * @tparam Iterator an iterator or pointer that dereferences to uint8_t or compatible type
  *
- * @param [inout] begin  Begin of the buffer, will be incremented to point to the first byte after
- *                       the read TLV-TYPE
+ * @param [inout] begin Begin of the buffer, will be incremented to point to the first byte after
+ *                      the read TLV-TYPE
  * @param [in]    end   End of the buffer
  * @param [out]   type  Read TLV-TYPE
  *
  * @return true if TLV-TYPE was successfully read from input, false otherwise
  * @note This function is largely equivalent to readVarNumber(), except that it returns false if
- *       the TLV-TYPE is larger than 2^32-1 (TLV-TYPE in this library is implemented as `uint32_t`)
+ *       the TLV-TYPE is zero or larger than 2^32-1 (maximum allowed by the packet format).
  */
 template<typename Iterator>
 bool
@@ -231,7 +232,7 @@
  *
  * @throw tlv::Error TLV-TYPE cannot be read
  * @note This function is largely equivalent to readVarNumber(), except that it throws if
- *       the TLV-TYPE is larger than 2^32-1 (TLV-TYPE in this library is implemented as `uint32_t`)
+ *       the TLV-TYPE is zero or larger than 2^32-1 (maximum allowed by the packet format).
  */
 template<typename Iterator>
 uint32_t
@@ -409,7 +410,7 @@
 {
   uint64_t number = 0;
   bool isOk = readVarNumber(begin, end, number);
-  if (!isOk || number > std::numeric_limits<uint32_t>::max()) {
+  if (!isOk || number == Invalid || number > std::numeric_limits<uint32_t>::max()) {
     return false;
   }
 
@@ -439,8 +440,8 @@
 readType(Iterator& begin, Iterator end)
 {
   uint64_t type = readVarNumber(begin, end);
-  if (type > std::numeric_limits<uint32_t>::max()) {
-    NDN_THROW(Error("TLV-TYPE number exceeds allowed maximum"));
+  if (type == Invalid || type > std::numeric_limits<uint32_t>::max()) {
+    NDN_THROW(Error("Illegal TLV-TYPE " + to_string(type)));
   }
 
   return static_cast<uint32_t>(type);
diff --git a/ndn-cxx/name-component.cpp b/ndn-cxx/name-component.cpp
index 5e2a9b3..b19eacc 100644
--- a/ndn-cxx/name-component.cpp
+++ b/ndn-cxx/name-component.cpp
@@ -59,7 +59,6 @@
       break;
     default:
       NDN_THROW(std::invalid_argument("Unknown naming convention"));
-      break;
   }
 }
 
@@ -153,15 +152,15 @@
     return parseUriEscapedValue(tlv::GenericNameComponent, input.data(), input.size());
   }
 
-  long type = std::strtol(input.data(), nullptr, 10);
+  auto typePrefix = input.substr(0, equalPos);
+  auto type = std::strtoul(typePrefix.data(), nullptr, 10);
   if (type >= tlv::NameComponentMin && type <= tlv::NameComponentMax &&
-      to_string(type).size() == equalPos) {
+      to_string(type) == typePrefix) {
     size_t valuePos = equalPos + 1;
-    return parseUriEscapedValue(static_cast<uint32_t>(type), input.data() + valuePos,
-                                input.size() - valuePos);
+    return parseUriEscapedValue(static_cast<uint32_t>(type),
+                                input.data() + valuePos, input.size() - valuePos);
   }
 
-  auto typePrefix = input.substr(0, equalPos);
   auto ct = detail::getComponentTypeTable().findByUriPrefix(typePrefix);
   if (ct == nullptr) {
     NDN_THROW(Error("Incorrect TLV-TYPE '" + typePrefix + "' in NameComponent URI"));
diff --git a/tests/unit/encoding/block.t.cpp b/tests/unit/encoding/block.t.cpp
index 18126e8..f6b245e 100644
--- a/tests/unit/encoding/block.t.cpp
+++ b/tests/unit/encoding/block.t.cpp
@@ -25,6 +25,8 @@
 #include "tests/boost-test.hpp"
 
 #include <boost/lexical_cast.hpp>
+#include <boost/mpl/vector.hpp>
+
 #include <cstring>
 #include <sstream>
 
@@ -37,15 +39,27 @@
 BOOST_AUTO_TEST_SUITE(Construction)
 
 static const uint8_t TEST_BUFFER[] = {
-  0x00, 0x01, 0xfa, // ok
-  0x01, 0x01, 0xfb, // ok
-  0x03, 0x02, 0xff // bad: TLV-LENGTH is 2 but there's only 1-octet TLV-VALUE
+  0x42, 0x01, 0xfa,
+  0x01, 0x01, 0xfb,
+  0xfe, 0xff, 0xff, 0xff, 0xff, 0x00, // bug #4726
 };
 
-BOOST_AUTO_TEST_CASE(Empty)
+BOOST_AUTO_TEST_CASE(Default)
 {
   Block b;
+
+  BOOST_CHECK_EQUAL(b.isValid(), false);
   BOOST_CHECK_EQUAL(b.empty(), true);
+  BOOST_CHECK_EQUAL(b.type(), tlv::Invalid);
+  BOOST_CHECK_EQUAL(b.hasValue(), false);
+  BOOST_CHECK_EQUAL(b.value_size(), 0);
+  BOOST_CHECK(b.value() == nullptr);
+
+  BOOST_CHECK_THROW(b.size(), Block::Error);
+  BOOST_CHECK_THROW(b.begin(), Block::Error);
+  BOOST_CHECK_THROW(b.end(), Block::Error);
+  BOOST_CHECK_THROW(b.wire(), Block::Error);
+  BOOST_CHECK_THROW(b.blockFromValue(), Block::Error);
 }
 
 BOOST_AUTO_TEST_CASE(FromEncodingBuffer)
@@ -134,18 +148,25 @@
 BOOST_AUTO_TEST_CASE(FromType)
 {
   Block b1(4);
-  BOOST_CHECK_EQUAL(b1.empty(), false);
+  BOOST_CHECK_EQUAL(b1.isValid(), true);
   BOOST_CHECK_EQUAL(b1.type(), 4);
   BOOST_CHECK_EQUAL(b1.size(), 2); // 1-octet TLV-TYPE and 1-octet TLV-LENGTH
   BOOST_CHECK_EQUAL(b1.hasValue(), false);
   BOOST_CHECK_EQUAL(b1.value_size(), 0);
 
   Block b2(258);
-  BOOST_CHECK_EQUAL(b2.empty(), false);
+  BOOST_CHECK_EQUAL(b2.isValid(), true);
   BOOST_CHECK_EQUAL(b2.type(), 258);
   BOOST_CHECK_EQUAL(b2.size(), 4); // 3-octet TLV-TYPE and 1-octet TLV-LENGTH
   BOOST_CHECK_EQUAL(b2.hasValue(), false);
   BOOST_CHECK_EQUAL(b2.value_size(), 0);
+
+  Block b3(tlv::Invalid);
+  BOOST_CHECK_EQUAL(b3.isValid(), false);
+  BOOST_CHECK_EQUAL(b3.type(), tlv::Invalid);
+  BOOST_CHECK_EQUAL(b3.hasValue(), false);
+  BOOST_CHECK_EQUAL(b3.value_size(), 0);
+  BOOST_CHECK(b3.value() == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(FromTypeAndBuffer)
@@ -154,7 +175,7 @@
   auto bufferPtr = make_shared<Buffer>(VALUE, sizeof(VALUE));
 
   Block b(42, bufferPtr);
-  BOOST_CHECK_EQUAL(b.empty(), false);
+  BOOST_CHECK_EQUAL(b.isValid(), true);
   BOOST_CHECK_EQUAL(b.type(), 42);
   BOOST_CHECK_EQUAL(b.size(), 6);
   BOOST_CHECK_EQUAL(b.hasValue(), true);
@@ -167,7 +188,7 @@
   Block nested(BUFFER, sizeof(BUFFER));
 
   Block b(84, nested);
-  BOOST_CHECK_EQUAL(b.empty(), false);
+  BOOST_CHECK_EQUAL(b.isValid(), true);
   BOOST_CHECK_EQUAL(b.type(), 84);
   BOOST_CHECK_EQUAL(b.size(), 10);
   BOOST_CHECK_EQUAL(b.hasValue(), true);
@@ -181,10 +202,10 @@
   stream.seekg(0);
 
   Block b = Block::fromStream(stream);
-  BOOST_CHECK_EQUAL(b.type(), 0);
+  BOOST_CHECK_EQUAL(b.type(), 66);
   BOOST_CHECK_EQUAL(b.size(), 3);
   BOOST_CHECK_EQUAL(b.value_size(), 1);
-  BOOST_CHECK_EQUAL(*b.wire(),  0x00);
+  BOOST_CHECK_EQUAL(*b.wire(),  0x42);
   BOOST_CHECK_EQUAL(*b.value(), 0xfa);
 
   b = Block::fromStream(stream);
@@ -194,6 +215,13 @@
   BOOST_CHECK_EQUAL(*b.wire(),  0x01);
   BOOST_CHECK_EQUAL(*b.value(), 0xfb);
 
+  b = Block::fromStream(stream);
+  BOOST_CHECK_EQUAL(b.type(), 0xffffffff);
+  BOOST_CHECK_EQUAL(b.size(), 6);
+  BOOST_CHECK_EQUAL(b.value_size(), 0);
+  BOOST_CHECK_EQUAL(*b.wire(),  0xfe);
+
+  BOOST_CHECK(stream.eof());
   BOOST_CHECK_THROW(Block::fromStream(stream), tlv::Error);
 }
 
@@ -222,6 +250,7 @@
   BOOST_CHECK_EQUAL(b.type(), 6);
   BOOST_CHECK_EQUAL(b.value_size(), 32);
   b.parse();
+  BOOST_CHECK_EQUAL(b.elements_size(), 5);
 }
 
 BOOST_AUTO_TEST_CASE(FromStreamZeroLength)
@@ -268,17 +297,17 @@
 
 BOOST_AUTO_TEST_CASE(FromWireBuffer)
 {
-  ConstBufferPtr buffer = make_shared<Buffer>(TEST_BUFFER, sizeof(TEST_BUFFER));
+  auto buffer = make_shared<Buffer>(TEST_BUFFER, sizeof(TEST_BUFFER));
 
   size_t offset = 0;
   bool isOk = false;
   Block b;
   std::tie(isOk, b) = Block::fromBuffer(buffer, offset);
   BOOST_CHECK(isOk);
-  BOOST_CHECK_EQUAL(b.type(), 0);
+  BOOST_CHECK_EQUAL(b.type(), 66);
   BOOST_CHECK_EQUAL(b.size(), 3);
   BOOST_CHECK_EQUAL(b.value_size(), 1);
-  BOOST_CHECK_EQUAL(*b.wire(),  0x00);
+  BOOST_CHECK_EQUAL(*b.wire(),  0x42);
   BOOST_CHECK_EQUAL(*b.value(), 0xfa);
   offset += b.size();
 
@@ -292,7 +321,11 @@
   offset += b.size();
 
   std::tie(isOk, b) = Block::fromBuffer(buffer, offset);
-  BOOST_CHECK(!isOk);
+  BOOST_CHECK(isOk);
+  BOOST_CHECK_EQUAL(b.type(), 0xffffffff);
+  BOOST_CHECK_EQUAL(b.size(), 6);
+  BOOST_CHECK_EQUAL(b.value_size(), 0);
+  BOOST_CHECK_EQUAL(*b.wire(),  0xfe);
 }
 
 BOOST_AUTO_TEST_CASE(FromRawBuffer)
@@ -302,10 +335,10 @@
   Block b;
   std::tie(isOk, b) = Block::fromBuffer(TEST_BUFFER + offset, sizeof(TEST_BUFFER) - offset);
   BOOST_CHECK(isOk);
-  BOOST_CHECK_EQUAL(b.type(), 0);
+  BOOST_CHECK_EQUAL(b.type(), 66);
   BOOST_CHECK_EQUAL(b.size(), 3);
   BOOST_CHECK_EQUAL(b.value_size(), 1);
-  BOOST_CHECK_EQUAL(*b.wire(),  0x00);
+  BOOST_CHECK_EQUAL(*b.wire(),  0x42);
   BOOST_CHECK_EQUAL(*b.value(), 0xfa);
   offset += b.size();
 
@@ -319,7 +352,66 @@
   offset += b.size();
 
   std::tie(isOk, b) = Block::fromBuffer(TEST_BUFFER + offset, sizeof(TEST_BUFFER) - offset);
+  BOOST_CHECK(isOk);
+  BOOST_CHECK_EQUAL(b.type(), 0xffffffff);
+  BOOST_CHECK_EQUAL(b.size(), 6);
+  BOOST_CHECK_EQUAL(b.value_size(), 0);
+  BOOST_CHECK_EQUAL(*b.wire(),  0xfe);
+}
+
+template<typename T>
+struct MalformedInput
+{
+  static const std::vector<uint8_t> INPUT;
+};
+
+template<>
+const std::vector<uint8_t> MalformedInput<struct TlvTypeZero>::INPUT{
+  0x00, 0x00
+};
+template<>
+const std::vector<uint8_t> MalformedInput<struct TlvTypeTooLarge>::INPUT{
+  0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+template<>
+const std::vector<uint8_t> MalformedInput<struct BadTlvLength>::INPUT{
+  0x01, 0xff, 0x42, 0x42
+};
+template<>
+const std::vector<uint8_t> MalformedInput<struct TruncatedTlvValue>::INPUT{
+  0x01, 0x02, 0x03
+};
+
+using MalformedInputs = boost::mpl::vector<
+  MalformedInput<TlvTypeZero>,
+  MalformedInput<TlvTypeTooLarge>,
+  MalformedInput<BadTlvLength>,
+  MalformedInput<TruncatedTlvValue>
+>;
+
+BOOST_AUTO_TEST_CASE_TEMPLATE(Malformed, T, MalformedInputs)
+{
+  // constructor from raw buffer
+  BOOST_CHECK_THROW(Block(T::INPUT.data(), T::INPUT.size()), tlv::Error);
+
+  // fromStream()
+  std::stringstream stream;
+  stream.write(reinterpret_cast<const char*>(T::INPUT.data()), T::INPUT.size());
+  stream.seekg(0);
+  BOOST_CHECK_THROW(Block::fromStream(stream), tlv::Error);
+
+  // fromBuffer(), ConstBufferPtr overload
+  auto buf = make_shared<Buffer>(T::INPUT.begin(), T::INPUT.end());
+  bool isOk;
+  Block b;
+  std::tie(isOk, b) = Block::fromBuffer(buf, 0);
   BOOST_CHECK(!isOk);
+  BOOST_CHECK(!b.isValid());
+
+  // fromBuffer(), raw buffer overload
+  std::tie(isOk, b) = Block::fromBuffer(T::INPUT.data(), T::INPUT.size());
+  BOOST_CHECK(!isOk);
+  BOOST_CHECK(!b.isValid());
 }
 
 BOOST_AUTO_TEST_SUITE_END() // Construction
@@ -341,7 +433,6 @@
                 0x1c, 0x00, // KeyLocator empty
           0x17, 0x00 // SignatureValue empty
   };
-
   Block data(PACKET, sizeof(PACKET));
   data.parse();
 
@@ -354,6 +445,13 @@
 
   BOOST_CHECK(data.find(0x15) == data.elements_begin() + 2);
   BOOST_CHECK(data.find(0x01) == data.elements_end());
+
+  const uint8_t MALFORMED[] = {
+    // TLV-LENGTH of nested element is greater than TLV-LENGTH of enclosing element
+    0x05, 0x05, 0x07, 0x07, 0x08, 0x05, 0x68, 0x65, 0x6c, 0x6c, 0x6f
+  };
+  Block bad(MALFORMED, sizeof(MALFORMED));
+  BOOST_CHECK_THROW(bad.parse(), Block::Error);
 }
 
 BOOST_AUTO_TEST_CASE(InsertBeginning)
@@ -483,15 +581,14 @@
   block.push_back(makeNonNegativeIntegerBlock(tlv::ContentType, 1));
 
   BOOST_CHECK_EQUAL(5, block.elements_size());
-  BOOST_REQUIRE_NO_THROW(block.remove(tlv::ContentType));
-  BOOST_CHECK_EQUAL(2, block.elements_size());
+  BOOST_CHECK_NO_THROW(block.remove(tlv::ContentType));
+  BOOST_REQUIRE_EQUAL(2, block.elements_size());
 
-  Block::element_container elements = block.elements();
-
+  auto elements = block.elements();
   BOOST_CHECK_EQUAL(tlv::FreshnessPeriod, elements[0].type());
   BOOST_CHECK_EQUAL(123, readNonNegativeInteger(elements[0]));
   BOOST_CHECK_EQUAL(tlv::Name, elements[1].type());
-  BOOST_CHECK(readString(elements[1]).compare("ndn:/test-prefix") == 0);
+  BOOST_CHECK_EQUAL(readString(elements[1]).compare("ndn:/test-prefix"), 0);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // SubElements
@@ -557,8 +654,8 @@
 
 BOOST_AUTO_TEST_CASE(Simple)
 {
-  Block b0 = "0000"_block;
-  BOOST_CHECK_EQUAL(b0.type(), 0x00);
+  Block b0 = "4200"_block;
+  BOOST_CHECK_EQUAL(b0.type(), 0x42);
   BOOST_CHECK_EQUAL(b0.value_size(), 0);
 
   Block b1 = "0101A0"_block;
@@ -581,9 +678,11 @@
   BOOST_CHECK_THROW(""_block, std::invalid_argument);
   BOOST_CHECK_THROW("1"_block, std::invalid_argument);
   BOOST_CHECK_THROW("333"_block, std::invalid_argument);
+  BOOST_CHECK_THROW("xx yy zz"_block, std::invalid_argument); // only comments
 
-  BOOST_CHECK_THROW("0202C0"_block, tlv::Error);
-  BOOST_CHECK_THROW("0201C0C1"_block, tlv::Error);
+  BOOST_CHECK_THROW("0000"_block, tlv::Error); // invalid type
+  BOOST_CHECK_THROW("0202C0"_block, tlv::Error); // truncated value
+  BOOST_CHECK_THROW("0201C0C1"_block, tlv::Error); // trailing garbage
 }
 
 BOOST_AUTO_TEST_SUITE_END() // BlockLiteral
diff --git a/tests/unit/encoding/tlv.t.cpp b/tests/unit/encoding/tlv.t.cpp
index 8c73267..1810f01 100644
--- a/tests/unit/encoding/tlv.t.cpp
+++ b/tests/unit/encoding/tlv.t.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2018 Regents of the University of California.
+ * Copyright (c) 2013-2019 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -112,6 +112,7 @@
 }
 
 static const uint8_t BUFFER[] = {
+  0x00, // == 0
   0x01, // == 1
   0xfc, // == 252
   0xfd, 0x00, 0xfd, // == 253
@@ -121,10 +122,12 @@
 
 BOOST_AUTO_TEST_CASE(SizeOf)
 {
+  BOOST_CHECK_EQUAL(sizeOfVarNumber(0), 1);
   BOOST_CHECK_EQUAL(sizeOfVarNumber(1), 1);
   BOOST_CHECK_EQUAL(sizeOfVarNumber(252), 1);
   BOOST_CHECK_EQUAL(sizeOfVarNumber(253), 3);
   BOOST_CHECK_EQUAL(sizeOfVarNumber(65536), 5);
+  BOOST_CHECK_EQUAL(sizeOfVarNumber(4294967295), 5);
   BOOST_CHECK_EQUAL(sizeOfVarNumber(4294967296), 9);
 }
 
@@ -132,6 +135,7 @@
 {
   std::ostringstream os;
 
+  writeVarNumber(os, 0);
   writeVarNumber(os, 1);
   writeVarNumber(os, 252);
   writeVarNumber(os, 253);
@@ -139,7 +143,7 @@
   writeVarNumber(os, 4294967296);
 
   std::string buffer = os.str();
-  const uint8_t* actual = reinterpret_cast<const uint8_t*>(buffer.c_str());
+  const uint8_t* actual = reinterpret_cast<const uint8_t*>(buffer.data());
 
   BOOST_CHECK_EQUAL(buffer.size(), sizeof(BUFFER));
   BOOST_CHECK_EQUAL_COLLECTIONS(BUFFER, BUFFER + sizeof(BUFFER),
@@ -155,60 +159,65 @@
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 1, value), true);
   begin = BUFFER;
   BOOST_CHECK_NO_THROW(readVarNumber(begin, begin + 1));
-  BOOST_CHECK_EQUAL(value, 1);
+  BOOST_CHECK_EQUAL(value, 0);
 
   begin = BUFFER + 1;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 1, value), true);
   begin = BUFFER + 1;
   BOOST_CHECK_NO_THROW(readVarNumber(begin, begin + 1));
+  BOOST_CHECK_EQUAL(value, 1);
+
+  begin = BUFFER + 2;
+  BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 1, value), true);
+  begin = BUFFER + 2;
+  BOOST_CHECK_NO_THROW(readVarNumber(begin, begin + 1));
   BOOST_CHECK_EQUAL(value, 252);
 
-  begin = BUFFER + 2;
+  begin = BUFFER + 3;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 1, value), false);
-  begin = BUFFER + 2;
+  begin = BUFFER + 3;
   BOOST_CHECK_THROW(readVarNumber(begin, begin + 1), Error);
 
-  begin = BUFFER + 2;
+  begin = BUFFER + 3;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 2, value), false);
-  begin = BUFFER + 2;
+  begin = BUFFER + 3;
   BOOST_CHECK_THROW(readVarNumber(begin, begin + 2), Error);
 
-  begin = BUFFER + 2;
+  begin = BUFFER + 3;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 3, value), true);
-  begin = BUFFER + 2;
+  begin = BUFFER + 3;
   BOOST_CHECK_NO_THROW(readVarNumber(begin, begin + 3));
   BOOST_CHECK_EQUAL(value, 253);
 
-
-  begin = BUFFER + 5;
+  begin = BUFFER + 6;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 1, value), false);
-  begin = BUFFER + 5;
+  begin = BUFFER + 6;
   BOOST_CHECK_THROW(readVarNumber(begin, begin + 1), Error);
 
-  begin = BUFFER + 5;
+  begin = BUFFER + 6;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 4, value), false);
-  begin = BUFFER + 5;
+  begin = BUFFER + 6;
   BOOST_CHECK_THROW(readVarNumber(begin, begin + 4), Error);
 
-  begin = BUFFER + 5;
+  begin = BUFFER + 6;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 5, value), true);
-  begin = BUFFER + 5;
+  begin = BUFFER + 6;
   BOOST_CHECK_NO_THROW(readVarNumber(begin, begin + 5));
   BOOST_CHECK_EQUAL(value, 65536);
 
-  begin = BUFFER + 10;
+  begin = BUFFER + 11;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 1, value), false);
-  begin = BUFFER + 10;
+  begin = BUFFER + 11;
   BOOST_CHECK_THROW(readVarNumber(begin, begin + 1), Error);
 
-  begin = BUFFER + 10;
+  begin = BUFFER + 11;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 8, value), false);
-  begin = BUFFER + 10;
+  begin = BUFFER + 11;
   BOOST_CHECK_THROW(readVarNumber(begin, begin + 8), Error);
 
-  begin = BUFFER + 10;
+  begin = BUFFER + 11;
   BOOST_CHECK_EQUAL(readVarNumber(begin, begin + 9, value), true);
-  begin = BUFFER + 10;
+  begin = BUFFER + 11;
   BOOST_CHECK_NO_THROW(readVarNumber(begin, begin + 9));
   BOOST_CHECK_EQUAL(value, 4294967296);
 }
@@ -217,6 +226,7 @@
 {
   StreamIterator end; // end of stream
   uint64_t value;
+
   {
     ArrayStream stream(reinterpret_cast<const char*>(BUFFER), 1);
     StreamIterator begin(stream);
@@ -226,118 +236,130 @@
     ArrayStream stream(reinterpret_cast<const char*>(BUFFER), 1);
     StreamIterator begin(stream);
     BOOST_CHECK_NO_THROW(readVarNumber(begin, end));
+    BOOST_CHECK_EQUAL(value, 0);
+  }
+
+  {
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 1, 1);
+    StreamIterator begin(stream);
+    BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), true);
+  }
+  {
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 1, 1);
+    StreamIterator begin(stream);
+    BOOST_CHECK_NO_THROW(readVarNumber(begin, end));
     BOOST_CHECK_EQUAL(value, 1);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 1, 1);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 2, 1);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), true);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 1, 1);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 2, 1);
     StreamIterator begin(stream);
     BOOST_CHECK_NO_THROW(readVarNumber(begin, end));
     BOOST_CHECK_EQUAL(value, 252);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 2, 1);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 3, 1);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), false);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 2, 1);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 3, 1);
     StreamIterator begin(stream);
     BOOST_CHECK_THROW(readVarNumber(begin, end), Error);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 2, 2);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 3, 2);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), false);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 2, 2);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 3, 2);
     StreamIterator begin(stream);
     BOOST_CHECK_THROW(readVarNumber(begin, end), Error);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 2, 3);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 3, 3);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), true);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 2, 3);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 3, 3);
     StreamIterator begin(stream);
     BOOST_CHECK_NO_THROW(readVarNumber(begin, end));
     BOOST_CHECK_EQUAL(value, 253);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 5, 1);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 6, 1);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), false);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 5, 1);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 6, 1);
     StreamIterator begin(stream);
     BOOST_CHECK_THROW(readVarNumber(begin, end), Error);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 5, 4);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 6, 4);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), false);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 5, 4);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 6, 4);
     StreamIterator begin(stream);
     BOOST_CHECK_THROW(readVarNumber(begin, end), Error);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 5, 5);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 6, 5);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), true);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 5, 5);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 6, 5);
     StreamIterator begin(stream);
     BOOST_CHECK_NO_THROW(readVarNumber(begin, end));
     BOOST_CHECK_EQUAL(value, 65536);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 10, 1);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 11, 1);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), false);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 10, 1);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 11, 1);
     StreamIterator begin(stream);
     BOOST_CHECK_THROW(readVarNumber(begin, end), Error);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 10, 8);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 11, 8);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), false);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 10, 8);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 11, 8);
     StreamIterator begin(stream);
     BOOST_CHECK_THROW(readVarNumber(begin, end), Error);
   }
 
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 10, 9);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 11, 9);
     StreamIterator begin(stream);
     BOOST_CHECK_EQUAL(readVarNumber(begin, end, value), true);
   }
   {
-    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 10, 9);
+    ArrayStream stream(reinterpret_cast<const char*>(BUFFER) + 11, 9);
     StreamIterator begin(stream);
     BOOST_CHECK_NO_THROW(readVarNumber(begin, end));
     BOOST_CHECK_EQUAL(value, 4294967296);
@@ -346,6 +368,72 @@
 
 BOOST_AUTO_TEST_SUITE_END() // VarNumber
 
+BOOST_AUTO_TEST_SUITE(Type)
+
+static const uint8_t BUFFER[] = {
+  0x00, // == 0 (illegal)
+  0x01, // == 1
+  0xfd, 0x00, 0xfd, // == 253
+  0xfe, 0xff, 0xff, 0xff, 0xff, // == 4294967295
+  0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00 // == 4294967296 (illegal)
+};
+
+BOOST_AUTO_TEST_CASE(Read)
+{
+  const uint8_t* begin;
+  uint32_t type;
+
+  begin = BUFFER;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 1, type), false);
+  begin = BUFFER;
+  BOOST_CHECK_THROW(readType(begin, begin + 1), Error);
+
+  begin = BUFFER + 1;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 1, type), true);
+  begin = BUFFER + 1;
+  BOOST_CHECK_NO_THROW(readType(begin, begin + 1));
+  BOOST_CHECK_EQUAL(type, 1);
+
+  begin = BUFFER + 2;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 1, type), false);
+  begin = BUFFER + 2;
+  BOOST_CHECK_THROW(readType(begin, begin + 1), Error);
+
+  begin = BUFFER + 2;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 2, type), false);
+  begin = BUFFER + 2;
+  BOOST_CHECK_THROW(readType(begin, begin + 2), Error);
+
+  begin = BUFFER + 2;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 3, type), true);
+  begin = BUFFER + 2;
+  BOOST_CHECK_NO_THROW(readType(begin, begin + 3));
+  BOOST_CHECK_EQUAL(type, 253);
+
+  begin = BUFFER + 5;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 1, type), false);
+  begin = BUFFER + 5;
+  BOOST_CHECK_THROW(readType(begin, begin + 1), Error);
+
+  begin = BUFFER + 5;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 5, type), true);
+  begin = BUFFER + 5;
+  BOOST_CHECK_NO_THROW(readType(begin, begin + 5));
+  BOOST_CHECK_EQUAL(type, 4294967295);
+
+  begin = BUFFER + 10;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 1, type), false);
+  begin = BUFFER + 10;
+  BOOST_CHECK_THROW(readType(begin, begin + 1), Error);
+
+  begin = BUFFER + 10;
+  BOOST_CHECK_EQUAL(readType(begin, begin + 9, type), false);
+  begin = BUFFER + 10;
+  BOOST_CHECK_THROW(readType(begin, begin + 9), Error);
+}
+
+BOOST_AUTO_TEST_SUITE_END() // Type
+
 BOOST_AUTO_TEST_SUITE(NonNegativeInteger)
 
 // This check ensures readNonNegativeInteger only requires InputIterator concept and nothing more.
@@ -388,7 +476,7 @@
   writeNonNegativeInteger(os, 72340172838076674);
 
   std::string buffer = os.str();
-  const uint8_t* actual = reinterpret_cast<const uint8_t*>(buffer.c_str());
+  const uint8_t* actual = reinterpret_cast<const uint8_t*>(buffer.data());
 
   BOOST_CHECK_EQUAL_COLLECTIONS(BUFFER, BUFFER + sizeof(BUFFER),
                                 actual, actual + sizeof(BUFFER));
diff --git a/tests/unit/name-component.t.cpp b/tests/unit/name-component.t.cpp
index 6f5bffc..4ee25ea 100644
--- a/tests/unit/name-component.t.cpp
+++ b/tests/unit/name-component.t.cpp
@@ -141,7 +141,7 @@
 BOOST_AUTO_TEST_CASE(InvalidType)
 {
   Component comp;
-  BOOST_CHECK_THROW(comp.wireDecode("0001 80"_block), Component::Error);
+  BOOST_CHECK_THROW(comp.wireDecode(Block{}), Component::Error);
   BOOST_CHECK_THROW(comp.wireDecode("FE0001000001 80"_block), Component::Error);
 
   BOOST_CHECK_THROW(Component::fromEscapedString("0=A"), Component::Error);