From 8c7f46f7d0291fe2d022dc5c2eb77bab213ed8eb Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sun, 25 Feb 2018 17:10:30 +0100 Subject: [PATCH] :hammer: removed a logic error and improved coverage --- include/nlohmann/detail/input/parser.hpp | 30 +++-- include/nlohmann/json.hpp | 3 +- single_include/nlohmann/json.hpp | 33 ++++-- test/src/unit-deserialization.cpp | 143 ++++++++++++++++++----- 4 files changed, 156 insertions(+), 53 deletions(-) diff --git a/include/nlohmann/detail/input/parser.hpp b/include/nlohmann/detail/input/parser.hpp index 009ea994..1cd6868f 100644 --- a/include/nlohmann/detail/input/parser.hpp +++ b/include/nlohmann/detail/input/parser.hpp @@ -594,7 +594,7 @@ class parser get_token(); // closing } -> we are done - if (last_token == token_type::end_object) + if (JSON_UNLIKELY(last_token == token_type::end_object)) { return sax->end_object(); } @@ -603,7 +603,12 @@ class parser while (true) { // parse key - if (last_token != token_type::value_string) + if (JSON_UNLIKELY(last_token != token_type::value_string)) + { + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); + } + else { if (not sax->key(m_lexer.move_string())) { @@ -613,9 +618,10 @@ class parser // parse separator (:) get_token(); - if (last_token != token_type::name_separator) + if (JSON_UNLIKELY(last_token != token_type::name_separator)) { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } // parse value @@ -634,13 +640,14 @@ class parser } // closing } - if (last_token == token_type::end_object) + if (JSON_LIKELY(last_token == token_type::end_object)) { return sax->end_object(); } else { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } } } @@ -679,13 +686,14 @@ class parser } // closing ] - if (last_token == token_type::end_array) + if (JSON_LIKELY(last_token == token_type::end_array)) { return sax->end_array(); } else { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } } } @@ -696,7 +704,8 @@ class parser if (JSON_UNLIKELY(not std::isfinite(res))) { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } else { @@ -736,7 +745,8 @@ class parser default: // the last token was unexpected { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } } } diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index e43d37e7..9515d29e 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -1054,8 +1054,6 @@ class basic_json */ using parse_event_t = typename parser::parse_event_t; - using SAX = typename parser::SAX; - /*! @brief per-element parser callback type @@ -1107,6 +1105,7 @@ class basic_json */ using parser_callback_t = typename parser::parser_callback_t; + using SAX = typename parser::SAX; ////////////////// // constructors // diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 53b03421..79581142 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -3717,7 +3717,7 @@ class parser get_token(); // closing } -> we are done - if (last_token == token_type::end_object) + if (JSON_UNLIKELY(last_token == token_type::end_object)) { return sax->end_object(); } @@ -3726,7 +3726,12 @@ class parser while (true) { // parse key - if (last_token != token_type::value_string) + if (JSON_UNLIKELY(last_token != token_type::value_string)) + { + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); + } + else { if (not sax->key(m_lexer.move_string())) { @@ -3736,9 +3741,10 @@ class parser // parse separator (:) get_token(); - if (last_token != token_type::name_separator) + if (JSON_UNLIKELY(last_token != token_type::name_separator)) { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } // parse value @@ -3757,13 +3763,14 @@ class parser } // closing } - if (last_token == token_type::end_object) + if (JSON_LIKELY(last_token == token_type::end_object)) { return sax->end_object(); } else { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } } } @@ -3802,13 +3809,14 @@ class parser } // closing ] - if (last_token == token_type::end_array) + if (JSON_LIKELY(last_token == token_type::end_array)) { return sax->end_array(); } else { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } } } @@ -3819,7 +3827,8 @@ class parser if (JSON_UNLIKELY(not std::isfinite(res))) { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } else { @@ -3859,7 +3868,8 @@ class parser default: // the last token was unexpected { - return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string()); + return sax->parse_error(m_lexer.get_position(), + m_lexer.get_token_string()); } } } @@ -10875,8 +10885,6 @@ class basic_json */ using parse_event_t = typename parser::parse_event_t; - using SAX = typename parser::SAX; - /*! @brief per-element parser callback type @@ -10928,6 +10936,7 @@ class basic_json */ using parser_callback_t = typename parser::parser_callback_t; + using SAX = typename parser::SAX; ////////////////// // constructors // diff --git a/test/src/unit-deserialization.cpp b/test/src/unit-deserialization.cpp index 64cb243d..9f0bc175 100644 --- a/test/src/unit-deserialization.cpp +++ b/test/src/unit-deserialization.cpp @@ -34,9 +34,8 @@ using nlohmann::json; #include #include -class SaxEventLogger : public nlohmann::json::SAX +struct SaxEventLogger : public nlohmann::json::SAX { - public: bool null() override { events.push_back("null()"); @@ -132,6 +131,47 @@ class SaxEventLogger : public nlohmann::json::SAX std::vector events; }; +struct SaxEventLoggerExitAfterStartObject : public SaxEventLogger +{ + bool start_object(std::size_t elements) override + { + if (elements == -1) + { + events.push_back("start_object()"); + } + else + { + events.push_back("start_object(" + std::to_string(elements) + ")"); + } + return false; + } +}; + +struct SaxEventLoggerExitAfterKey : public SaxEventLogger +{ + bool key(const std::string& val) override + { + events.push_back("key(" + val + ")"); + return false; + } +}; + +struct SaxEventLoggerExitAfterStartArray : public SaxEventLogger +{ + bool start_array(std::size_t elements) override + { + if (elements == -1) + { + events.push_back("start_array()"); + } + else + { + events.push_back("start_array(" + std::to_string(elements) + ")"); + } + return false; + } +}; + TEST_CASE("deserialization") { SECTION("successful deserialization") @@ -148,13 +188,13 @@ TEST_CASE("deserialization") SaxEventLogger l; CHECK(json::sax_parse(ss3, &l)); - CHECK(l.events.size() == 10); + CHECK(l.events.size() == 11); CHECK(l.events == std::vector( { - "start_array()", "string(foo)", - "number_unsigned(1)", "number_unsigned(2)", "number_unsigned(3)", - "boolean(false)", "start_object()", - "number_unsigned(1)", "end_object()", "end_array()" + "start_array()", "string(foo)", "number_unsigned(1)", + "number_unsigned(2)", "number_unsigned(3)", "boolean(false)", + "start_object()", "key(one)", "number_unsigned(1)", + "end_object()", "end_array()" })); } @@ -167,13 +207,13 @@ TEST_CASE("deserialization") SaxEventLogger l; CHECK(json::sax_parse(s, &l)); - CHECK(l.events.size() == 10); + CHECK(l.events.size() == 11); CHECK(l.events == std::vector( { - "start_array()", "string(foo)", - "number_unsigned(1)", "number_unsigned(2)", "number_unsigned(3)", - "boolean(false)", "start_object()", - "number_unsigned(1)", "end_object()", "end_array()" + "start_array()", "string(foo)", "number_unsigned(1)", + "number_unsigned(2)", "number_unsigned(3)", "boolean(false)", + "start_object()", "key(one)", "number_unsigned(1)", + "end_object()", "end_array()" })); } @@ -186,13 +226,13 @@ TEST_CASE("deserialization") SaxEventLogger l; CHECK(json::sax_parse(s, &l)); - CHECK(l.events.size() == 10); + CHECK(l.events.size() == 11); CHECK(l.events == std::vector( { - "start_array()", "string(foo)", - "number_unsigned(1)", "number_unsigned(2)", "number_unsigned(3)", - "boolean(false)", "start_object()", - "number_unsigned(1)", "end_object()", "end_array()" + "start_array()", "string(foo)", "number_unsigned(1)", + "number_unsigned(2)", "number_unsigned(3)", "boolean(false)", + "start_object()", "key(one)", "number_unsigned(1)", + "end_object()", "end_array()" })); } @@ -241,13 +281,13 @@ TEST_CASE("deserialization") SaxEventLogger l; CHECK(not json::sax_parse(ss5, &l)); - CHECK(l.events.size() == 10); + CHECK(l.events.size() == 11); CHECK(l.events == std::vector( { - "start_array()", "string(foo)", - "number_unsigned(1)", "number_unsigned(2)", "number_unsigned(3)", - "boolean(false)", "start_object()", - "number_unsigned(1)", "end_object()", "parse_error(29)" + "start_array()", "string(foo)", "number_unsigned(1)", + "number_unsigned(2)", "number_unsigned(3)", "boolean(false)", + "start_object()", "key(one)", "number_unsigned(1)", + "end_object()", "parse_error(29)" })); } @@ -265,13 +305,13 @@ TEST_CASE("deserialization") SaxEventLogger l; CHECK(not json::sax_parse(s, &l)); - CHECK(l.events.size() == 10); + CHECK(l.events.size() == 11); CHECK(l.events == std::vector( { - "start_array()", "string(foo)", - "number_unsigned(1)", "number_unsigned(2)", "number_unsigned(3)", - "boolean(false)", "start_object()", - "number_unsigned(1)", "end_object()", "parse_error(29)" + "start_array()", "string(foo)", "number_unsigned(1)", + "number_unsigned(2)", "number_unsigned(3)", "boolean(false)", + "start_object()", "key(one)", "number_unsigned(1)", + "end_object()", "parse_error(29)" })); } @@ -746,10 +786,10 @@ TEST_CASE("deserialization") SaxEventLogger l; CHECK(not json::sax_parse(std::begin(v), std::end(v), &l)); - CHECK(l.events.size() == 3); + CHECK(l.events.size() == 4); CHECK(l.events == std::vector( { - "start_object()", "number_unsigned(11)", + "start_object()", "key()", "number_unsigned(11)", "parse_error(7)" })); } @@ -912,4 +952,49 @@ TEST_CASE("deserialization") CHECK(j == 456); } } + + SECTION("SAX and early abort") + { + std::string s = "[1, [\"string\", 43.12], null, {\"key1\": true, \"key2\": false}]"; + + SaxEventLogger default_logger; + SaxEventLoggerExitAfterStartObject exit_after_start_object; + SaxEventLoggerExitAfterKey exit_after_key; + SaxEventLoggerExitAfterStartArray exit_after_start_array; + + json::sax_parse(s, &default_logger); + CHECK(default_logger.events.size() == 14); + CHECK(default_logger.events == std::vector( + { + "start_array()", "number_unsigned(1)", "start_array()", + "string(string)", "number_float(43.12)", "end_array()", "null()", + "start_object()", "key(key1)", "boolean(true)", "key(key2)", + "boolean(false)", "end_object()", "end_array()" + })); + + json::sax_parse(s, &exit_after_start_object); + CHECK(exit_after_start_object.events.size() == 8); + CHECK(exit_after_start_object.events == std::vector( + { + "start_array()", "number_unsigned(1)", "start_array()", + "string(string)", "number_float(43.12)", "end_array()", "null()", + "start_object()" + })); + + json::sax_parse(s, &exit_after_key); + CHECK(exit_after_key.events.size() == 9); + CHECK(exit_after_key.events == std::vector( + { + "start_array()", "number_unsigned(1)", "start_array()", + "string(string)", "number_float(43.12)", "end_array()", "null()", + "start_object()", "key(key1)" + })); + + json::sax_parse(s, &exit_after_start_array); + CHECK(exit_after_start_array.events.size() == 1); + CHECK(exit_after_start_array.events == std::vector( + { + "start_array()" + })); + } }