From 1b792d613011265f58bf98ce6a3a9d49687bd37b Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Tue, 19 Jul 2022 21:20:18 +0300 Subject: [PATCH 01/12] Change signture of newCharReader at CharReaderBuilder to return unique_ptr to ensure correct memory management --- doc/jsoncpp.dox | 2 +- example/readFromString/readFromString.cpp | 2 +- include/json/reader.h | 4 +- src/jsontestrunner/main.cpp | 2 +- src/lib_json/json_reader.cpp | 6 +-- src/test_lib_json/fuzz.cpp | 2 +- src/test_lib_json/main.cpp | 60 +++++++++++------------ 7 files changed, 39 insertions(+), 39 deletions(-) diff --git a/doc/jsoncpp.dox b/doc/jsoncpp.dox index f340591fd..ce4c9765c 100644 --- a/doc/jsoncpp.dox +++ b/doc/jsoncpp.dox @@ -116,7 +116,7 @@ CharReaders and StreamWriters are not thread-safe, but they are re-usable. \code Json::CharReaderBuilder rbuilder; cfg >> rbuilder.settings_; -std::unique_ptr const reader(rbuilder.newCharReader()); +std::unique_ptr const reader = rbuilder.newCharReader(); reader->parse(start, stop, &value1, &errs); // ... reader->parse(start, stop, &value2, &errs); diff --git a/example/readFromString/readFromString.cpp b/example/readFromString/readFromString.cpp index 0b29a4e86..520b96b2d 100644 --- a/example/readFromString/readFromString.cpp +++ b/example/readFromString/readFromString.cpp @@ -22,7 +22,7 @@ int main() { reader.parse(rawJson, root); } else { Json::CharReaderBuilder builder; - const std::unique_ptr reader(builder.newCharReader()); + const std::unique_ptr reader = builder.newCharReader(); if (!reader->parse(rawJson.c_str(), rawJson.c_str() + rawJsonLength, &root, &err)) { std::cout << "error" << std::endl; diff --git a/include/json/reader.h b/include/json/reader.h index 46975d86f..bc3aedbc2 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -270,7 +270,7 @@ class JSON_API CharReader { /** \brief Allocate a CharReader via operator new(). * \throw std::exception if something goes wrong (e.g. invalid settings) */ - virtual CharReader* newCharReader() const = 0; + virtual std::unique_ptr newCharReader() const = 0; }; // Factory }; // CharReader @@ -337,7 +337,7 @@ class JSON_API CharReaderBuilder : public CharReader::Factory { CharReaderBuilder(); ~CharReaderBuilder() override; - CharReader* newCharReader() const override; + std::unique_ptr newCharReader() const override; /** \return true if 'settings' are legal and consistent; * otherwise, indicate bad settings via 'invalid'. diff --git a/src/jsontestrunner/main.cpp b/src/jsontestrunner/main.cpp index df717ffd5..60ca44537 100644 --- a/src/jsontestrunner/main.cpp +++ b/src/jsontestrunner/main.cpp @@ -138,7 +138,7 @@ static int parseAndSaveValueTree(const Json::String& input, features.allowDroppedNullPlaceholders_; builder.settings_["allowNumericKeys"] = features.allowNumericKeys_; - std::unique_ptr reader(builder.newCharReader()); + std::unique_ptr reader = builder.newCharReader(); Json::String errors; const bool parsingSuccessful = reader->parse(input.data(), input.data() + input.size(), root, &errors); diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 1ac5e81ab..6d1650dfd 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1891,7 +1891,7 @@ class OurCharReader : public CharReader { CharReaderBuilder::CharReaderBuilder() { setDefaults(&settings_); } CharReaderBuilder::~CharReaderBuilder() = default; -CharReader* CharReaderBuilder::newCharReader() const { +std::unique_ptr CharReaderBuilder::newCharReader() const { bool collectComments = settings_["collectComments"].asBool(); OurFeatures features = OurFeatures::all(); features.allowComments_ = settings_["allowComments"].asBool(); @@ -1909,7 +1909,7 @@ CharReader* CharReaderBuilder::newCharReader() const { features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); features.skipBom_ = settings_["skipBom"].asBool(); - return new OurCharReader(collectComments, features); + return std::make_unique(OurCharReader(collectComments, features)); } bool CharReaderBuilder::validate(Json::Value* invalid) const { @@ -1987,7 +1987,7 @@ bool parseFromStream(CharReader::Factory const& fact, IStream& sin, Value* root, char const* begin = doc.data(); char const* end = begin + doc.size(); // Note that we do not actually need a null-terminator. - CharReaderPtr const reader(fact.newCharReader()); + CharReaderPtr const reader = fact.newCharReader(); return reader->parse(begin, end, root, errs); } diff --git a/src/test_lib_json/fuzz.cpp b/src/test_lib_json/fuzz.cpp index 5b75c22e6..893277985 100644 --- a/src/test_lib_json/fuzz.cpp +++ b/src/test_lib_json/fuzz.cpp @@ -41,7 +41,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { builder.settings_["collectComments"] = hash_settings & (1 << 9); builder.settings_["allowTrailingCommas_"] = hash_settings & (1 << 10); - std::unique_ptr reader(builder.newCharReader()); + std::unique_ptr reader = builder.newCharReader(); Json::Value root; const auto data_str = reinterpret_cast(data); diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index d0f5364ac..beae9f51d 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -2994,7 +2994,7 @@ struct CharReaderTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" : "value" })"; @@ -3005,7 +3005,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrorsTestingOffsets) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = "{ \"property\" : [\"value\", \"value2\"], \"obj\" : " @@ -3018,7 +3018,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrorsTestingOffsets) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseNumber) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; { @@ -3034,7 +3034,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseNumber) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3090,7 +3090,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { } { b.settings_["allowSingleQuotes"] = true; - CharReaderPtr charreader(b.newCharReader()); + CharReaderPtr charreader = b.newCharReader(); char const doc[] = R"({'a': 'x\ty', "b":'x\\y'})"; bool ok = charreader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3103,7 +3103,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3134,7 +3134,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseObjectWithErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3157,7 +3157,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseObjectWithErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseArrayWithErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3180,7 +3180,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseArrayWithErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithOneError) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" :: "value" })"; @@ -3193,7 +3193,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithOneError) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseChineseWithOneError) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = "{ \"pr佐藤erty\" :: \"value\" }"; @@ -3206,7 +3206,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseChineseWithOneError) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithDetailError) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" : "v\alue" })"; @@ -3223,7 +3223,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { char const doc[] = R"({ "property" : "value" })"; { b.settings_["stackLimit"] = 2; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3232,7 +3232,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { } { b.settings_["stackLimit"] = 1; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; JSONTEST_ASSERT_THROWS( reader->parse(doc, doc + std::strlen(doc), &root, &errs)); @@ -3256,7 +3256,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderStrictModeTest, dupKeys) { R"({ "property" : "value", "key" : "val1", "key" : "val2" })"; { b.strictMode(&b.settings_); - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3275,7 +3275,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { char const doc[] = R"( "property" : "value" })"; { b.settings_["failIfExtra"] = false; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3284,7 +3284,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { } { b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3295,7 +3295,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { } { b.strictMode(&b.settings_); - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3307,7 +3307,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { { b.strictMode(&b.settings_); b.settings_["failIfExtra"] = false; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3325,7 +3325,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue107) { Json::Value root; char const doc[] = "1:2:3"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3340,7 +3340,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterObject) { { char const doc[] = "{ \"property\" : \"value\" } //trailing\n//comment\n"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3353,7 +3353,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterArray) { Json::Value root; char const doc[] = "[ \"property\" , \"value\" ] //trailing\n//comment\n"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3365,7 +3365,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterBool) { Json::Value root; char const doc[] = " true /*trailing\ncomment*/"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3376,7 +3376,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterBool) { JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, parseComment) { Json::CharReaderBuilder b; b.settings_["failIfExtra"] = true; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3454,7 +3454,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowDropNullTest, issue178) { for (const auto& spec : specs) { Json::CharReaderBuilder b; b.settings_["allowDroppedNullPlaceholders"] = true; - std::unique_ptr reader(b.newCharReader()); + std::unique_ptr reader = b.newCharReader(); Json::Value root; Json::String errs; @@ -3475,7 +3475,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowNumericKeysTest, allowNumericKeys) { b.settings_["allowNumericKeys"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); char const doc[] = "{15:true,-16:true,12.01:true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3493,7 +3493,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowSingleQuotesTest, issue182) { b.settings_["allowSingleQuotes"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); { char const doc[] = "{'a':true,\"b\":true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3521,7 +3521,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowZeroesTest, issue176) { b.settings_["allowSingleQuotes"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); { char const doc[] = "{'a':true,\"b\":true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3546,7 +3546,7 @@ struct CharReaderAllowSpecialFloatsTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderAllowSpecialFloatsTest, specialFloat) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; { @@ -3574,7 +3574,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowSpecialFloatsTest, issue209) { b.settings_["allowSpecialFloats"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); { char const doc[] = R"({"a":NaN,"b":Infinity,"c":-Infinity,"d":+Infinity})"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3655,7 +3655,7 @@ JSONTEST_FIXTURE_LOCAL(EscapeSequenceTest, readerParseEscapeSequence) { JSONTEST_FIXTURE_LOCAL(EscapeSequenceTest, charReaderParseEscapeSequence) { Json::CharReaderBuilder b; - CharReaderPtr reader(b.newCharReader()); + CharReaderPtr reader = b.newCharReader(); Json::Value root; Json::String errs; char const doc[] = "[\"\\\"\",\"\\/\",\"\\\\\",\"\\b\"," From f39db4fd99af3bd7454542db7d80e0e3ec9d9251 Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Tue, 19 Jul 2022 22:11:44 +0300 Subject: [PATCH 02/12] check error --- src/lib_json/json_reader.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 6d1650dfd..4c49d851e 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1909,7 +1909,8 @@ std::unique_ptr CharReaderBuilder::newCharReader() const { features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); features.skipBom_ = settings_["skipBom"].asBool(); - return std::make_unique(OurCharReader(collectComments, features)); + CharReader* preader = OurCharReader(collectComments, features); + return std::make_unique(preader); } bool CharReaderBuilder::validate(Json::Value* invalid) const { From 1bda7f9af151492832a021a9f9c4bda6238e5400 Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Tue, 19 Jul 2022 22:14:24 +0300 Subject: [PATCH 03/12] fix error --- src/lib_json/json_reader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 4c49d851e..6e7111016 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1909,7 +1909,7 @@ std::unique_ptr CharReaderBuilder::newCharReader() const { features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); features.skipBom_ = settings_["skipBom"].asBool(); - CharReader* preader = OurCharReader(collectComments, features); + CharReader* preader = new OurCharReader(collectComments, features); return std::make_unique(preader); } From 2f202852f010eeb640add9e51d24535e83bc40ab Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Tue, 19 Jul 2022 22:29:34 +0300 Subject: [PATCH 04/12] create OurCharReader unique_ptr --- src/lib_json/json_reader.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 6e7111016..9bb128081 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1909,8 +1909,7 @@ std::unique_ptr CharReaderBuilder::newCharReader() const { features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); features.skipBom_ = settings_["skipBom"].asBool(); - CharReader* preader = new OurCharReader(collectComments, features); - return std::make_unique(preader); + return std::make_unique(collectComments, features); } bool CharReaderBuilder::validate(Json::Value* invalid) const { From 15972d98e124a558e8ee650ee5e79e4aca12ab3e Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Thu, 21 Jul 2022 09:40:52 +0300 Subject: [PATCH 05/12] fix build with ninja --- src/lib_json/json_reader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 9bb128081..0ae5b0311 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1909,7 +1909,7 @@ std::unique_ptr CharReaderBuilder::newCharReader() const { features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); features.skipBom_ = settings_["skipBom"].asBool(); - return std::make_unique(collectComments, features); + return std::unique_ptr(new OurCharReader(collectComments, features)); } bool CharReaderBuilder::validate(Json::Value* invalid) const { From a1468ffa556d7ab489c8c745af1f9a141c7a49e2 Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Thu, 21 Jul 2022 10:03:47 +0300 Subject: [PATCH 06/12] clang-format --- src/lib_json/json_reader.cpp | 7 ++++--- src/lib_json/json_writer.cpp | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 0ae5b0311..5b38262ea 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -608,7 +608,7 @@ bool Reader::decodeDouble(Token& token, Value& decoded) { value = -std::numeric_limits::infinity(); else if (!std::isinf(value)) return addError( - "'" + String(token.start_, token.end_) + "' is not a number.", token); + "'" + String(token.start_, token.end_) + "' is not a number.", token); } decoded = value; return true; @@ -1660,7 +1660,7 @@ bool OurReader::decodeDouble(Token& token, Value& decoded) { value = -std::numeric_limits::infinity(); else if (!std::isinf(value)) return addError( - "'" + String(token.start_, token.end_) + "' is not a number.", token); + "'" + String(token.start_, token.end_) + "' is not a number.", token); } decoded = value; return true; @@ -1909,7 +1909,8 @@ std::unique_ptr CharReaderBuilder::newCharReader() const { features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); features.skipBom_ = settings_["skipBom"].asBool(); - return std::unique_ptr(new OurCharReader(collectComments, features)); + return std::unique_ptr( + new OurCharReader(collectComments, features)); } bool CharReaderBuilder::validate(Json::Value* invalid) const { diff --git a/src/lib_json/json_writer.cpp b/src/lib_json/json_writer.cpp index 0dd160e45..239c429a1 100644 --- a/src/lib_json/json_writer.cpp +++ b/src/lib_json/json_writer.cpp @@ -132,8 +132,9 @@ String valueToString(double value, bool useSpecialFloats, if (!isfinite(value)) { static const char* const reps[2][3] = {{"NaN", "-Infinity", "Infinity"}, {"null", "-1e+9999", "1e+9999"}}; - return reps[useSpecialFloats ? 0 : 1] - [isnan(value) ? 0 : (value < 0) ? 1 : 2]; + return reps[useSpecialFloats ? 0 : 1][isnan(value) ? 0 + : (value < 0) ? 1 + : 2]; } String buffer(size_t(36), '\0'); From 72fa37aeb7b4f482bf619bd778ddda53e7ba2fc9 Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Tue, 26 Jul 2022 17:29:45 +0300 Subject: [PATCH 07/12] Add function to CharReaderBuilder named makeCharReader the that function return unique_ptr, in order to prevent breaking changes, mark the newCharReader function as deprecated --- example/readFromString/readFromString.cpp | 2 +- include/json/reader.h | 7 +-- src/jsontestrunner/main.cpp | 2 +- src/lib_json/json_reader.cpp | 14 ++++-- src/test_lib_json/fuzz.cpp | 2 +- src/test_lib_json/main.cpp | 60 +++++++++++------------ 6 files changed, 47 insertions(+), 40 deletions(-) diff --git a/example/readFromString/readFromString.cpp b/example/readFromString/readFromString.cpp index 520b96b2d..7cda532b4 100644 --- a/example/readFromString/readFromString.cpp +++ b/example/readFromString/readFromString.cpp @@ -22,7 +22,7 @@ int main() { reader.parse(rawJson, root); } else { Json::CharReaderBuilder builder; - const std::unique_ptr reader = builder.newCharReader(); + const std::unique_ptr reader = builder.makeCharReader(); if (!reader->parse(rawJson.c_str(), rawJson.c_str() + rawJsonLength, &root, &err)) { std::cout << "error" << std::endl; diff --git a/include/json/reader.h b/include/json/reader.h index bc3aedbc2..dcaf26c9e 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -270,7 +270,8 @@ class JSON_API CharReader { /** \brief Allocate a CharReader via operator new(). * \throw std::exception if something goes wrong (e.g. invalid settings) */ - virtual std::unique_ptr newCharReader() const = 0; + virtual CharReader* newCharReader() const = 0; + virtual std::unique_ptr makeCharReader() const = 0; }; // Factory }; // CharReader @@ -337,8 +338,8 @@ class JSON_API CharReaderBuilder : public CharReader::Factory { CharReaderBuilder(); ~CharReaderBuilder() override; - std::unique_ptr newCharReader() const override; - + CharReader* newCharReader() const override; + std::unique_ptr makeCharReader() const override; /** \return true if 'settings' are legal and consistent; * otherwise, indicate bad settings via 'invalid'. */ diff --git a/src/jsontestrunner/main.cpp b/src/jsontestrunner/main.cpp index 60ca44537..acb13767a 100644 --- a/src/jsontestrunner/main.cpp +++ b/src/jsontestrunner/main.cpp @@ -138,7 +138,7 @@ static int parseAndSaveValueTree(const Json::String& input, features.allowDroppedNullPlaceholders_; builder.settings_["allowNumericKeys"] = features.allowNumericKeys_; - std::unique_ptr reader = builder.newCharReader(); + std::unique_ptr reader = builder.makeCharReader(); Json::String errors; const bool parsingSuccessful = reader->parse(input.data(), input.data() + input.size(), root, &errors); diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 5b38262ea..0584792b9 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1891,7 +1891,14 @@ class OurCharReader : public CharReader { CharReaderBuilder::CharReaderBuilder() { setDefaults(&settings_); } CharReaderBuilder::~CharReaderBuilder() = default; -std::unique_ptr CharReaderBuilder::newCharReader() const { +std::unique_ptr CharReaderBuilder::makeCharReader() const { + return std::unique_ptr(newCharReader()); +} + +/* + * \deprecated Use makeCharReader. + */ +CharReader* CharReaderBuilder::newCharReader() const { bool collectComments = settings_["collectComments"].asBool(); OurFeatures features = OurFeatures::all(); features.allowComments_ = settings_["allowComments"].asBool(); @@ -1909,8 +1916,7 @@ std::unique_ptr CharReaderBuilder::newCharReader() const { features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); features.skipBom_ = settings_["skipBom"].asBool(); - return std::unique_ptr( - new OurCharReader(collectComments, features)); + return new OurCharReader(collectComments, features); } bool CharReaderBuilder::validate(Json::Value* invalid) const { @@ -1988,7 +1994,7 @@ bool parseFromStream(CharReader::Factory const& fact, IStream& sin, Value* root, char const* begin = doc.data(); char const* end = begin + doc.size(); // Note that we do not actually need a null-terminator. - CharReaderPtr const reader = fact.newCharReader(); + CharReaderPtr const reader = fact.makeCharReader(); return reader->parse(begin, end, root, errs); } diff --git a/src/test_lib_json/fuzz.cpp b/src/test_lib_json/fuzz.cpp index 893277985..0031a14d2 100644 --- a/src/test_lib_json/fuzz.cpp +++ b/src/test_lib_json/fuzz.cpp @@ -41,7 +41,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { builder.settings_["collectComments"] = hash_settings & (1 << 9); builder.settings_["allowTrailingCommas_"] = hash_settings & (1 << 10); - std::unique_ptr reader = builder.newCharReader(); + std::unique_ptr reader = builder.makeCharReader(); Json::Value root; const auto data_str = reinterpret_cast(data); diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index beae9f51d..80cd2a590 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -2994,7 +2994,7 @@ struct CharReaderTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" : "value" })"; @@ -3005,7 +3005,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrorsTestingOffsets) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; Json::Value root; char const doc[] = "{ \"property\" : [\"value\", \"value2\"], \"obj\" : " @@ -3018,7 +3018,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrorsTestingOffsets) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseNumber) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; Json::Value root; { @@ -3034,7 +3034,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseNumber) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::Value root; Json::String errs; { @@ -3090,7 +3090,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { } { b.settings_["allowSingleQuotes"] = true; - CharReaderPtr charreader = b.newCharReader(); + CharReaderPtr charreader = b.makeCharReader(); char const doc[] = R"({'a': 'x\ty', "b":'x\\y'})"; bool ok = charreader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3103,7 +3103,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::Value root; Json::String errs; { @@ -3134,7 +3134,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseObjectWithErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::Value root; Json::String errs; { @@ -3157,7 +3157,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseObjectWithErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseArrayWithErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::Value root; Json::String errs; { @@ -3180,7 +3180,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseArrayWithErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithOneError) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" :: "value" })"; @@ -3193,7 +3193,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithOneError) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseChineseWithOneError) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; Json::Value root; char const doc[] = "{ \"pr佐藤erty\" :: \"value\" }"; @@ -3206,7 +3206,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseChineseWithOneError) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithDetailError) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; Json::Value root; char const doc[] = R"({ "property" : "v\alue" })"; @@ -3223,7 +3223,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { char const doc[] = R"({ "property" : "value" })"; { b.settings_["stackLimit"] = 2; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3232,7 +3232,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { } { b.settings_["stackLimit"] = 1; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; JSONTEST_ASSERT_THROWS( reader->parse(doc, doc + std::strlen(doc), &root, &errs)); @@ -3256,7 +3256,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderStrictModeTest, dupKeys) { R"({ "property" : "value", "key" : "val1", "key" : "val2" })"; { b.strictMode(&b.settings_); - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3275,7 +3275,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { char const doc[] = R"( "property" : "value" })"; { b.settings_["failIfExtra"] = false; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3284,7 +3284,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { } { b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3295,7 +3295,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { } { b.strictMode(&b.settings_); - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3307,7 +3307,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { { b.strictMode(&b.settings_); b.settings_["failIfExtra"] = false; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3325,7 +3325,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue107) { Json::Value root; char const doc[] = "1:2:3"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3340,7 +3340,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterObject) { { char const doc[] = "{ \"property\" : \"value\" } //trailing\n//comment\n"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3353,7 +3353,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterArray) { Json::Value root; char const doc[] = "[ \"property\" , \"value\" ] //trailing\n//comment\n"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3365,7 +3365,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterBool) { Json::Value root; char const doc[] = " true /*trailing\ncomment*/"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3376,7 +3376,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterBool) { JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, parseComment) { Json::CharReaderBuilder b; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::Value root; Json::String errs; { @@ -3454,7 +3454,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowDropNullTest, issue178) { for (const auto& spec : specs) { Json::CharReaderBuilder b; b.settings_["allowDroppedNullPlaceholders"] = true; - std::unique_ptr reader = b.newCharReader(); + std::unique_ptr reader = b.makeCharReader(); Json::Value root; Json::String errs; @@ -3475,7 +3475,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowNumericKeysTest, allowNumericKeys) { b.settings_["allowNumericKeys"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); char const doc[] = "{15:true,-16:true,12.01:true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3493,7 +3493,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowSingleQuotesTest, issue182) { b.settings_["allowSingleQuotes"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); { char const doc[] = "{'a':true,\"b\":true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3521,7 +3521,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowZeroesTest, issue176) { b.settings_["allowSingleQuotes"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); { char const doc[] = "{'a':true,\"b\":true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3546,7 +3546,7 @@ struct CharReaderAllowSpecialFloatsTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderAllowSpecialFloatsTest, specialFloat) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::Value root; Json::String errs; { @@ -3574,7 +3574,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowSpecialFloatsTest, issue209) { b.settings_["allowSpecialFloats"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); { char const doc[] = R"({"a":NaN,"b":Infinity,"c":-Infinity,"d":+Infinity})"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3655,7 +3655,7 @@ JSONTEST_FIXTURE_LOCAL(EscapeSequenceTest, readerParseEscapeSequence) { JSONTEST_FIXTURE_LOCAL(EscapeSequenceTest, charReaderParseEscapeSequence) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.newCharReader(); + CharReaderPtr reader = b.makeCharReader(); Json::Value root; Json::String errs; char const doc[] = "[\"\\\"\",\"\\/\",\"\\\\\",\"\\b\"," From 4f41c0f9380d56ea65d9bf4b1d18865fcbe09d36 Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Tue, 26 Jul 2022 23:56:57 +0300 Subject: [PATCH 08/12] revert changes, move deprecated to header, --- doc/jsoncpp.dox | 2 +- example/readFromString/readFromString.cpp | 2 +- include/json/reader.h | 3 ++ src/jsontestrunner/main.cpp | 2 +- src/lib_json/json_reader.cpp | 5 +- src/test_lib_json/fuzz.cpp | 2 +- src/test_lib_json/main.cpp | 60 +++++++++++------------ 7 files changed, 38 insertions(+), 38 deletions(-) diff --git a/doc/jsoncpp.dox b/doc/jsoncpp.dox index ce4c9765c..381fb9185 100644 --- a/doc/jsoncpp.dox +++ b/doc/jsoncpp.dox @@ -116,7 +116,7 @@ CharReaders and StreamWriters are not thread-safe, but they are re-usable. \code Json::CharReaderBuilder rbuilder; cfg >> rbuilder.settings_; -std::unique_ptr const reader = rbuilder.newCharReader(); +std::unique_ptr const reader = rbuilder.makeCharReader(); reader->parse(start, stop, &value1, &errs); // ... reader->parse(start, stop, &value2, &errs); diff --git a/example/readFromString/readFromString.cpp b/example/readFromString/readFromString.cpp index 7cda532b4..0b29a4e86 100644 --- a/example/readFromString/readFromString.cpp +++ b/example/readFromString/readFromString.cpp @@ -22,7 +22,7 @@ int main() { reader.parse(rawJson, root); } else { Json::CharReaderBuilder builder; - const std::unique_ptr reader = builder.makeCharReader(); + const std::unique_ptr reader(builder.newCharReader()); if (!reader->parse(rawJson.c_str(), rawJson.c_str() + rawJsonLength, &root, &err)) { std::cout << "error" << std::endl; diff --git a/include/json/reader.h b/include/json/reader.h index dcaf26c9e..9e44d1273 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -338,6 +338,9 @@ class JSON_API CharReaderBuilder : public CharReader::Factory { CharReaderBuilder(); ~CharReaderBuilder() override; + /* + * \deprecated Use makeCharReader. + */ CharReader* newCharReader() const override; std::unique_ptr makeCharReader() const override; /** \return true if 'settings' are legal and consistent; diff --git a/src/jsontestrunner/main.cpp b/src/jsontestrunner/main.cpp index acb13767a..df717ffd5 100644 --- a/src/jsontestrunner/main.cpp +++ b/src/jsontestrunner/main.cpp @@ -138,7 +138,7 @@ static int parseAndSaveValueTree(const Json::String& input, features.allowDroppedNullPlaceholders_; builder.settings_["allowNumericKeys"] = features.allowNumericKeys_; - std::unique_ptr reader = builder.makeCharReader(); + std::unique_ptr reader(builder.newCharReader()); Json::String errors; const bool parsingSuccessful = reader->parse(input.data(), input.data() + input.size(), root, &errors); diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 0584792b9..b79cafb46 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1895,9 +1895,6 @@ std::unique_ptr CharReaderBuilder::makeCharReader() const { return std::unique_ptr(newCharReader()); } -/* - * \deprecated Use makeCharReader. - */ CharReader* CharReaderBuilder::newCharReader() const { bool collectComments = settings_["collectComments"].asBool(); OurFeatures features = OurFeatures::all(); @@ -1994,7 +1991,7 @@ bool parseFromStream(CharReader::Factory const& fact, IStream& sin, Value* root, char const* begin = doc.data(); char const* end = begin + doc.size(); // Note that we do not actually need a null-terminator. - CharReaderPtr const reader = fact.makeCharReader(); + CharReaderPtr const reader(fact.newCharReader()); return reader->parse(begin, end, root, errs); } diff --git a/src/test_lib_json/fuzz.cpp b/src/test_lib_json/fuzz.cpp index 0031a14d2..5b75c22e6 100644 --- a/src/test_lib_json/fuzz.cpp +++ b/src/test_lib_json/fuzz.cpp @@ -41,7 +41,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { builder.settings_["collectComments"] = hash_settings & (1 << 9); builder.settings_["allowTrailingCommas_"] = hash_settings & (1 << 10); - std::unique_ptr reader = builder.makeCharReader(); + std::unique_ptr reader(builder.newCharReader()); Json::Value root; const auto data_str = reinterpret_cast(data); diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index 80cd2a590..d0f5364ac 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -2994,7 +2994,7 @@ struct CharReaderTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; Json::Value root; char const doc[] = R"({ "property" : "value" })"; @@ -3005,7 +3005,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrorsTestingOffsets) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; Json::Value root; char const doc[] = "{ \"property\" : [\"value\", \"value2\"], \"obj\" : " @@ -3018,7 +3018,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrorsTestingOffsets) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseNumber) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; Json::Value root; { @@ -3034,7 +3034,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseNumber) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::Value root; Json::String errs; { @@ -3090,7 +3090,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { } { b.settings_["allowSingleQuotes"] = true; - CharReaderPtr charreader = b.makeCharReader(); + CharReaderPtr charreader(b.newCharReader()); char const doc[] = R"({'a': 'x\ty', "b":'x\\y'})"; bool ok = charreader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3103,7 +3103,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseString) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::Value root; Json::String errs; { @@ -3134,7 +3134,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseComment) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseObjectWithErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::Value root; Json::String errs; { @@ -3157,7 +3157,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseObjectWithErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseArrayWithErrors) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::Value root; Json::String errs; { @@ -3180,7 +3180,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseArrayWithErrors) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithOneError) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; Json::Value root; char const doc[] = R"({ "property" :: "value" })"; @@ -3193,7 +3193,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithOneError) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseChineseWithOneError) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; Json::Value root; char const doc[] = "{ \"pr佐藤erty\" :: \"value\" }"; @@ -3206,7 +3206,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseChineseWithOneError) { JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithDetailError) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; Json::Value root; char const doc[] = R"({ "property" : "v\alue" })"; @@ -3223,7 +3223,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { char const doc[] = R"({ "property" : "value" })"; { b.settings_["stackLimit"] = 2; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3232,7 +3232,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) { } { b.settings_["stackLimit"] = 1; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; JSONTEST_ASSERT_THROWS( reader->parse(doc, doc + std::strlen(doc), &root, &errs)); @@ -3256,7 +3256,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderStrictModeTest, dupKeys) { R"({ "property" : "value", "key" : "val1", "key" : "val2" })"; { b.strictMode(&b.settings_); - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3275,7 +3275,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { char const doc[] = R"( "property" : "value" })"; { b.settings_["failIfExtra"] = false; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3284,7 +3284,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { } { b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3295,7 +3295,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { } { b.strictMode(&b.settings_); - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3307,7 +3307,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue164) { { b.strictMode(&b.settings_); b.settings_["failIfExtra"] = false; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3325,7 +3325,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, issue107) { Json::Value root; char const doc[] = "1:2:3"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(!ok); @@ -3340,7 +3340,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterObject) { { char const doc[] = "{ \"property\" : \"value\" } //trailing\n//comment\n"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3353,7 +3353,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterArray) { Json::Value root; char const doc[] = "[ \"property\" , \"value\" ] //trailing\n//comment\n"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3365,7 +3365,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterBool) { Json::Value root; char const doc[] = " true /*trailing\ncomment*/"; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::String errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3376,7 +3376,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, commentAfterBool) { JSONTEST_FIXTURE_LOCAL(CharReaderFailIfExtraTest, parseComment) { Json::CharReaderBuilder b; b.settings_["failIfExtra"] = true; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::Value root; Json::String errs; { @@ -3454,7 +3454,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowDropNullTest, issue178) { for (const auto& spec : specs) { Json::CharReaderBuilder b; b.settings_["allowDroppedNullPlaceholders"] = true; - std::unique_ptr reader = b.makeCharReader(); + std::unique_ptr reader(b.newCharReader()); Json::Value root; Json::String errs; @@ -3475,7 +3475,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowNumericKeysTest, allowNumericKeys) { b.settings_["allowNumericKeys"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); char const doc[] = "{15:true,-16:true,12.01:true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); @@ -3493,7 +3493,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowSingleQuotesTest, issue182) { b.settings_["allowSingleQuotes"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); { char const doc[] = "{'a':true,\"b\":true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3521,7 +3521,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowZeroesTest, issue176) { b.settings_["allowSingleQuotes"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); { char const doc[] = "{'a':true,\"b\":true}"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3546,7 +3546,7 @@ struct CharReaderAllowSpecialFloatsTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderAllowSpecialFloatsTest, specialFloat) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::Value root; Json::String errs; { @@ -3574,7 +3574,7 @@ JSONTEST_FIXTURE_LOCAL(CharReaderAllowSpecialFloatsTest, issue209) { b.settings_["allowSpecialFloats"] = true; Json::Value root; Json::String errs; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); { char const doc[] = R"({"a":NaN,"b":Infinity,"c":-Infinity,"d":+Infinity})"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); @@ -3655,7 +3655,7 @@ JSONTEST_FIXTURE_LOCAL(EscapeSequenceTest, readerParseEscapeSequence) { JSONTEST_FIXTURE_LOCAL(EscapeSequenceTest, charReaderParseEscapeSequence) { Json::CharReaderBuilder b; - CharReaderPtr reader = b.makeCharReader(); + CharReaderPtr reader(b.newCharReader()); Json::Value root; Json::String errs; char const doc[] = "[\"\\\"\",\"\\/\",\"\\\\\",\"\\b\"," From f2ec46663f43a2bcf53e654a7070e47019333041 Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Wed, 27 Jul 2022 00:54:53 +0300 Subject: [PATCH 09/12] remove virtual, and implemnt the makeCharReader in the CharReeader::Factory interface --- include/json/reader.h | 3 +-- src/lib_json/json_reader.cpp | 7 ++++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/json/reader.h b/include/json/reader.h index 9e44d1273..6fe7fbaff 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -271,7 +271,7 @@ class JSON_API CharReader { * \throw std::exception if something goes wrong (e.g. invalid settings) */ virtual CharReader* newCharReader() const = 0; - virtual std::unique_ptr makeCharReader() const = 0; + std::unique_ptr makeCharReader() const; }; // Factory }; // CharReader @@ -342,7 +342,6 @@ class JSON_API CharReaderBuilder : public CharReader::Factory { * \deprecated Use makeCharReader. */ CharReader* newCharReader() const override; - std::unique_ptr makeCharReader() const override; /** \return true if 'settings' are legal and consistent; * otherwise, indicate bad settings via 'invalid'. */ diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index b79cafb46..364b45381 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1872,6 +1872,10 @@ std::vector OurReader::getStructuredErrors() const { return allErrors; } +std::unique_ptr CharReader::Factory::makeCharReader() const { + return std::unique_ptr(newCharReader()); +} + class OurCharReader : public CharReader { bool const collectComments_; OurReader reader_; @@ -1891,9 +1895,6 @@ class OurCharReader : public CharReader { CharReaderBuilder::CharReaderBuilder() { setDefaults(&settings_); } CharReaderBuilder::~CharReaderBuilder() = default; -std::unique_ptr CharReaderBuilder::makeCharReader() const { - return std::unique_ptr(newCharReader()); -} CharReader* CharReaderBuilder::newCharReader() const { bool collectComments = settings_["collectComments"].asBool(); From d60e466dd17d851b9eb1534eb6649bf67382076e Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Fri, 5 Aug 2022 15:41:07 +0300 Subject: [PATCH 10/12] PR changes: add document of makeCharReader, revert changes made by `reformat.sh` script --- include/json/reader.h | 4 ++++ src/lib_json/json_reader.cpp | 5 ++--- src/lib_json/json_writer.cpp | 5 ++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/json/reader.h b/include/json/reader.h index 6fe7fbaff..7bb83b2d3 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -271,6 +271,10 @@ class JSON_API CharReader { * \throw std::exception if something goes wrong (e.g. invalid settings) */ virtual CharReader* newCharReader() const = 0; + /** \brief Allocate a CharReader via newCharReader(). + * wrap the object in std::unique_ptr to esnure deletion. + * \throw std::exception if something goes wrong (e.g. invalid settings) + */ std::unique_ptr makeCharReader() const; }; // Factory }; // CharReader diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 364b45381..00b560a5f 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -608,7 +608,7 @@ bool Reader::decodeDouble(Token& token, Value& decoded) { value = -std::numeric_limits::infinity(); else if (!std::isinf(value)) return addError( - "'" + String(token.start_, token.end_) + "' is not a number.", token); + "'" + String(token.start_, token.end_) + "' is not a number.", token); } decoded = value; return true; @@ -1660,7 +1660,7 @@ bool OurReader::decodeDouble(Token& token, Value& decoded) { value = -std::numeric_limits::infinity(); else if (!std::isinf(value)) return addError( - "'" + String(token.start_, token.end_) + "' is not a number.", token); + "'" + String(token.start_, token.end_) + "' is not a number.", token); } decoded = value; return true; @@ -1895,7 +1895,6 @@ class OurCharReader : public CharReader { CharReaderBuilder::CharReaderBuilder() { setDefaults(&settings_); } CharReaderBuilder::~CharReaderBuilder() = default; - CharReader* CharReaderBuilder::newCharReader() const { bool collectComments = settings_["collectComments"].asBool(); OurFeatures features = OurFeatures::all(); diff --git a/src/lib_json/json_writer.cpp b/src/lib_json/json_writer.cpp index 239c429a1..0dd160e45 100644 --- a/src/lib_json/json_writer.cpp +++ b/src/lib_json/json_writer.cpp @@ -132,9 +132,8 @@ String valueToString(double value, bool useSpecialFloats, if (!isfinite(value)) { static const char* const reps[2][3] = {{"NaN", "-Infinity", "Infinity"}, {"null", "-1e+9999", "1e+9999"}}; - return reps[useSpecialFloats ? 0 : 1][isnan(value) ? 0 - : (value < 0) ? 1 - : 2]; + return reps[useSpecialFloats ? 0 : 1] + [isnan(value) ? 0 : (value < 0) ? 1 : 2]; } String buffer(size_t(36), '\0'); From a5ef87288eba561ac10fa98105502b268c8859eb Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Fri, 5 Aug 2022 15:46:43 +0300 Subject: [PATCH 11/12] revert missing empty line bt `reformat.sh` --- include/json/reader.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/json/reader.h b/include/json/reader.h index 7bb83b2d3..2a134405d 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -271,6 +271,7 @@ class JSON_API CharReader { * \throw std::exception if something goes wrong (e.g. invalid settings) */ virtual CharReader* newCharReader() const = 0; + /** \brief Allocate a CharReader via newCharReader(). * wrap the object in std::unique_ptr to esnure deletion. * \throw std::exception if something goes wrong (e.g. invalid settings) @@ -346,6 +347,7 @@ class JSON_API CharReaderBuilder : public CharReader::Factory { * \deprecated Use makeCharReader. */ CharReader* newCharReader() const override; + /** \return true if 'settings' are legal and consistent; * otherwise, indicate bad settings via 'invalid'. */ From e90c3c3303bb69f3b51c48d3dd81033828246fa9 Mon Sep 17 00:00:00 2001 From: Assaf Passal Date: Mon, 22 Aug 2022 10:38:08 +0300 Subject: [PATCH 12/12] add makeCharReader test, fix spelling --- include/json/reader.h | 5 +---- src/test_lib_json/main.cpp | 13 +++++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/include/json/reader.h b/include/json/reader.h index 2a134405d..dd86d64eb 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -272,10 +272,7 @@ class JSON_API CharReader { */ virtual CharReader* newCharReader() const = 0; - /** \brief Allocate a CharReader via newCharReader(). - * wrap the object in std::unique_ptr to esnure deletion. - * \throw std::exception if something goes wrong (e.g. invalid settings) - */ + /** Wrap newCharReader's result in a std::unique_ptr. Throws if newCharReader throws. */ std::unique_ptr makeCharReader() const; }; // Factory }; // CharReader diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index d0f5364ac..cafca3223 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -2990,6 +2990,19 @@ JSONTEST_FIXTURE_LOCAL(ReaderTest, allowNumericKeysTest) { checkParse(R"({ 123 : "abc" })"); } +struct UniqueReaderTest : JsonTest::TestCase {}; + +JSONTEST_FIXTURE_LOCAL(UniqueReaderTest, parseWithNoErrors) { + Json::CharReaderBuilder b; + CharReaderPtr reader = b.makeCharReader(); + Json::String errs; + Json::Value root; + char const doc[] = R"({ "property" : "value" })"; + bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); + JSONTEST_ASSERT(ok); + JSONTEST_ASSERT(errs.empty()); +} + struct CharReaderTest : JsonTest::TestCase {}; JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithNoErrors) {