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.
diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp
index ce9ca1b..164d41d 100644
--- a/src/lib_json/json_reader.cpp
+++ b/src/lib_json/json_reader.cpp
@@ -926,6 +926,7 @@
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 @@
}
}
+// 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 @@
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_ &&
diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp
index 495bbb5..90025b4 100644
--- a/src/test_lib_json/main.cpp
+++ b/src/test_lib_json/main.cpp
@@ -3385,6 +3385,39 @@
}
}
+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