From: Wez Furlong <wez@fb.com>
Date: Fri, 19 Jan 2018 02:39:48 +0000 (-0800)
Subject: fixup decode logic for fragmented IOBufs
X-Git-Tag: v2018.01.22.00~15
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=93f4d05d584569bdede7e2d124a21c31559ea93e

fixup decode logic for fragmented IOBufs

Summary:
D6635325 exposed a long standing issue with the way
that we were consuming Cursor::length, so let's fix it up!

More details are in the new unit test for this!

Reviewed By: spalamarchuk

Differential Revision: D6755842

fbshipit-source-id: f8b20406c32682892791e7375be577d54d52e0ad
---

diff --git a/folly/experimental/bser/Load.cpp b/folly/experimental/bser/Load.cpp
index a0a49c81..ad390ee8 100644
--- a/folly/experimental/bser/Load.cpp
+++ b/folly/experimental/bser/Load.cpp
@@ -61,7 +61,10 @@ static std::string decodeString(Cursor& curs) {
   }
   str.reserve(size_t(len));
 
-  size_t available = curs.length();
+  // peekBytes will advance over any "empty" IOBuf elements until
+  // it reaches the next one with data, so do that to obtain the
+  // true remaining length.
+  size_t available = curs.peekBytes().size();
   while (available < (size_t)len) {
     if (available == 0) {
       // Saw this case when we decodeHeader was returning the incorrect length
@@ -74,7 +77,7 @@ static std::string decodeString(Cursor& curs) {
     str.append(reinterpret_cast<const char*>(curs.data()), available);
     curs.skipAtMost(available);
     len -= available;
-    available = curs.length();
+    available = curs.peekBytes().size();
   }
 
   str.append(reinterpret_cast<const char*>(curs.data()), size_t(len));
diff --git a/folly/experimental/bser/test/BserTest.cpp b/folly/experimental/bser/test/BserTest.cpp
index 61f07f2b..97bf5c75 100644
--- a/folly/experimental/bser/test/BserTest.cpp
+++ b/folly/experimental/bser/test/BserTest.cpp
@@ -118,5 +118,32 @@ TEST(Bser, PduLength) {
   EXPECT_EQ(len, 44) << "PduLength should be 44, got " << len;
 }
 
+TEST(Bser, CursorLength) {
+  folly::bser::serialization_opts opts;
+  std::string inputStr("hello there please break");
+
+  // This test is exercising the decode logic for pathological
+  // fragmentation cases.  We try a few permutations with the
+  // BSER header being fragmented to tickle boundary conditions
+
+  auto longSerialized = folly::bser::toBser(inputStr, opts);
+  for (uint32_t i = 1; i < longSerialized.size(); ++i) {
+    folly::IOBufQueue q;
+
+    q.append(folly::IOBuf::wrapBuffer(longSerialized.data(), i));
+    uint32_t j = i;
+    while (j < longSerialized.size()) {
+      q.append(folly::IOBuf::wrapBuffer(&longSerialized[j], 1));
+      ++j;
+    }
+
+    auto pdu_len = folly::bser::decodePduLength(q.front());
+    auto buf = q.split(pdu_len);
+
+    auto hello = folly::bser::parseBser(buf.get());
+    EXPECT_EQ(inputStr, hello.asString());
+  }
+}
+
 /* vim:ts=2:sw=2:et:
  */