fix: allow a comment between a trailing comma and ']' (#1500) (#1696)

Trailing commas and comments are both allowed by default, but they did
not compose inside arrays: readArray detected a trailing-comma ']' with a
raw `*current_ == ']'` peek that skipped only whitespace, not comments.
So `[1, 2, /* c */]` left current_ at the comment, the peek failed, and
the parser tried to read another value -- which hit ']' and reported
"value, object or array expected". readObject already handled this via
readTokenSkippingComments.

Add skipCommentTokens() (skip whitespace and comments, leaving current_
at the next significant character) and use it in readArray before the
']' check. Consumed comments stay in commentsBefore_, so a comment before
a real element is still attached to it; if the array ends, they are
simply not attached -- matching object behavior.

Adds CharReaderTest/parseTrailingCommaWithComment covering line/block
comments after a trailing comma, an empty array containing only a
comment, the object form, and that a comment before a real element is
still attached.

Author: baylesj

src/lib_json/json_reader.cpp

@@ -926,6 +926,7 @@ class OurReader {
   bool readToken(Token& token);
   bool readTokenSkippingComments(Token& token);
   void skipSpaces();
+  void skipCommentTokens();
   void skipBom(bool skipBom);
   bool match(const Char* pattern, int patternLength);
   bool readComment();
@@ -1269,6 +1270,24 @@ void OurReader::skipSpaces() {
   }
 }
 
+// Skip whitespace and any comments, leaving current_ at the next significant
+// character. Consumed comments are recorded (commentsBefore_) so the next value
+// still receives them; if none follows they are simply not attached. This lets
+// callers peek for a delimiter that is preceded by comments (e.g. a ']' after a
+// trailing comma -- see readArray and issue #1500).
+void OurReader::skipCommentTokens() {
+  skipSpaces();
+  if (!features_.allowComments_)
+    return;
+  while (current_ != end_ && *current_ == '/' && (current_ + 1) != end_ &&
+         (current_[1] == '/' || current_[1] == '*')) {
+    Token comment;
+    if (!readToken(comment))
+      return;
+    skipSpaces();
+  }
+}
+
 void OurReader::skipBom(bool skipBom) {
   // The default behavior is to skip BOM.
   if (skipBom) {
@@ -1501,7 +1520,10 @@ bool OurReader::readArray(Token& token) {
   currentValue().setOffsetStart(token.start_ - begin_);
   int index = 0;
   for (;;) {
-    skipSpaces();
+    // Skip comments too, so a ']' that follows a trailing comma (or comments in
+    // an otherwise empty array) is recognized rather than mistaken for the
+    // start of another value. See issue #1500.
+    skipCommentTokens();
     if (current_ != end_ && *current_ == ']' &&
         (index == 0 ||
          (features_.allowTrailingCommas_ &&

src/test_lib_json/main.cpp

@@ -3385,6 +3385,39 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) {
   }
 }
 
+JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseTrailingCommaWithComment) {
+  // Regression test for #1500: trailing commas and comments are both allowed by
+  // default, so they must compose -- a comment between a trailing comma and the
+  // closing ']' must not turn a valid document into a parse error. (Objects
+  // already handled this; arrays did not.)
+  Json::CharReaderBuilder b;
+  CharReaderPtr reader(b.newCharReader());
+  Json::Value root;
+  Json::String errs;
+
+  for (const char* doc : {
+           "[1,2,\n// trailing\n]",     // line comment after trailing comma
+           "[1,2,/* trailing */]",      // block comment after trailing comma
+           "[{},\n// trailing\n]",      // trailing comma after a nested value
+           "[\n// only a comment\n]",   // empty array containing a comment
+           "{\"a\":1,\n// trailing\n}", // object form (guard the existing case)
+       }) {
+    bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs);
+    JSONTEST_ASSERT(ok);
+    JSONTEST_ASSERT(errs.empty());
+  }
+
+  // A comment before a real (non-closing) element is still attached to it.
+  {
+    char const doc[] = "[1,\n// before two\n2]";
+    bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs);
+    JSONTEST_ASSERT(ok);
+    JSONTEST_ASSERT_EQUAL(2u, root.size());
+    JSONTEST_ASSERT_EQUAL(2, root[1]);
+    JSONTEST_ASSERT(root[1].hasComment(Json::commentBefore));
+  }
+}
+
 JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseCommentsAfterValueScansLinearly) {
   // A value, then a comment whose only newline is at its end, then many
   // trailing comments. Comment handling should scan the value->comment gap a