Skip to content

Commit 78b1e69

Browse files
committed
fix: allow a comment between a trailing comma and ']' (#1500)
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.
1 parent 5f1f240 commit 78b1e69

2 files changed

Lines changed: 56 additions & 1 deletion

File tree

src/lib_json/json_reader.cpp

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,7 @@ class OurReader {
920920
bool readToken(Token& token);
921921
bool readTokenSkippingComments(Token& token);
922922
void skipSpaces();
923+
void skipCommentTokens();
923924
void skipBom(bool skipBom);
924925
bool match(const Char* pattern, int patternLength);
925926
bool readComment();
@@ -1263,6 +1264,24 @@ void OurReader::skipSpaces() {
12631264
}
12641265
}
12651266

1267+
// Skip whitespace and any comments, leaving current_ at the next significant
1268+
// character. Consumed comments are recorded (commentsBefore_) so the next value
1269+
// still receives them; if none follows they are simply not attached. This lets
1270+
// callers peek for a delimiter that is preceded by comments (e.g. a ']' after a
1271+
// trailing comma -- see readArray and issue #1500).
1272+
void OurReader::skipCommentTokens() {
1273+
skipSpaces();
1274+
if (!features_.allowComments_)
1275+
return;
1276+
while (current_ != end_ && *current_ == '/' && (current_ + 1) != end_ &&
1277+
(current_[1] == '/' || current_[1] == '*')) {
1278+
Token comment;
1279+
if (!readToken(comment))
1280+
return;
1281+
skipSpaces();
1282+
}
1283+
}
1284+
12661285
void OurReader::skipBom(bool skipBom) {
12671286
// The default behavior is to skip BOM.
12681287
if (skipBom) {
@@ -1495,7 +1514,10 @@ bool OurReader::readArray(Token& token) {
14951514
currentValue().setOffsetStart(token.start_ - begin_);
14961515
int index = 0;
14971516
for (;;) {
1498-
skipSpaces();
1517+
// Skip comments too, so a ']' that follows a trailing comma (or comments in
1518+
// an otherwise empty array) is recognized rather than mistaken for the
1519+
// start of another value. See issue #1500.
1520+
skipCommentTokens();
14991521
if (current_ != end_ && *current_ == ']' &&
15001522
(index == 0 ||
15011523
(features_.allowTrailingCommas_ &&

src/test_lib_json/main.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3313,6 +3313,39 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) {
33133313
}
33143314
}
33153315

3316+
JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseTrailingCommaWithComment) {
3317+
// Regression test for #1500: trailing commas and comments are both allowed by
3318+
// default, so they must compose -- a comment between a trailing comma and the
3319+
// closing ']' must not turn a valid document into a parse error. (Objects
3320+
// already handled this; arrays did not.)
3321+
Json::CharReaderBuilder b;
3322+
CharReaderPtr reader(b.newCharReader());
3323+
Json::Value root;
3324+
Json::String errs;
3325+
3326+
for (const char* doc : {
3327+
"[1,2,\n// trailing\n]", // line comment after trailing comma
3328+
"[1,2,/* trailing */]", // block comment after trailing comma
3329+
"[{},\n// trailing\n]", // trailing comma after a nested value
3330+
"[\n// only a comment\n]", // empty array containing a comment
3331+
"{\"a\":1,\n// trailing\n}", // object form (guard the existing case)
3332+
}) {
3333+
bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs);
3334+
JSONTEST_ASSERT(ok);
3335+
JSONTEST_ASSERT(errs.empty());
3336+
}
3337+
3338+
// A comment before a real (non-closing) element is still attached to it.
3339+
{
3340+
char const doc[] = "[1,\n// before two\n2]";
3341+
bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs);
3342+
JSONTEST_ASSERT(ok);
3343+
JSONTEST_ASSERT_EQUAL(2u, root.size());
3344+
JSONTEST_ASSERT_EQUAL(2, root[1]);
3345+
JSONTEST_ASSERT(root[1].hasComment(Json::commentBefore));
3346+
}
3347+
}
3348+
33163349
JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseCommentsAfterValueScansLinearly) {
33173350
// A value, then a comment whose only newline is at its end, then many
33183351
// trailing comments. Comment handling should scan the value->comment gap a

0 commit comments

Comments
 (0)