From aa52cf686850b268bcf16fdeb13765aa0a4e33eb Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Sat, 21 Aug 2021 10:22:59 -0400 Subject: [PATCH] Alive fix. (#1700) --- include/simdjson/generic/ondemand/array.h | 17 ++-- .../simdjson/generic/ondemand/document-inl.h | 4 +- include/simdjson/generic/ondemand/document.h | 5 ++ .../simdjson/generic/ondemand/json_iterator.h | 2 +- .../generic/ondemand/value_iterator-inl.h | 2 + tests/ondemand/ondemand_array_error_tests.cpp | 11 +++ tests/ondemand/ondemand_misc_tests.cpp | 86 +++++++++++++++++++ 7 files changed, 116 insertions(+), 11 deletions(-) diff --git a/include/simdjson/generic/ondemand/array.h b/include/simdjson/generic/ondemand/array.h index 131df5a39..db129b076 100644 --- a/include/simdjson/generic/ondemand/array.h +++ b/include/simdjson/generic/ondemand/array.h @@ -79,6 +79,14 @@ public: */ simdjson_really_inline simdjson_result raw_json() noexcept; + /** + * Get the value at the given index. This function has linear-time complexity. + * This function should only be called once as the array iterator is not reset between each call. + * + * @return The value at the given index, or: + * - INDEX_OUT_OF_BOUNDS if the array index is larger than an array length + */ + simdjson_really_inline simdjson_result at(size_t index) noexcept; protected: /** * Go to the end of the array, no matter where you are right now. @@ -121,15 +129,6 @@ protected: */ simdjson_really_inline array(const value_iterator &iter) noexcept; - /** - * Get the value at the given index. This function has linear-time complexity. - * This function should only be called once as the array iterator is not reset between each call. - * - * @return The value at the given index, or: - * - INDEX_OUT_OF_BOUNDS if the array index is larger than an array length - */ - simdjson_really_inline simdjson_result at(size_t index) noexcept; - /** * Iterator marking current position. * diff --git a/include/simdjson/generic/ondemand/document-inl.h b/include/simdjson/generic/ondemand/document-inl.h index e8bbec0df..1cb17066d 100644 --- a/include/simdjson/generic/ondemand/document-inl.h +++ b/include/simdjson/generic/ondemand/document-inl.h @@ -19,7 +19,9 @@ inline void document::rewind() noexcept { inline std::string document::to_debug_string() noexcept { return iter.to_string(); } - +inline bool document::is_alive() noexcept { + return iter.is_alive(); +} simdjson_really_inline value_iterator document::resume_value_iterator() noexcept { return value_iterator(&iter, 1, iter.root_position()); } diff --git a/include/simdjson/generic/ondemand/document.h b/include/simdjson/generic/ondemand/document.h index e01cc5b56..355c8831d 100644 --- a/include/simdjson/generic/ondemand/document.h +++ b/include/simdjson/generic/ondemand/document.h @@ -376,6 +376,11 @@ public: * Returns debugging information. */ inline std::string to_debug_string() noexcept; + /** + * Some unrecoverable error conditions may render the document instance unusable. + * The is_alive() method returns true when the document is still suitable. + */ + inline bool is_alive() noexcept; /** * Get the value associated with the given JSON pointer. We use the RFC 6901 diff --git a/include/simdjson/generic/ondemand/json_iterator.h b/include/simdjson/generic/ondemand/json_iterator.h index 6d7bfcdf9..d18ab48ce 100644 --- a/include/simdjson/generic/ondemand/json_iterator.h +++ b/include/simdjson/generic/ondemand/json_iterator.h @@ -216,7 +216,7 @@ public: simdjson_really_inline uint8_t *&string_buf_loc() noexcept; /** - * Report an error, preventing further iteration. + * Report an unrecoverable error, preventing further iteration. * * @param error The error to report. Must not be SUCCESS, UNINITIALIZED, INCORRECT_TYPE, or NO_SUCH_FIELD. * @param message An error message to report with the error. diff --git a/include/simdjson/generic/ondemand/value_iterator-inl.h b/include/simdjson/generic/ondemand/value_iterator-inl.h index ff23741e1..62a1a1127 100644 --- a/include/simdjson/generic/ondemand/value_iterator-inl.h +++ b/include/simdjson/generic/ondemand/value_iterator-inl.h @@ -40,6 +40,7 @@ simdjson_warn_unused simdjson_really_inline simdjson_result value_iterator // Note that adding a check for 'streaming' is not expensive since we only have at most // one root element. if (! _json_iter->streaming() && (*_json_iter->peek_last() != '}')) { + _json_iter->abandon(); return report_error(INCOMPLETE_ARRAY_OR_OBJECT, "missing } at end"); } return started_object(); @@ -408,6 +409,7 @@ simdjson_warn_unused simdjson_really_inline simdjson_result value_iterator // Note that adding a check for 'streaming' is not expensive since we only have at most // one root element. if ( ! _json_iter->streaming() && (*_json_iter->peek_last() != ']')) { + _json_iter->abandon(); return report_error(INCOMPLETE_ARRAY_OR_OBJECT, "missing ] at end"); } return started_array(); diff --git a/tests/ondemand/ondemand_array_error_tests.cpp b/tests/ondemand/ondemand_array_error_tests.cpp index 2625d30d9..7d82c51f0 100644 --- a/tests/ondemand/ondemand_array_error_tests.cpp +++ b/tests/ondemand/ondemand_array_error_tests.cpp @@ -8,8 +8,14 @@ namespace array_error_tests { template bool assert_iterate(T array, V *expected, size_t N, simdjson::error_code *expected_error, size_t N2) { + /** + * We use printouts because the assert_iterate is abstract and hard to + * understand intuitively. + */ + std::cout << " --- assert_iterate "; size_t count = 0; for (auto elem : std::forward(array)) { + std::cout << "-"; std::cout.flush(); V actual; auto actual_error = elem.get(actual); if (count >= N) { @@ -17,14 +23,19 @@ namespace array_error_tests { std::cerr << "FAIL: Extra error reported: " << actual_error << std::endl; return false; } + std::cout << "[ expect: " << expected_error[count - N] << " ]"; std::cout.flush(); ASSERT_ERROR(actual_error, expected_error[count - N]); } else { + std::cout << "[ expect: SUCCESS ]"; std::cout.flush(); ASSERT_SUCCESS(actual_error); + std::cout << "{ expect value : "<< expected[count] << " }"; std::cout.flush(); + ASSERT_EQUAL(actual, expected[count]); } count++; } ASSERT_EQUAL(count, N+N2); + std::cout << std::endl; return true; } diff --git a/tests/ondemand/ondemand_misc_tests.cpp b/tests/ondemand/ondemand_misc_tests.cpp index 3a66b73ae..b29d26c02 100644 --- a/tests/ondemand/ondemand_misc_tests.cpp +++ b/tests/ondemand/ondemand_misc_tests.cpp @@ -266,7 +266,89 @@ namespace misc_tests { } TEST_SUCCEED(); } + bool is_alive_root_array() { + TEST_START(); + ondemand::parser parser; + padded_string docdata = R"([1,2,3)"_padded; + ondemand::document doc; + ASSERT_SUCCESS(parser.iterate(docdata).get(doc)); + ASSERT_TRUE(doc.is_alive()); + size_t count{0}; + for(simdjson_unused simdjson_result val : doc) { + ASSERT_ERROR(val.error(), INCOMPLETE_ARRAY_OR_OBJECT); + count++; + } + ASSERT_EQUAL(count, 1); + ASSERT_FALSE(doc.is_alive()); + TEST_SUCCEED(); + } + bool is_alive_array() { + TEST_START(); + ondemand::parser parser; + padded_string docdata = R"({"a":[1,2,3})"_padded; + ondemand::document doc; + ASSERT_SUCCESS(parser.iterate(docdata).get(doc)); + ASSERT_TRUE(doc.is_alive()); + ondemand::object obj; + ASSERT_SUCCESS(doc.get_object().get(obj)); + ondemand::array internal_arr; + ASSERT_SUCCESS(obj["a"].get_array().get(internal_arr)); + size_t count{0}; + for(simdjson_unused simdjson_result val : internal_arr) { + if(count < 3) { + ASSERT_SUCCESS(val.error()); + ASSERT_TRUE(doc.is_alive()); + } else { + ASSERT_ERROR(val.error(), TAPE_ERROR); + ASSERT_FALSE(doc.is_alive()); + } + count++; + } + ASSERT_EQUAL(count, 4); + ASSERT_FALSE(doc.is_alive()); + TEST_SUCCEED(); + } + + bool is_alive_root_object() { + TEST_START(); + ondemand::parser parser; + padded_string docdata = R"({"a":1)"_padded; + ondemand::document doc; + ASSERT_SUCCESS(parser.iterate(docdata).get(doc)); + ASSERT_TRUE(doc.is_alive()); + ondemand::object obj; + ASSERT_ERROR(doc.get_object().get(obj), INCOMPLETE_ARRAY_OR_OBJECT); + ASSERT_FALSE(doc.is_alive()); + TEST_SUCCEED(); + } + + bool is_alive_object() { + TEST_START(); + ondemand::parser parser; + padded_string docdata = R"([{"a":1])"_padded; + ondemand::document doc; + ASSERT_SUCCESS(parser.iterate(docdata).get(doc)); + ASSERT_TRUE(doc.is_alive()); + ondemand::object obj; + ASSERT_SUCCESS(doc.at(0).get_object().get(obj)); + ASSERT_TRUE(doc.is_alive()); + size_t count{0}; + for(simdjson_unused simdjson_result val : obj) { + if(count == 0) { + ASSERT_SUCCESS(val.error()); + ASSERT_TRUE(doc.is_alive()); + } + if(count == 1) { + ASSERT_ERROR(val.error(), TAPE_ERROR); + ASSERT_FALSE(doc.is_alive()); + } + count++; + } + ASSERT_EQUAL(count, 2); + ASSERT_FALSE(doc.is_alive()); + TEST_SUCCEED(); + } bool issue1661() { TEST_START(); ondemand::parser parser; @@ -365,6 +447,10 @@ namespace misc_tests { bool run() { return + is_alive_root_array() && + is_alive_root_object() && + is_alive_array() && + is_alive_object() && test_get_value() && issue1660_with_uint64() && issue1660_with_int64() &&