Fix potential buffer over-read errors for un-terminated JSON strings and comments.

base/json/json_parser_unittest.cc

Index: base/json/json_parser_unittest.cc
diff --git a/base/json/json_parser_unittest.cc b/base/json/json_parser_unittest.cc
index e3f635b76f1361f3c1bb8db00ffa5d20b0410d4a..dfbe77de82b5c404c9d021b88ac27fac1980cdc9 100644
--- a/base/json/json_parser_unittest.cc
+++ b/base/json/json_parser_unittest.cc
@@ -28,6 +28,17 @@ class JSONParserTest : public testing::Test {
     return parser;
   }
 
+  // MSan will do a better job detecting over-read errors if the input is
+  // not nul-terminated on the heap. This will copy |input| to a new buffer
+  // owned by |owner|, returning a StringPiece to |owner|.
+  StringPiece MakeNotNullTerminatedInput(const char* input,
+                                         std::unique_ptr<char[]>* owner) {
+    size_t str_len = strlen(input);
+    owner->reset(new char[str_len]);
+    memcpy(owner->get(), input, str_len);
+    return StringPiece(owner->get(), str_len);
+  }
+
   void TestLastThree(JSONParser* parser) {
     EXPECT_EQ(',', *parser->NextChar());
     EXPECT_EQ('|', *parser->NextChar());
@@ -367,14 +378,11 @@ TEST_F(JSONParserTest, ParseNumberErrors) {
     auto test_case = kCases[i];
     SCOPED_TRACE(StringPrintf("case %u: \"%s\"", i, test_case.input));
 
-    // MSan will do a better job detecting over-read errors if the input is
-    // not nul-terminated on the heap.
-    size_t str_len = strlen(test_case.input);
-    auto non_nul_termianted = MakeUnique<char[]>(str_len);
-    memcpy(non_nul_termianted.get(), test_case.input, str_len);
+    std::unique_ptr<char[]> input_owner;
+    StringPiece input =
+        MakeNotNullTerminatedInput(test_case.input, &input_owner);
 
-    StringPiece string_piece(non_nul_termianted.get(), str_len);
-    std::unique_ptr<Value> result = JSONReader::Read(string_piece);
+    std::unique_ptr<Value> result = JSONReader::Read(input);
     if (test_case.parse_success) {
       EXPECT_TRUE(result);
     } else {
@@ -390,5 +398,34 @@ TEST_F(JSONParserTest, ParseNumberErrors) {
   }
 }
 
+TEST_F(JSONParserTest, UnterminatedInputs) {
+  const char* kCases[] = {
+      // clang-format off
+      "/",
+      "//",
+      "/*",
+      "\"xxxxxx",
+      "\"",
+      "{   ",
+      "[\t",
+      "tru",
+      "fals",
+      "nul",
+      "\"\\x2",
+      "\"\\u123",
+      // clang-format on
+  };
+
+  for (unsigned int i = 0; i < arraysize(kCases); ++i) {

[brettw, 2017/05/09 19:53:14]

Random optional thought: Does C++11 let you do:
  for (const char* test_case : kCases)
here? I thought begin() and end() worked for static arrays. Maybe I'm dreaming.

[Robert Sesek, 2017/05/09 21:27:36]

The |for (:)| construct does work, but I use |i| in the SCOPED_TRACE.

+    auto* test_case = kCases[i];
+    SCOPED_TRACE(StringPrintf("case %u: \"%s\"", i, test_case));
+
+    std::unique_ptr<char[]> input_owner;
+    StringPiece input = MakeNotNullTerminatedInput(test_case, &input_owner);
+
+    EXPECT_FALSE(JSONReader::Read(input));
+  }
+}
+
 }  // namespace internal
 }  // namespace base