Fix infinite loop in Cursor::readTerminatedString

Summary: readTerminatedString could infinite loop if the terminator does not appear in the contained IOBuf chain and maxLength > chain.computeChainLength.  I'm throwing out_of_range here because that more closely mirrors what the other read() functions do.

Reviewed By: siyengar

Differential Revision: D2571039

fb-gh-sync-id: 1db22089562d8767920d66a0a1b091b02de6571f

folly/io/Cursor.h

@@ -212,7 +212,7 @@ class CursorBase {
       size_t maxLength = std::numeric_limits<size_t>::max()) {
     std::string str;
 
-    for (;;) {
+    while (!isAtEnd()) {
       const uint8_t* buf = data();
       size_t buflen = length();
 
@@ -235,6 +235,7 @@ class CursorBase {
 
       skip(i);
     }
+    throw std::out_of_range("terminator not found");
   }
 
   size_t skipAtMost(size_t len) {

folly/io/test/IOBufCursorTest.cpp

@@ -683,6 +683,33 @@ TEST(IOBuf, StringOperations) {
     EXPECT_STREQ("hello", curs.readTerminatedString().c_str());
   }
 
+  // Test reading a null-terminated string from a chain that doesn't contain the
+  // terminator
+  {
+    std::unique_ptr<IOBuf> buf(IOBuf::create(8));
+    Appender app(buf.get(), 0);
+    app.push(reinterpret_cast<const uint8_t*>("hello"), 5);
+    std::unique_ptr<IOBuf> chain(IOBuf::create(8));
+    chain->prependChain(std::move(buf));
+
+    Cursor curs(chain.get());
+    EXPECT_THROW(curs.readTerminatedString(),
+                 std::out_of_range);
+  }
+
+  // Test reading a null-terminated string past the maximum length
+  {
+    std::unique_ptr<IOBuf> buf(IOBuf::create(8));
+    Appender app(buf.get(), 0);
+    app.push(reinterpret_cast<const uint8_t*>("hello\0"), 6);
+    std::unique_ptr<IOBuf> chain(IOBuf::create(8));
+    chain->prependChain(std::move(buf));
+
+    Cursor curs(chain.get());
+    EXPECT_THROW(curs.readTerminatedString('\0', 3),
+                 std::length_error);
+  }
+
   // Test reading a two fixed-length strings from a single buffer with an extra
   // uint8_t at the end
   {