From c9187480d12ed094d4cea06e4da40b54ce2c8aaf Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Tue, 26 May 2020 10:17:17 +0100
Subject: [PATCH] Fix short reads in ContentReader::buffer()

The actual length of data read by `dap::Reader::read()` was not being correctly inspected.

Fixed, added test.

Bug identified by @kuafuwang.

Fixes #27
---
 src/content_stream.cpp      |  9 +++++----
 src/content_stream_test.cpp | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/src/content_stream.cpp b/src/content_stream.cpp
index d528669..e7c6628 100644
--- a/src/content_stream.cpp
+++ b/src/content_stream.cpp
@@ -136,14 +136,15 @@ bool ContentReader::buffer(size_t bytes) {
   bytes -= buf.size();
   while (bytes > 0) {
     uint8_t chunk[256];
-    auto c = std::min(sizeof(chunk), bytes);
-    if (reader->read(chunk, c) <= 0) {
+    auto numWant = std::min(sizeof(chunk), bytes);
+    auto numGot = reader->read(chunk, numWant);
+    if (numGot == 0) {
       return false;
     }
-    for (size_t i = 0; i < c; i++) {
+    for (size_t i = 0; i < numGot; i++) {
       buf.push_back(chunk[i]);
     }
-    bytes -= c;
+    bytes -= numGot;
   }
   return true;
 }
diff --git a/src/content_stream_test.cpp b/src/content_stream_test.cpp
index df29b2e..3e5d732 100644
--- a/src/content_stream_test.cpp
+++ b/src/content_stream_test.cpp
@@ -19,6 +19,27 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include <memory>
+
+namespace {
+
+// SingleByteReader wraps a dap::Reader to only provide a single byte for each
+// read() call, regardless of the number of bytes actually requested.
+class SingleByteReader : public virtual dap::Reader {
+ public:
+  SingleByteReader(std::unique_ptr<dap::Reader>&& inner)
+      : inner(std::move(inner)) {}
+
+  bool isOpen() override { return inner->isOpen(); }
+  void close() override { return inner->close(); }
+  size_t read(void* buffer, size_t) override { return inner->read(buffer, 1); };
+
+ private:
+  std::unique_ptr<dap::Reader> inner;
+};
+
+}  // namespace
+
 TEST(ContentStreamTest, Write) {
   auto sb = dap::StringBuffer::create();
   auto ptr = sb.get();
@@ -45,3 +66,18 @@ TEST(ContentStreamTest, Read) {
   ASSERT_EQ(cs.read(), "Content payload number three");
   ASSERT_EQ(cs.read(), "");
 }
+
+TEST(ContentStreamTest, ShortRead) {
+  auto sb = dap::StringBuffer::create();
+  sb->write("Content-Length: 26\r\n\r\nContent payload number one");
+  sb->write("some unrecognised garbage");
+  sb->write("Content-Length: 26\r\n\r\nContent payload number two");
+  sb->write("some more unrecognised garbage");
+  sb->write("Content-Length: 28\r\n\r\nContent payload number three");
+  dap::ContentReader cs(
+      std::unique_ptr<SingleByteReader>(new SingleByteReader(std::move(sb))));
+  ASSERT_EQ(cs.read(), "Content payload number one");
+  ASSERT_EQ(cs.read(), "Content payload number two");
+  ASSERT_EQ(cs.read(), "Content payload number three");
+  ASSERT_EQ(cs.read(), "");
+}