From 1ecfc091f716e868d844429b65ef820ecda06b65 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Sun, 16 Mar 2025 15:49:39 -0700 Subject: [PATCH 1/5] Expose JSON_USE_EXCEPTION and JSON_HAS_INT64 as Bazel config flags with defaults that match the existing Bazel build. Switch //:jsoncpp from using copts ro defines for JSON_USE_EXCEPTION and JSON_HAS_INT64 so that rules that depend on it get the same config. Make src/test_lib_json/fuzz.cpp respect JSON_USE_EXCEPTION. --- BUILD.bazel | 33 +++++++++++++++++++++++++++++---- MODULE.bazel | 5 +++++ src/test_lib_json/fuzz.cpp | 8 ++++---- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 6d7ac3da9..2227fee23 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -1,7 +1,29 @@ licenses(["unencumbered"]) # Public Domain or MIT +load("@bazel_skylib//rules:common_settings.bzl", "bool_flag") + exports_files(["LICENSE"]) +bool_flag( + name = "use_exception", + build_setting_default = False, +) + +config_setting( + name = "use_exception_cfg", + flag_values = {":use_exception": "true"}, +) + +bool_flag( + name = "has_int64", + build_setting_default = True, +) + +config_setting( + name = "has_int64_cfg", + flag_values = {":has_int64": "true"}, +) + cc_library( name = "jsoncpp", srcs = [ @@ -22,10 +44,13 @@ cc_library( "include/json/version.h", "include/json/writer.h", ], - copts = [ - "-DJSON_USE_EXCEPTION=0", - "-DJSON_HAS_INT64", - ], + defines = select({ + ":use_exception_cfg": ["JSON_USE_EXCEPTION=1"], + "//conditions:default": ["JSON_USE_EXCEPTION=0"], + }) + select({ + ":has_int64_cfg": ["JSON_HAS_INT64"], + "//conditions:default": [], + }), includes = ["include"], visibility = ["//visibility:public"], deps = [":private"], diff --git a/MODULE.bazel b/MODULE.bazel index 03f192dd4..e60fa06d1 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -12,3 +12,8 @@ module( version = "1.9.7", compatibility_level = 1, ) + +bazel_dep( + name = "bazel_skylib", + version = "1.7.1", +) diff --git a/src/test_lib_json/fuzz.cpp b/src/test_lib_json/fuzz.cpp index 5b75c22e6..3679a95ec 100644 --- a/src/test_lib_json/fuzz.cpp +++ b/src/test_lib_json/fuzz.cpp @@ -11,10 +11,6 @@ #include #include -namespace Json { -class Exception; -} - extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { Json::CharReaderBuilder builder; @@ -45,10 +41,14 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { Json::Value root; const auto data_str = reinterpret_cast(data); +#if JSON_USE_EXCEPTION try { +#endif // JSON_USE_EXCEPTION reader->parse(data_str, data_str + size, &root, nullptr); +#if JSON_USE_EXCEPTION } catch (Json::Exception const&) { } +#endif // JSON_USE_EXCEPTION // Whether it succeeded or not doesn't matter. return 0; } From 8751d1528ec530d00d2d7617d42581ddfb4ca771 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Sun, 16 Mar 2025 16:09:01 -0700 Subject: [PATCH 2/5] #ifdef stuff that should only be used with JSON_USE_EXCEPTION. --- src/test_lib_json/jsontest.h | 4 ++++ src/test_lib_json/main.cpp | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/test_lib_json/jsontest.h b/src/test_lib_json/jsontest.h index 69e3264b9..3652c4029 100644 --- a/src/test_lib_json/jsontest.h +++ b/src/test_lib_json/jsontest.h @@ -228,6 +228,8 @@ TestResult& checkStringEqual(TestResult& result, const Json::String& expected, JsonTest::ToJsonString(actual), __FILE__, \ __LINE__, #expected " == " #actual) +#if JSON_USE_EXCEPTION + /// \brief Asserts that a given expression throws an exception #define JSONTEST_ASSERT_THROWS(expr) \ do { \ @@ -242,6 +244,8 @@ TestResult& checkStringEqual(TestResult& result, const Json::String& expected, "expected exception thrown: " #expr); \ } while (0) +#endif // JSON_USE_EXCEPTION + /// \brief Begin a fixture test case. #define JSONTEST_FIXTURE(FixtureType, name) \ class Test##FixtureType##name : public FixtureType { \ diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index 60f149d5e..e20723498 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -1888,7 +1888,7 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, typeChecksThrowExceptions) { JSONTEST_ASSERT_THROWS(objVal.asBool()); JSONTEST_ASSERT_THROWS(arrVal.asBool()); -#endif +#endif // JSON_USE_EXCEPTION } JSONTEST_FIXTURE_LOCAL(ValueTest, offsetAccessors) { @@ -3323,6 +3323,8 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithDetailError) { } JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { +#if JSON_USE_EXCEPTION + Json::CharReaderBuilder b; Json::Value root; char const doc[] = R"({ "property" : "value" })"; @@ -3342,6 +3344,8 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { JSONTEST_ASSERT_THROWS( reader->parse(doc, doc + std::strlen(doc), &root, &errs)); } + +#endif // JSON_USE_EXCEPTION } JSONTEST_FIXTURE_LOCAL(CharReaderTest, testOperator) { @@ -3961,6 +3965,8 @@ JSONTEST_FIXTURE_LOCAL(IteratorTest, indexes) { } JSONTEST_FIXTURE_LOCAL(IteratorTest, constness) { +#if JSON_USE_EXCEPTION + Json::Value const v; JSONTEST_ASSERT_THROWS( Json::Value::iterator it(v.begin())); // Compile, but throw. @@ -3982,6 +3988,8 @@ JSONTEST_FIXTURE_LOCAL(IteratorTest, constness) { } Json::String expected = R"(" 9","10","11",)"; JSONTEST_ASSERT_STRING_EQUAL(expected, out.str()); + +#endif // JSON_USE_EXCEPTION } struct RValueTest : JsonTest::TestCase {}; From 8fcfda4f1c7726e2b20f3ed1a308441e1b6fa2ad Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Sun, 16 Mar 2025 17:41:26 -0700 Subject: [PATCH 3/5] Modify runjsontests.py to allow passing a single test case file. --- test/runjsontests.py | 61 +++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/test/runjsontests.py b/test/runjsontests.py index 49cc7a960..3099195e9 100644 --- a/test/runjsontests.py +++ b/test/runjsontests.py @@ -66,38 +66,51 @@ class FailError(Exception): def __init__(self, msg): super(Exception, self).__init__(msg) -def runAllTests(jsontest_executable_path, input_dir = None, +def runAllTests(jsontest_executable_path, input_path = None, use_valgrind=False, with_json_checker=False, writerClass='StyledWriter'): - if not input_dir: - input_dir = os.path.join(os.getcwd(), 'data') - tests = glob(os.path.join(input_dir, '*.json')) - if with_json_checker: - all_tests = glob(os.path.join(input_dir, '../jsonchecker', '*.json')) - # These tests fail with strict json support, but pass with JsonCPP's - # extra leniency features. When adding a new exclusion to this list, - # remember to add the test's number and reasoning here: - known = ["fail{}.json".format(n) for n in [ - 4, 9, # fail because we allow trailing commas - 7, # fails because we allow commas after close - 8, # fails because we allow extra close - 10, # fails because we allow extra values after close - 13, # fails because we allow leading zeroes in numbers - 18, # fails because we allow deeply nested values - 25, # fails because we allow tab characters in strings - 27, # fails because we allow string line breaks - ]] - test_jsonchecker = [ test for test in all_tests - if os.path.basename(test) not in known] + if not input_path: + input_path = os.path.join(os.getcwd(), 'data') + if os.path.isdir(input_path): + tests = [ + os.path.normpath(os.path.abspath(test)) + for test in glob(os.path.join(input_path, '*.json')) + ] + + if with_json_checker: + tests += [ + os.path.normpath(os.path.abspath(test)) + for test in glob(os.path.join(input_path, '../jsonchecker', '*.json')) + ] else: - test_jsonchecker = [] + tests = [input_path] + + # These tests fail with strict json support, but pass with JsonCPP's + # extra leniency features. When adding a new exclusion to this list, + # remember to add the test's number and reasoning here: + known = ["fail{}.json".format(n) for n in [ + 4, 9, # fail because we allow trailing commas + 7, # fails because we allow commas after close + 8, # fails because we allow extra close + 10, # fails because we allow extra values after close + 13, # fails because we allow leading zeroes in numbers + 18, # fails because we allow deeply nested values + 25, # fails because we allow tab characters in strings + 27, # fails because we allow string line breaks + ]] + + tests = [ + test for test in tests + if os.path.basename(test) not in known and + os.path.basename(os.path.dirname(test)) == "jsonchecker" + ] failed_tests = [] valgrind_path = use_valgrind and VALGRIND_CMD or '' - for input_path in tests + test_jsonchecker: + for input_path in tests: expect_failure = os.path.basename(input_path).startswith('fail') - is_json_checker_test = input_path in test_jsonchecker + is_json_checker_test = os.path.basename(os.path.dirname(input_path)) == "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=' ') From 0383d14b761d764220340ddadf098e36b593e255 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Sun, 16 Mar 2025 17:46:38 -0700 Subject: [PATCH 4/5] Add tests to the Bazel builds. --- src/jsontestrunner/BUILD.bazel | 6 ++++++ src/test_lib_json/BUILD.bazel | 11 +++++++++++ test/BUILD.bazel | 22 ++++++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 src/jsontestrunner/BUILD.bazel create mode 100644 src/test_lib_json/BUILD.bazel create mode 100644 test/BUILD.bazel diff --git a/src/jsontestrunner/BUILD.bazel b/src/jsontestrunner/BUILD.bazel new file mode 100644 index 000000000..543bc5d88 --- /dev/null +++ b/src/jsontestrunner/BUILD.bazel @@ -0,0 +1,6 @@ +cc_binary( + name = "jsontestrunner", + srcs = ["main.cpp"], + deps = ["//:jsoncpp"], + visibility = ["//test:__pkg__"], +) diff --git a/src/test_lib_json/BUILD.bazel b/src/test_lib_json/BUILD.bazel new file mode 100644 index 000000000..7e83f5219 --- /dev/null +++ b/src/test_lib_json/BUILD.bazel @@ -0,0 +1,11 @@ +cc_test( + name = "jsoncpp_test", + srcs = [ + "jsontest.cpp", + "jsontest.h", + "main.cpp", + "fuzz.h", + "fuzz.cpp", + ], + deps = ["//:jsoncpp"], +) diff --git a/test/BUILD.bazel b/test/BUILD.bazel new file mode 100644 index 000000000..1e83c2d11 --- /dev/null +++ b/test/BUILD.bazel @@ -0,0 +1,22 @@ +filegroup( + name = "expected", + srcs = glob(["data/**", "jsonchecker/**"], exclude=["**/*.json"]), +) + +######## + +[py_test( + name = "runjson_%s_test" % "_".join(f.split("/")), + srcs = ["runjsontests.py"], + main = "runjsontests.py", + args = [ + "--with-json-checker", + "$(location //src/jsontestrunner:jsontestrunner)", + "$(location :%s)" % f, + ], + data = [ + "//src/jsontestrunner:jsontestrunner", + ":expected", + ":%s" % f, + ], +) for f in glob(["**/*.json"])] From bb4ac3c6c479b83e2beac305e687c99b2723a7a6 Mon Sep 17 00:00:00 2001 From: bcsgh <33939446+bcsgh@users.noreply.github.com> Date: Tue, 18 Mar 2025 18:11:14 -0700 Subject: [PATCH 5/5] Reverse the polarity to fix a bug. --- test/BUILD.bazel | 2 -- test/runjsontests.py | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/test/BUILD.bazel b/test/BUILD.bazel index 1e83c2d11..269cd8646 100644 --- a/test/BUILD.bazel +++ b/test/BUILD.bazel @@ -3,8 +3,6 @@ filegroup( srcs = glob(["data/**", "jsonchecker/**"], exclude=["**/*.json"]), ) -######## - [py_test( name = "runjson_%s_test" % "_".join(f.split("/")), srcs = ["runjsontests.py"], diff --git a/test/runjsontests.py b/test/runjsontests.py index 3099195e9..14275ec22 100644 --- a/test/runjsontests.py +++ b/test/runjsontests.py @@ -102,8 +102,8 @@ def runAllTests(jsontest_executable_path, input_path = None, tests = [ test for test in tests - if os.path.basename(test) not in known and - os.path.basename(os.path.dirname(test)) == "jsonchecker" + if os.path.basename(test) not in known or + os.path.basename(os.path.dirname(test)) != "jsonchecker" ] failed_tests = []