From 595b114216f8899eb72695394bff9cb5856313e6 Mon Sep 17 00:00:00 2001 From: StilesCrisis Date: Mon, 27 Feb 2017 22:36:48 -0800 Subject: [PATCH 1/5] Unit test Add unit tests expecting an assertion when writing an object with a key but no value. --- test/unittest/writertest.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/unittest/writertest.cpp b/test/unittest/writertest.cpp index d346e0f..398a63d 100644 --- a/test/unittest/writertest.cpp +++ b/test/unittest/writertest.cpp @@ -442,6 +442,28 @@ TEST(Writer, InvalidEventSequence) { EXPECT_THROW(writer.Int(1), AssertException); EXPECT_FALSE(writer.IsComplete()); } + + // { 'a' } + { + StringBuffer buffer; + Writer writer(buffer); + writer.StartObject(); + writer.Key("a"); + EXPECT_THROW(writer.EndObject(), AssertException); + EXPECT_FALSE(writer.IsComplete()); + } + + // { 'a':'b','c' } + { + StringBuffer buffer; + Writer writer(buffer); + writer.StartObject(); + writer.Key("a"); + writer.String("b"); + writer.Key("c"); + EXPECT_THROW(writer.EndObject(), AssertException); + EXPECT_FALSE(writer.IsComplete()); + } } TEST(Writer, NaN) { From 2e9b7b1ae6ef162b5aa0c04f2b4444be4c8b72cb Mon Sep 17 00:00:00 2001 From: StilesCrisis Date: Mon, 27 Feb 2017 22:44:13 -0800 Subject: [PATCH 2/5] Added assertion Documented existing assertions in EndObject Added new assertion in EndObject to catch error condition where objects are ended with a key but no matching value. --- include/rapidjson/writer.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/rapidjson/writer.h b/include/rapidjson/writer.h index 5b3004b..43ec5dc 100644 --- a/include/rapidjson/writer.h +++ b/include/rapidjson/writer.h @@ -221,8 +221,9 @@ public: bool EndObject(SizeType memberCount = 0) { (void)memberCount; - RAPIDJSON_ASSERT(level_stack_.GetSize() >= sizeof(Level)); - RAPIDJSON_ASSERT(!level_stack_.template Top()->inArray); + RAPIDJSON_ASSERT(level_stack_.GetSize() >= sizeof(Level)); // not inside an Object + RAPIDJSON_ASSERT(!level_stack_.template Top()->inArray); // currently inside an Array, not Object + RAPIDJSON_ASSERT(0 == level_stack_.template Top()->valueCount % 2); // Object has a Key without a Value level_stack_.template Pop(1); return EndValue(WriteEndObject()); } From fa84cd18f48294dc94f659c3f7a272b26ea5b1b5 Mon Sep 17 00:00:00 2001 From: StilesCrisis Date: Mon, 27 Feb 2017 22:53:59 -0800 Subject: [PATCH 3/5] Add matching fix for PrettyWriter PrettyWriter EndObject will now also assert if called when a key is missing its matching value. --- include/rapidjson/prettywriter.h | 6 ++-- test/unittest/prettywritertest.cpp | 51 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/include/rapidjson/prettywriter.h b/include/rapidjson/prettywriter.h index d663208..b68b687 100644 --- a/include/rapidjson/prettywriter.h +++ b/include/rapidjson/prettywriter.h @@ -136,8 +136,10 @@ public: bool EndObject(SizeType memberCount = 0) { (void)memberCount; - RAPIDJSON_ASSERT(Base::level_stack_.GetSize() >= sizeof(typename Base::Level)); - RAPIDJSON_ASSERT(!Base::level_stack_.template Top()->inArray); + RAPIDJSON_ASSERT(Base::level_stack_.GetSize() >= sizeof(typename Base::Level)); // not inside an Object + RAPIDJSON_ASSERT(!Base::level_stack_.template Top()->inArray); // currently inside an Array, not Object + RAPIDJSON_ASSERT(0 == Base::level_stack_.template Top()->valueCount % 2); // Object has a Key without a Value + bool empty = Base::level_stack_.template Pop(1)->valueCount == 0; if (!empty) { diff --git a/test/unittest/prettywritertest.cpp b/test/unittest/prettywritertest.cpp index 13d1a8d..2891c76 100644 --- a/test/unittest/prettywritertest.cpp +++ b/test/unittest/prettywritertest.cpp @@ -207,6 +207,57 @@ TEST(PrettyWriter, RawValue) { buffer.GetString()); } +TEST(PrettyWriter, InvalidEventSequence) { + // {] + { + StringBuffer buffer; + PrettyWriter writer(buffer); + writer.StartObject(); + EXPECT_THROW(writer.EndArray(), AssertException); + EXPECT_FALSE(writer.IsComplete()); + } + + // [} + { + StringBuffer buffer; + PrettyWriter writer(buffer); + writer.StartArray(); + EXPECT_THROW(writer.EndObject(), AssertException); + EXPECT_FALSE(writer.IsComplete()); + } + + // { 1: + { + StringBuffer buffer; + PrettyWriter writer(buffer); + writer.StartObject(); + EXPECT_THROW(writer.Int(1), AssertException); + EXPECT_FALSE(writer.IsComplete()); + } + + // { 'a' } + { + StringBuffer buffer; + PrettyWriter writer(buffer); + writer.StartObject(); + writer.Key("a"); + EXPECT_THROW(writer.EndObject(), AssertException); + EXPECT_FALSE(writer.IsComplete()); + } + + // { 'a':'b','c' } + { + StringBuffer buffer; + PrettyWriter writer(buffer); + writer.StartObject(); + writer.Key("a"); + writer.String("b"); + writer.Key("c"); + EXPECT_THROW(writer.EndObject(), AssertException); + EXPECT_FALSE(writer.IsComplete()); + } +} + #if RAPIDJSON_HAS_CXX11_RVALUE_REFS static PrettyWriter WriterGen(StringBuffer &target) { From 0ec4e86f14a0b801cee4a707f51245a6b735fef2 Mon Sep 17 00:00:00 2001 From: StilesCrisis Date: Mon, 27 Feb 2017 23:06:05 -0800 Subject: [PATCH 4/5] Unit test Add unit test for Issue 848 (segfault in ~Document) --- test/unittest/schematest.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unittest/schematest.cpp b/test/unittest/schematest.cpp index 4780516..30b3260 100644 --- a/test/unittest/schematest.cpp +++ b/test/unittest/schematest.cpp @@ -1281,6 +1281,12 @@ TEST(SchemaValidatingWriter, Simple) { EXPECT_TRUE(validator.GetInvalidDocumentPointer() == SchemaDocument::PointerType("")); } +TEST(Schema, Issue848) { + rapidjson::Document d; + rapidjson::SchemaDocument s(d); + rapidjson::GenericSchemaValidator v(s); +} + #if RAPIDJSON_HAS_CXX11_RVALUE_REFS static SchemaDocument ReturnSchemaDocument() { From 4643104b8a0424f8f645b2777fbcdccf9a17acbf Mon Sep 17 00:00:00 2001 From: StilesCrisis Date: Mon, 27 Feb 2017 23:28:25 -0800 Subject: [PATCH 5/5] Fix null handler construction We should not malloc the null-handler object and cast to OutputHandler; we need to actually invoke the constructor via placement new. --- include/rapidjson/schema.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rapidjson/schema.h b/include/rapidjson/schema.h index c20a838..3dddd3a 100644 --- a/include/rapidjson/schema.h +++ b/include/rapidjson/schema.h @@ -1928,7 +1928,7 @@ private: const Context& CurrentContext() const { return *schemaStack_.template Top(); } OutputHandler& CreateNullHandler() { - return *(nullHandler_ = static_cast(GetStateAllocator().Malloc(sizeof(OutputHandler)))); + return *(nullHandler_ = new (GetStateAllocator().Malloc(sizeof(OutputHandler))) OutputHandler); } static const size_t kDefaultSchemaStackCapacity = 1024;