encoding: Block::fromStream properly handles TLV-LENGTH=0
Block::fromStream implementation is also optimized to reduce
memory copying.
refs #4180
Change-Id: I7cc073330b9ec4a9af4863db62e6c108d220066b
diff --git a/src/encoding/block.cpp b/src/encoding/block.cpp
index 768b624..3aa2701 100644
--- a/src/encoding/block.cpp
+++ b/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 @@
// 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 @@
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>
diff --git a/src/encoding/block.hpp b/src/encoding/block.hpp
index a5f1e1a..91ab51c 100644
--- a/src/encoding/block.hpp
+++ b/src/encoding/block.hpp
@@ -38,7 +38,8 @@
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 @@
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);
diff --git a/tests/unit-tests/encoding/block.t.cpp b/tests/unit-tests/encoding/block.t.cpp
index 479ea43..0036175 100644
--- a/tests/unit-tests/encoding/block.t.cpp
+++ b/tests/unit-tests/encoding/block.t.cpp
@@ -199,21 +199,46 @@
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)