From 74a24377a8f7043941f7fa7c64fea554305613d3 Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Thu, 26 Jun 2014 22:31:54 +0800 Subject: [PATCH 1/4] Remove setjmp()/longjmp() --- include/rapidjson/reader.h | 45 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/include/rapidjson/reader.h b/include/rapidjson/reader.h index d58702f..df4b60e 100644 --- a/include/rapidjson/reader.h +++ b/include/rapidjson/reader.h @@ -21,12 +21,21 @@ #pragma warning(disable : 4127) // conditional expression is constant #endif +#ifndef RAPIDJSON_PARSE_ERROR_NORETURN +#define RAPIDJSON_PARSE_ERROR_NORETURN(msg, offset) \ + RAPIDJSON_MULTILINEMACRO_BEGIN \ + if (!HasParseError()) {\ + parseError_ = msg; \ + errorOffset_ = offset; \ + }\ +RAPIDJSON_MULTILINEMACRO_END +#endif + #ifndef RAPIDJSON_PARSE_ERROR #define RAPIDJSON_PARSE_ERROR(msg, offset) \ RAPIDJSON_MULTILINEMACRO_BEGIN \ - parseError_ = msg; \ - errorOffset_ = offset; \ - longjmp(jmpbuf_, 1); \ + RAPIDJSON_PARSE_ERROR_NORETURN(msg, offset); \ + return; \ RAPIDJSON_MULTILINEMACRO_END #endif @@ -225,35 +234,23 @@ public: parseError_ = 0; errorOffset_ = 0; -#ifdef _MSC_VER -#pragma warning(push) -#pragma warning(disable : 4611) // interaction between '_setjmp' and C++ object destruction is non-portable -#endif - if (setjmp(jmpbuf_)) { -#ifdef _MSC_VER -#pragma warning(pop) -#endif - stack_.Clear(); - return false; - } - SkipWhitespace(is); if (is.Peek() == '\0') - RAPIDJSON_PARSE_ERROR("Text only contains white space(s)", is.Tell()); + RAPIDJSON_PARSE_ERROR_NORETURN("Text only contains white space(s)", is.Tell()); else { switch (is.Peek()) { - case '{': ParseObject(is, handler); break; - case '[': ParseArray(is, handler); break; - default: RAPIDJSON_PARSE_ERROR("Expect either an object or array at root", is.Tell()); + case '{': ParseObject(is, handler); break; + case '[': ParseArray(is, handler); break; + default: RAPIDJSON_PARSE_ERROR_NORETURN("Expect either an object or array at root", is.Tell()); } SkipWhitespace(is); if (is.Peek() != '\0') - RAPIDJSON_PARSE_ERROR("Nothing should follow the root object or array.", is.Tell()); + RAPIDJSON_PARSE_ERROR_NORETURN("Nothing should follow the root object or array.", is.Tell()); } - return true; + return !HasParseError(); } bool HasParseError() const { return parseError_ != 0; } @@ -375,8 +372,10 @@ private: codepoint -= 'A' - 10; else if (c >= 'a' && c <= 'f') codepoint -= 'a' - 10; - else - RAPIDJSON_PARSE_ERROR("Incorrect hex digit after \\u escape", s.Tell() - 1); + else { + RAPIDJSON_PARSE_ERROR_NORETURN("Incorrect hex digit after \\u escape", s.Tell() - 1); + return 0; + } } is = s; // Restore is return codepoint; From 188b99b471446675e022eb79d08076233d127e0e Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Thu, 26 Jun 2014 23:14:05 +0800 Subject: [PATCH 2/4] Clear the stack after parsing and fixes indentation. --- include/rapidjson/reader.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/rapidjson/reader.h b/include/rapidjson/reader.h index df4b60e..43e13e4 100644 --- a/include/rapidjson/reader.h +++ b/include/rapidjson/reader.h @@ -240,9 +240,9 @@ public: RAPIDJSON_PARSE_ERROR_NORETURN("Text only contains white space(s)", is.Tell()); else { switch (is.Peek()) { - case '{': ParseObject(is, handler); break; - case '[': ParseArray(is, handler); break; - default: RAPIDJSON_PARSE_ERROR_NORETURN("Expect either an object or array at root", is.Tell()); + case '{': ParseObject(is, handler); break; + case '[': ParseArray(is, handler); break; + default: RAPIDJSON_PARSE_ERROR_NORETURN("Expect either an object or array at root", is.Tell()); } SkipWhitespace(is); @@ -250,6 +250,7 @@ public: RAPIDJSON_PARSE_ERROR_NORETURN("Nothing should follow the root object or array.", is.Tell()); } + stack_.Clear(); return !HasParseError(); } From 3d9dd745a1c2ac887fa1eadcc9cfc1064889a0f8 Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Thu, 26 Jun 2014 23:24:16 +0800 Subject: [PATCH 3/4] Remove setjmp header and jmpbuf member variable --- include/rapidjson/reader.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/rapidjson/reader.h b/include/rapidjson/reader.h index 43e13e4..0806d96 100644 --- a/include/rapidjson/reader.h +++ b/include/rapidjson/reader.h @@ -8,7 +8,6 @@ #include "encodings.h" #include "internal/pow10.h" #include "internal/stack.h" -#include #ifdef RAPIDJSON_SSE42 #include @@ -652,7 +651,6 @@ private: static const size_t kDefaultStackCapacity = 256; //!< Default stack capacity in bytes for storing a single decoded string. internal::Stack stack_; //!< A stack for storing decoded string temporarily during non-destructive parsing. - jmp_buf jmpbuf_; //!< setjmp buffer for fast exit from nested parsing function calls. const char* parseError_; size_t errorOffset_; }; // class GenericReader From a1a8abd0d9fe1410e5b6b08c9b578382b285d99e Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Thu, 26 Jun 2014 23:35:13 +0800 Subject: [PATCH 4/4] Add safe checks in parsing compound types. Compound types (object and array) call ParseString() and ParseValue() for key and values. If there is parse errors inside those calls, it should stop continue parsing. Otherwise, it may be possible to continue parsing and calling handler incorrectly. For example, in ["a\u,","b"], \u generates an error (it should follow but 4 hex digits), the parser continues to treat the first comma as element separator, and treat "," as a JSON string and call the handler. It may be unacceptable in the application code. --- include/rapidjson/reader.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/rapidjson/reader.h b/include/rapidjson/reader.h index 0806d96..4a184c0 100644 --- a/include/rapidjson/reader.h +++ b/include/rapidjson/reader.h @@ -277,6 +277,9 @@ private: RAPIDJSON_PARSE_ERROR("Name of an object member must be a string", is.Tell()); ParseString(is, handler); + if (HasParseError()) + return; + SkipWhitespace(is); if (is.Take() != ':') @@ -285,6 +288,9 @@ private: SkipWhitespace(is); ParseValue(is, handler); + if (HasParseError()) + return; + SkipWhitespace(is); ++memberCount; @@ -313,6 +319,9 @@ private: for (SizeType elementCount = 0;;) { ParseValue(is, handler); + if (HasParseError()) + return; + ++elementCount; SkipWhitespace(is);