From 98dbbadf51f51369116bfb13215fc15ef1b613e5 Mon Sep 17 00:00:00 2001 From: Billy Donahue Date: Fri, 10 Nov 2017 15:02:19 -0500 Subject: SERVER-30244 escape NUL in Terminated status msgs. --- src/mongo/base/data_type_terminated.cpp | 9 +- src/mongo/base/data_type_terminated_test.cpp | 222 ++++++++++++++++++++++++--- 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 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 +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 +struct DataType::Handler> { + using handledType = Dummy; + 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(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 -- cgit v1.2.1