From eb21dc06978466c54379fee774ee9dd066159560 Mon Sep 17 00:00:00 2001
From: Greg Falcon <gfalcon@google.com>
Date: Wed, 19 Jul 2023 12:19:12 -0400
Subject: [PATCH] Fix a parser bug where tokens are misidentified as commas.

In the old and new readers, when parsing an object, a comment
followed by any non-`}` token is treated as a comma.

The new unit test required changing the runjsontests.py
flag regime so that failure tests could be run with default settings.
---
 src/jsontestrunner/main.cpp        |  7 +++++--
 src/lib_json/json_reader.cpp       | 24 ++++++++++++------------
 test/data/fail_test_object_02.json |  1 +
 test/runjsontests.py               |  9 ++++++---
 4 files changed, 24 insertions(+), 17 deletions(-)
 create mode 100644 test/data/fail_test_object_02.json

diff --git a/src/jsontestrunner/main.cpp b/src/jsontestrunner/main.cpp
index df717ff..ab6a800 100644
--- a/src/jsontestrunner/main.cpp
+++ b/src/jsontestrunner/main.cpp
@@ -240,11 +240,14 @@ static int parseCommandLine(int argc, const char* argv[], Options* opts) {
     return printUsage(argv);
   }
   int index = 1;
-  if (Json::String(argv[index]) == "--json-checker") {
-    opts->features = Json::Features::strictMode();
+  if (Json::String(argv[index]) == "--parse-only") {
     opts->parseOnly = true;
     ++index;
   }
+  if (Json::String(argv[index]) == "--strict") {
+    opts->features = Json::Features::strictMode();
+    ++index;
+  }
   if (Json::String(argv[index]) == "--json-config") {
     printConfig();
     return 3;
diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp
index 1ac5e81..6ac7b20 100644
--- a/src/lib_json/json_reader.cpp
+++ b/src/lib_json/json_reader.cpp
@@ -480,15 +480,15 @@ bool Reader::readObject(Token& token) {
       return recoverFromError(tokenObjectEnd);
 
     Token comma;
-    if (!readToken(comma) ||
-        (comma.type_ != tokenObjectEnd && comma.type_ != tokenArraySeparator &&
-         comma.type_ != tokenComment)) {
+    bool finalizeTokenOk = readToken(comma);
+    while (comma.type_ == tokenComment && finalizeTokenOk) {
+      finalizeTokenOk = readToken(comma);
+    }
+    if (!finalizeTokenOk ||
+        (comma.type_ != tokenObjectEnd && comma.type_ != tokenArraySeparator)) {
       return addErrorAndRecover("Missing ',' or '}' in object declaration",
                                 comma, tokenObjectEnd);
     }
-    bool finalizeTokenOk = true;
-    while (comma.type_ == tokenComment && finalizeTokenOk)
-      finalizeTokenOk = readToken(comma);
     if (comma.type_ == tokenObjectEnd)
       return true;
   }
@@ -1491,15 +1491,15 @@ bool OurReader::readObject(Token& token) {
       return recoverFromError(tokenObjectEnd);
 
     Token comma;
-    if (!readToken(comma) ||
-        (comma.type_ != tokenObjectEnd && comma.type_ != tokenArraySeparator &&
-         comma.type_ != tokenComment)) {
+    bool finalizeTokenOk = readToken(comma);
+    while (comma.type_ == tokenComment && finalizeTokenOk) {
+      finalizeTokenOk = readToken(comma);
+    }
+    if (!finalizeTokenOk ||
+        (comma.type_ != tokenObjectEnd && comma.type_ != tokenArraySeparator)) {
       return addErrorAndRecover("Missing ',' or '}' in object declaration",
                                 comma, tokenObjectEnd);
     }
-    bool finalizeTokenOk = true;
-    while (comma.type_ == tokenComment && finalizeTokenOk)
-      finalizeTokenOk = readToken(comma);
     if (comma.type_ == tokenObjectEnd)
       return true;
   }
diff --git a/test/data/fail_test_object_02.json b/test/data/fail_test_object_02.json
new file mode 100644
index 0000000..afe62c5
--- /dev/null
+++ b/test/data/fail_test_object_02.json
@@ -0,0 +1 @@
+{"one": 1 /* } */ { "two" : 2 }
diff --git a/test/runjsontests.py b/test/runjsontests.py
index 5496e2c..49cc7a9 100644
--- a/test/runjsontests.py
+++ b/test/runjsontests.py
@@ -97,14 +97,17 @@ def runAllTests(jsontest_executable_path, input_dir = None,
     valgrind_path = use_valgrind and VALGRIND_CMD or ''
     for input_path in tests + test_jsonchecker:
         expect_failure = os.path.basename(input_path).startswith('fail')
-        is_json_checker_test = (input_path in test_jsonchecker) or expect_failure
+        is_json_checker_test = input_path in test_jsonchecker
+        is_parse_only = is_json_checker_test or expect_failure
+        is_strict_test = ('_strict_' in os.path.basename(input_path)) or is_json_checker_test
         print('TESTING:', input_path, end=' ')
-        options = is_json_checker_test and '--json-checker' or ''
+        options = is_parse_only and '--parse-only' or ''
+        options += is_strict_test and ' --strict' or ''
         options += ' --json-writer %s'%writerClass
         cmd = '%s%s %s "%s"' % (            valgrind_path, jsontest_executable_path, options,
             input_path)
         status, process_output = getStatusOutput(cmd)
-        if is_json_checker_test:
+        if is_parse_only:
             if expect_failure:
                 if not status:
                     print('FAILED')