From: Mainak Mandal Date: Wed, 26 Nov 2014 02:27:13 +0000 (-0800) Subject: fix varint decoding in case the first buffer is smaller than encoded varint X-Git-Tag: v0.22.0~142 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=29ca6827b5387ebeeffcd1407492685a3327df25;p=folly.git fix varint decoding in case the first buffer is smaller than encoded varint Summary: This is not a super clean solution, given that I have just copied the code over form Varint.h. Another way would be to add a peek(IOBuf&, size_t) method to CursorBase and use that to peek deeper into the chain. Test Plan: unit tests Reviewed By: tulloch@fb.com Subscribers: trunkagent, njormrod, dcapel, benr, folly-diffs@ FB internal diff: D1689872 Signature: t1:1689872:1416964989:b7f12a2686233f161401288ffcb8c51926b01fdf --- diff --git a/folly/Varint.h b/folly/Varint.h index 2f8739fe..a58866fd 100644 --- a/folly/Varint.h +++ b/folly/Varint.h @@ -108,7 +108,7 @@ inline uint64_t decodeVarint(ByteRange& data) { b = *p++; val |= (b & 0x7f) << 49; if (b >= 0) break; b = *p++; val |= (b & 0x7f) << 56; if (b >= 0) break; b = *p++; val |= (b & 0x7f) << 63; if (b >= 0) break; - throw std::invalid_argument("Invalid varint value"); // too big + throw std::invalid_argument("Invalid varint value. Too big."); } while (false); } else { int shift = 0; @@ -116,7 +116,10 @@ inline uint64_t decodeVarint(ByteRange& data) { val |= static_cast(*p++ & 0x7f) << shift; shift += 7; } - if (p == end) throw std::invalid_argument("Invalid varint value"); + if (p == end) { + throw std::invalid_argument("Invalid varint value. Too small: " + + std::to_string(end - begin) + " bytes"); + } val |= static_cast(*p++) << shift; } diff --git a/folly/io/Compression.cpp b/folly/io/Compression.cpp index d7a0544d..1a820d42 100644 --- a/folly/io/Compression.cpp +++ b/folly/io/Compression.cpp @@ -155,12 +155,19 @@ void encodeVarintToIOBuf(uint64_t val, folly::IOBuf* out) { out->append(encodeVarint(val, out->writableTail())); } -uint64_t decodeVarintFromCursor(folly::io::Cursor& cursor) { - // Must have enough room in *this* buffer. - auto p = cursor.peek(); - folly::ByteRange range(p.first, p.second); - uint64_t val = decodeVarint(range); - cursor.skip(range.data() - p.first); +inline uint64_t decodeVarintFromCursor(folly::io::Cursor& cursor) { + uint64_t val = 0; + int8_t b = 0; + for (int shift = 0; shift <= 63; shift += 7) { + b = cursor.pullByte(); + val |= static_cast(b & 0x7f) << shift; + if (b >= 0) { + break; + } + } + if (b < 0) { + throw std::invalid_argument("Invalid varint value. Too big."); + } return val; } diff --git a/folly/io/test/CompressionTest.cpp b/folly/io/test/CompressionTest.cpp index 51faa4e0..d2c542da 100644 --- a/folly/io/test/CompressionTest.cpp +++ b/folly/io/test/CompressionTest.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include namespace folly { namespace io { namespace test { @@ -129,8 +130,8 @@ TEST(CompressionTestNeedsUncompressedLength, Simple) { ->needsUncompressedLength()); } -class CompressionTest : public testing::TestWithParam< - std::tr1::tuple> { +class CompressionTest + : public testing::TestWithParam> { protected: void SetUp() { auto tup = GetParam(); @@ -149,6 +150,7 @@ void CompressionTest::runSimpleTest(const DataHolder& dh) { auto compressed = codec_->compress(original.get()); if (!codec_->needsUncompressedLength()) { auto uncompressed = codec_->uncompress(compressed.get()); + EXPECT_EQ(uncompressedLength_, uncompressed->computeChainDataLength()); EXPECT_EQ(dh.hash(uncompressedLength_), hashIOBuf(uncompressed.get())); } @@ -171,15 +173,67 @@ TEST_P(CompressionTest, ConstantData) { INSTANTIATE_TEST_CASE_P( CompressionTest, CompressionTest, - testing::Combine( - testing::Values(0, 1, 12, 22, 25, 27), - testing::Values(CodecType::NO_COMPRESSION, - CodecType::LZ4, - CodecType::SNAPPY, - CodecType::ZLIB, - CodecType::LZ4_VARINT_SIZE, - CodecType::LZMA2, - CodecType::LZMA2_VARINT_SIZE))); + testing::Combine(testing::Values(0, 1, 12, 22, 25, 27), + testing::Values(CodecType::NO_COMPRESSION, + CodecType::LZ4, + CodecType::SNAPPY, + CodecType::ZLIB, + CodecType::LZ4_VARINT_SIZE, + CodecType::LZMA2, + CodecType::LZMA2_VARINT_SIZE))); + +class CompressionVarintTest + : public testing::TestWithParam> { + protected: + void SetUp() { + auto tup = GetParam(); + uncompressedLength_ = uint64_t(1) << std::tr1::get<0>(tup); + codec_ = getCodec(std::tr1::get<1>(tup)); + } + + void runSimpleTest(const DataHolder& dh); + + uint64_t uncompressedLength_; + std::unique_ptr codec_; +}; + +inline uint64_t oneBasedMsbPos(uint64_t number) { + uint64_t pos = 0; + for (; number > 0; ++pos, number >>= 1) { + } + return pos; +} + +void CompressionVarintTest::runSimpleTest(const DataHolder& dh) { + auto original = IOBuf::wrapBuffer(dh.data(uncompressedLength_)); + auto compressed = codec_->compress(original.get()); + auto breakPoint = + 1UL + + Random::rand64(std::max(9UL, oneBasedMsbPos(uncompressedLength_)) / 9UL); + auto tinyBuf = IOBuf::copyBuffer(compressed->data(), + std::min(compressed->length(), breakPoint)); + compressed->trimStart(breakPoint); + tinyBuf->prependChain(std::move(compressed)); + compressed = std::move(tinyBuf); + + auto uncompressed = codec_->uncompress(compressed.get()); + + EXPECT_EQ(uncompressedLength_, uncompressed->computeChainDataLength()); + EXPECT_EQ(dh.hash(uncompressedLength_), hashIOBuf(uncompressed.get())); +} + +TEST_P(CompressionVarintTest, RandomData) { runSimpleTest(randomDataHolder); } + +TEST_P(CompressionVarintTest, ConstantData) { + runSimpleTest(constantDataHolder); +} + +INSTANTIATE_TEST_CASE_P( + CompressionVarintTest, + CompressionVarintTest, + testing::Combine(testing::Values(0, 1, 12, 22, 25, 27), + testing::Values(CodecType::LZ4_VARINT_SIZE, + CodecType::LZMA2_VARINT_SIZE))); class CompressionCorruptionTest : public testing::TestWithParam { protected: