SERVER-30244 escape NUL in Terminated status msgs.

2 files changed, 204 insertions, 27 deletions

diff --git a/src/mongo/base/data_type_terminated.cpp b/src/mongo/base/data_type_terminated.cpp
index 6171fd8b10d..18cf48d2ce3 100644
--- a/src/mongo/base/data_type_terminated.cpp
+++ b/src/mongo/base/data_type_terminated.cpp
@@ -28,6 +28,7 @@
 #include "mongo/base/data_type_terminated.h"
 
 #include "mongo/util/mongoutils/str.h"
+#include "mongo/util/stringutils.h"
 
 namespace mongo {
 
@@ -35,7 +36,7 @@ Status TerminatedHelper::makeLoadNoTerminalStatus(char c,
                                                   size_t length,
                                                   std::ptrdiff_t debug_offset) {
     str::stream ss;
-    ss << "couldn't locate terminal char (" << c << ") in buffer[" << length
+    ss << "couldn't locate terminal char (" << escape(StringData(&c, 1)) << ") in buffer[" << length
        << "] at offset: " << debug_offset;
     return Status(ErrorCodes::Overflow, ss);
 }
@@ -45,15 +46,15 @@ Status TerminatedHelper::makeLoadShortReadStatus(char c,
                                                  size_t length,
                                                  std::ptrdiff_t debug_offset) {
     str::stream ss;
-    ss << "only read (" << read << ") bytes. (" << length << ") bytes to terminal char (" << c
-       << ") at offset: " << debug_offset;
+    ss << "only read (" << read << ") bytes. (" << length << ") bytes to terminal char ("
+       << escape(StringData(&c, 1)) << ") at offset: " << debug_offset;
 
     return Status(ErrorCodes::Overflow, ss);
 }
 
 Status TerminatedHelper::makeStoreStatus(char c, size_t length, std::ptrdiff_t debug_offset) {
     str::stream ss;
-    ss << "couldn't write terminal char (" << c << ") in buffer[" << length
+    ss << "couldn't write terminal char (" << escape(StringData(&c, 1)) << ") in buffer[" << length
        << "] at offset: " << debug_offset;
     return Status(ErrorCodes::Overflow, ss);
 }
diff --git a/src/mongo/base/data_type_terminated_test.cpp b/src/mongo/base/data_type_terminated_test.cpp
index e0192b53dd5..d3cc3d64d18 100644
--- a/src/mongo/base/data_type_terminated_test.cpp
+++ b/src/mongo/base/data_type_terminated_test.cpp
@@ -31,42 +31,218 @@
 #include "mongo/base/data_range.h"
 #include "mongo/base/data_range_cursor.h"
 #include "mongo/unittest/unittest.h"
+#include "mongo/util/stringutils.h"
+#include <string>
 
 namespace mongo {
+namespace {
 
-TEST(DataTypeTerminated, Basic) {
-    char buf[100];
-    char a[] = "a";
-    char b[] = "bb";
-    char c[] = "ccc";
+// For testing purposes, a type that has a fixed load and store size, and some
+// arbitrary serialization format of 'd' repeated N times.
+template <size_t N>
+struct Dummy {
+    static constexpr size_t extent = N;
+};
+}  // namespace
+// Pop the anonymous namespace to specialize mongo::DataType::Handler.
+// Template specialization is a drag.
 
-    {
-        DataRangeCursor drc(buf, buf + sizeof(buf));
-        ConstDataRange cdr_a(a, a + sizeof(a) + -1);
-        ConstDataRange cdr_b(b, b + sizeof(b) + -1);
-        ConstDataRange cdr_c(c, c + sizeof(c) + -1);
+template <size_t N>
+struct DataType::Handler<Dummy<N>> {
+    using handledType = Dummy<N>;
+    static constexpr size_t extent = handledType::extent;
 
-        ASSERT_OK(drc.writeAndAdvance(Terminated<'\0', ConstDataRange>(cdr_a)));
-        ASSERT_OK(drc.writeAndAdvance(Terminated<'\0', ConstDataRange>(cdr_b)));
-        ASSERT_OK(drc.writeAndAdvance(Terminated<'\0', ConstDataRange>(cdr_c)));
+    static Status load(handledType* sdata,
+                       const char* ptr,
+                       size_t length,
+                       size_t* advanced,
+                       std::ptrdiff_t debug_offset) {
+        if (length < extent) {
+            return Status(ErrorCodes::Overflow, "too short for Dummy");
+        }
+        for (size_t i = 0; i < extent; ++i) {
+            if (*ptr++ != 'd') {
+                return Status(ErrorCodes::Overflow, "load of invalid Dummy object");
+            }
+        }
+        *advanced = extent;
+        return Status::OK();
+    }
 
-        ASSERT_EQUALS(1 + 2 + 3 + 3, drc.data() - buf);
+    static Status store(const handledType& sdata,
+                        char* ptr,
+                        size_t length,
+                        size_t* advanced,
+                        std::ptrdiff_t debug_offset) {
+        if (length < extent) {
+            return Status(ErrorCodes::Overflow, "insufficient space for Dummy");
+        }
+        for (size_t i = 0; i < extent; ++i) {
+            *ptr++ = 'd';
+        }
+        *advanced = extent;
+        return Status::OK();
     }
 
-    {
-        ConstDataRangeCursor cdrc(buf, buf + sizeof(buf));
+    static handledType defaultConstruct() {
+        return {};
+    }
+};
 
-        Terminated<'\0', ConstDataRange> tcdr;
+// Re-push the anonymous namespace.
+namespace {
 
-        ASSERT_OK(cdrc.readAndAdvance(&tcdr));
-        ASSERT_EQUALS(std::string(a), tcdr.value.data());
+/**
+ * Tests specifically for Terminated, unrelated to the DataRange
+ * or DataRangeCursor classes that call it.
+ */
 
-        ASSERT_OK(cdrc.readAndAdvance(&tcdr));
-        ASSERT_EQUALS(std::string(b), tcdr.value.data());
+TEST(DataTypeTerminated, StringDataNormalStore) {
+    const StringData writes[] = {StringData("a"), StringData("bb"), StringData("ccc")};
+    std::string buf(100, '\xff');
+    char* const bufBegin = &*buf.begin();
+    char* ptr = bufBegin;
+    size_t avail = buf.size();
+    std::string expected;
+    for (const auto& w : writes) {
+        size_t adv;
+        ASSERT_OK(
+            DataType::store(Terminated<'\0', StringData>(w), ptr, avail, &adv, ptr - bufBegin));
+        ASSERT_EQ(adv, w.size() + 1);
+        ptr += adv;
+        avail -= adv;
+        expected += w.toString();
+        expected += '\0';
+    }
+    ASSERT_EQUALS(expected, buf.substr(0, buf.size() - avail));
+}
 
-        ASSERT_OK(cdrc.readAndAdvance(&tcdr));
-        ASSERT_EQUALS(std::string(c), tcdr.value.data());
+TEST(DataTypeTerminated, StringDataNormalLoad) {
+    const StringData writes[] = {StringData("a"), StringData("bb"), StringData("ccc")};
+    std::string buf;
+    for (const auto& w : writes) {
+        buf += w.toString();
+        buf += '\0';
+    }
+    const char* const bufBegin = &*buf.begin();
+    const char* ptr = bufBegin;
+    size_t avail = buf.size();
+
+    for (const auto& w : writes) {
+        size_t adv;
+        auto term = Terminated<'\0', StringData>{};
+        ASSERT_OK(DataType::load(&term, ptr, avail, &adv, ptr - bufBegin));
+        ASSERT_EQ(adv, term.value.size() + 1);
+        ptr += adv;
+        avail -= adv;
+        ASSERT_EQUALS(term.value, w);
+    }
+}
+
+TEST(DataTypeTerminated, LoadStatusOkPropagation) {
+    // Test that the nested type's .load complaints are surfaced.
+    const char buf[] = {'d', 'd', 'd', '\0'};
+    size_t advanced = 123;
+    auto x = Terminated<'\0', Dummy<3>>();
+    Status s = DataType::load(&x, buf, sizeof(buf), &advanced, 0);
+    ASSERT_OK(s);
+    ASSERT_EQUALS(advanced, 4u);  // OK must overwrite advanced
+}
+
+TEST(DataTypeTerminated, StoreStatusOkAdvanced) {
+    // Test that an OK .store sets proper 'advanced'.
+    char buf[4] = {};
+    size_t advanced = 123;  // should be overwritten
+    Status s = DataType::store(Terminated<'\0', Dummy<3>>(), buf, sizeof(buf), &advanced, 0);
+    ASSERT_OK(s);
+    ASSERT_EQ(StringData(buf, 4), StringData(std::string{'d', 'd', 'd', '\0'}));
+    ASSERT_EQUALS(advanced, 4u);  // OK must overwrite advanced
+}
+
+TEST(DataTypeTerminated, ErrorUnterminatedRead) {
+    const char buf[] = {'h', 'e', 'l', 'l', 'o'};
+    size_t advanced = 123;
+    auto x = Terminated<'\0', StringData>();
+    Status s = DataType::load(&x, buf, sizeof(buf), &advanced, 0);
+    ASSERT_EQ(s.codeString(), "Overflow");
+    ASSERT_STRING_CONTAINS(s.reason(), "couldn't locate");
+    ASSERT_STRING_CONTAINS(s.reason(), "terminal char (\\u0000)");
+    ASSERT_EQUALS(advanced, 123u);  // fails must not overwrite advanced
+}
+
+TEST(DataTypeTerminated, LoadStatusPropagation) {
+    // Test that the nested type's .load complaints are surfaced.
+    const char buf[] = {'d', 'd', '\0'};
+    size_t advanced = 123;
+    auto x = Terminated<'\0', Dummy<3>>();
+    Status s = DataType::load(&x, buf, sizeof(buf), &advanced, 0);
+    ASSERT_EQ(s.codeString(), "Overflow");
+    ASSERT_STRING_CONTAINS(s.reason(), "too short for Dummy");
+    // ASSERT_STRING_CONTAINS(s.reason(), "terminal char (\\u0000)");
+    ASSERT_EQUALS(advanced, 123u);  // fails must not overwrite advanced
+}
+
+TEST(DataTypeTerminated, StoreStatusPropagation) {
+    // Test that the nested type's .store complaints are surfaced.
+    char in[2];  // Not big enough to hold a Dummy<3>.
+    size_t advanced = 123;
+    Status s = DataType::store(Terminated<'\0', Dummy<3>>(), in, sizeof(in), &advanced, 0);
+    ASSERT_EQ(s.codeString(), "Overflow");
+    ASSERT_STRING_CONTAINS(s.reason(), "insufficient space for Dummy");
+    ASSERT_EQUALS(advanced, 123u);  // fails must not overwrite advanced
+}
+
+TEST(DataTypeTerminated, ErrorShortRead) {
+    // The span before the '\0' is passed to Dummy<3>'s load.
+    // This consumes only the first 3 bytes, so Terminated complains
+    // about the unconsumed 'X'.
+    const char buf[] = {'d', 'd', 'd', 'X', '\0'};
+    size_t advanced = 123;
+    auto x = Terminated<'\0', Dummy<3>>();
+    Status s = DataType::load(&x, buf, sizeof(buf), &advanced, 0);
+    ASSERT_EQ(s.codeString(), "Overflow");
+    ASSERT_STRING_CONTAINS(s.reason(), "only read");
+    ASSERT_STRING_CONTAINS(s.reason(), "terminal char (\\u0000)");
+    ASSERT_EQUALS(advanced, 123u);  // fails must not overwrite advanced
+}
+
+TEST(DataTypeTerminated, ErrorShortWrite) {
+    char in[3] = {};
+    auto x = Terminated<'\0', Dummy<3>>();
+    size_t advanced = 123;
+    Status s = DataType::store(x, in, sizeof(in), &advanced, 0);
+    ASSERT_EQ(s.codeString(), "Overflow");
+    ASSERT_STRING_CONTAINS(s.reason(), "couldn't write");
+    ASSERT_STRING_CONTAINS(s.reason(), "terminal char (\\u0000)");
+    ASSERT_EQUALS(advanced, 123u);  // fails must not overwrite advanced
+}
+
+TEST(DataTypeTerminated, ThroughDataRangeCursor) {
+    char buf[100];
+    const std::string parts[] = {"a", "bb", "ccc"};
+    std::string serialized;
+    for (const std::string& s : parts) {
+        serialized += s + '\0';
+    }
+    {
+        auto buf_writer = DataRangeCursor(buf, buf + sizeof(buf));
+        for (const std::string& s : parts) {
+            Terminated<'\0', ConstDataRange> tcdr(ConstDataRange(s.data(), s.data() + s.size()));
+            ASSERT_OK(buf_writer.writeAndAdvance(tcdr));
+        }
+        const auto written = std::string(static_cast<const char*>(buf), buf_writer.data());
+        ASSERT_EQUALS(written, serialized);
+    }
+    {
+        auto buf_source = ConstDataRangeCursor(buf, buf + sizeof(buf));
+        for (const std::string& s : parts) {
+            Terminated<'\0', ConstDataRange> tcdr;
+            ASSERT_OK(buf_source.readAndAdvance(&tcdr));
+            std::string read(tcdr.value.data(), tcdr.value.data() + tcdr.value.length());
+            ASSERT_EQUALS(s, read);
+        }
     }
 }
 
+}  // namespace
 }  // namespace mongo