From 48152a03266b7953856bed28d9f6f38eeb96cd5c Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Sat, 8 Apr 2023 16:11:40 -0400 Subject: [PATCH] Fix for issue 1981 (#1982) * Fix * Rewording. * Doing it differently. --- cmake/developer-options.cmake | 2 ++ .../generic/ondemand/json_iterator-inl.h | 15 +++++------ .../simdjson/generic/ondemand/json_iterator.h | 7 +++++- .../generic/ondemand/value_iterator-inl.h | 20 +++++++-------- tests/ondemand/ondemand_misc_tests.cpp | 25 +++++++++++++++++++ tests/test_macros.h | 1 + 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/cmake/developer-options.cmake b/cmake/developer-options.cmake index d4cbd4f43..b678d352c 100644 --- a/cmake/developer-options.cmake +++ b/cmake/developer-options.cmake @@ -90,6 +90,8 @@ if(NOT is_multi_config AND NOT CMAKE_BUILD_TYPE) if(SIMDJSON_SANITIZE OR SIMDJSON_SANITIZE_UNDEFINED) message(STATUS "No build type selected and you have enabled the sanitizer, \ default to Debug. Consider setting CMAKE_BUILD_TYPE.") + message(STATUS "Setting debug optimization flag to -O1 to help sanitizer.") + set(CMAKE_CXX_FLAGS_DEBUG "-O1" CACHE STRING "" FORCE) set(CMAKE_BUILD_TYPE Debug CACHE STRING "Choose the type of build." FORCE) else() message(STATUS "No build type selected, default to Release") diff --git a/include/simdjson/generic/ondemand/json_iterator-inl.h b/include/simdjson/generic/ondemand/json_iterator-inl.h index cfa3ecbb7..5111ffffa 100644 --- a/include/simdjson/generic/ondemand/json_iterator-inl.h +++ b/include/simdjson/generic/ondemand/json_iterator-inl.h @@ -358,19 +358,16 @@ simdjson_inline error_code json_iterator::optional_error(error_code _error, cons return _error; } -template -simdjson_warn_unused simdjson_inline bool json_iterator::copy_to_buffer(const uint8_t *json, uint32_t max_len, uint8_t (&tmpbuf)[N]) noexcept { + +simdjson_warn_unused simdjson_inline bool json_iterator::copy_to_buffer(const uint8_t *json, uint32_t max_len, uint8_t *tmpbuf, size_t N) noexcept { + // This function is not expected to be called in performance-sensitive settings. // Let us guard against silly cases: if((N < max_len) || (N == 0)) { return false; } - // Truncate whitespace to fit the buffer. - if (max_len > N-1) { - // if (jsoncharutils::is_not_structural_or_whitespace(json[N-1])) { return false; } - max_len = N-1; - } - // Copy to the buffer. std::memcpy(tmpbuf, json, max_len); - tmpbuf[max_len] = ' '; + if(N > max_len) { // We pad whatever remains with ' '. + std::memset(tmpbuf + max_len, ' ', N - max_len); + } return true; } diff --git a/include/simdjson/generic/ondemand/json_iterator.h b/include/simdjson/generic/ondemand/json_iterator.h index 17d2a093d..94ba59bd9 100644 --- a/include/simdjson/generic/ondemand/json_iterator.h +++ b/include/simdjson/generic/ondemand/json_iterator.h @@ -238,7 +238,12 @@ public: */ simdjson_inline error_code optional_error(error_code error, const char *message) noexcept; - template simdjson_warn_unused simdjson_inline bool copy_to_buffer(const uint8_t *json, uint32_t max_len, uint8_t (&tmpbuf)[N]) noexcept; + /** + * Take an input in json containing max_len characters and attempt to copy it over to tmpbuf, a buffer with + * N bytes of capacity. It will return false if N is too small (smaller than max_len) of if it is zero. + * The buffer (tmpbuf) is padded with space characters. + */ + simdjson_warn_unused simdjson_inline bool copy_to_buffer(const uint8_t *json, uint32_t max_len, uint8_t *tmpbuf, size_t N) noexcept; simdjson_inline token_position position() const noexcept; /** diff --git a/include/simdjson/generic/ondemand/value_iterator-inl.h b/include/simdjson/generic/ondemand/value_iterator-inl.h index b2d251b06..9c9dfa50a 100644 --- a/include/simdjson/generic/ondemand/value_iterator-inl.h +++ b/include/simdjson/generic/ondemand/value_iterator-inl.h @@ -573,7 +573,7 @@ simdjson_inline simdjson_result value_iterator::is_root_integer(bool check auto max_len = peek_start_length(); auto json = peek_root_scalar("is_root_integer"); uint8_t tmpbuf[20+1]; // <20 digits> is the longest possible unsigned integer - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 20+1)) { return false; // if there are more than 20 characters, it cannot be represented as an integer. } auto answer = numberparsing::is_integer(tmpbuf); @@ -591,7 +591,7 @@ simdjson_inline simdjson_result // 1074 is the maximum number of significant fractional digits. Add 8 more digits for the biggest // number: -0.e-308. uint8_t tmpbuf[1074+8+1]; - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 1074+8+1)) { logger::log_error(*_json_iter, start_position(), depth(), "Root number more than 1082 characters"); return NUMBER_ERROR; } @@ -606,7 +606,7 @@ simdjson_inline simdjson_result value_iterator::get_root_number(bool che // 1074 is the maximum number of significant fractional digits. Add 8 more digits for the biggest // number: -0.e-308. uint8_t tmpbuf[1074+8+1]; - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 1074+8+1)) { logger::log_error(*_json_iter, start_position(), depth(), "Root number more than 1082 characters"); return NUMBER_ERROR; } @@ -634,7 +634,7 @@ simdjson_warn_unused simdjson_inline simdjson_result value_iterator::g auto max_len = peek_start_length(); auto json = peek_root_scalar("uint64"); uint8_t tmpbuf[20+1]; // <20 digits> is the longest possible unsigned integer - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 20+1)) { logger::log_error(*_json_iter, start_position(), depth(), "Root number more than 20 characters"); return NUMBER_ERROR; } @@ -649,7 +649,7 @@ simdjson_warn_unused simdjson_inline simdjson_result value_iterator::g auto max_len = peek_start_length(); auto json = peek_root_scalar("uint64"); uint8_t tmpbuf[20+1]; // <20 digits> is the longest possible unsigned integer - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 20+1)) { logger::log_error(*_json_iter, start_position(), depth(), "Root number more than 20 characters"); return NUMBER_ERROR; } @@ -664,7 +664,7 @@ simdjson_warn_unused simdjson_inline simdjson_result value_iterator::ge auto max_len = peek_start_length(); auto json = peek_root_scalar("int64"); uint8_t tmpbuf[20+1]; // -<19 digits> is the longest possible integer - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 20+1)) { logger::log_error(*_json_iter, start_position(), depth(), "Root number more than 20 characters"); return NUMBER_ERROR; } @@ -680,7 +680,7 @@ simdjson_warn_unused simdjson_inline simdjson_result value_iterator::ge auto max_len = peek_start_length(); auto json = peek_root_scalar("int64"); uint8_t tmpbuf[20+1]; // -<19 digits> is the longest possible integer - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 20+1)) { logger::log_error(*_json_iter, start_position(), depth(), "Root number more than 20 characters"); return NUMBER_ERROR; } @@ -699,7 +699,7 @@ simdjson_warn_unused simdjson_inline simdjson_result value_iterator::get // 1074 is the maximum number of significant fractional digits. Add 8 more digits for the biggest // number: -0.e-308. uint8_t tmpbuf[1074+8+1]; - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 1074+8+1)) { logger::log_error(*_json_iter, start_position(), depth(), "Root number more than 1082 characters"); return NUMBER_ERROR; } @@ -718,7 +718,7 @@ simdjson_warn_unused simdjson_inline simdjson_result value_iterator::get // 1074 is the maximum number of significant fractional digits. Add 8 more digits for the biggest // number: -0.e-308. uint8_t tmpbuf[1074+8+1]; - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 1074+8+1)) { logger::log_error(*_json_iter, start_position(), depth(), "Root number more than 1082 characters"); return NUMBER_ERROR; } @@ -733,7 +733,7 @@ simdjson_warn_unused simdjson_inline simdjson_result value_iterator::get_r auto max_len = peek_start_length(); auto json = peek_root_scalar("bool"); uint8_t tmpbuf[5+1]; - if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { return incorrect_type_error("Not a boolean"); } + if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf, 5+1)) { return incorrect_type_error("Not a boolean"); } auto result = parse_bool(tmpbuf); if(result.error() == SUCCESS) { if (check_trailing && !_json_iter->is_single_token()) { return TRAILING_CONTENT; } diff --git a/tests/ondemand/ondemand_misc_tests.cpp b/tests/ondemand/ondemand_misc_tests.cpp index 58ae099c9..5ed838508 100644 --- a/tests/ondemand/ondemand_misc_tests.cpp +++ b/tests/ondemand/ondemand_misc_tests.cpp @@ -5,6 +5,29 @@ using namespace simdjson; namespace misc_tests { using namespace std; + bool issue1981_success() { + auto error_phrase = R"(false)"_padded; + TEST_START(); + ondemand::parser parser; + ondemand::document doc; + ASSERT_SUCCESS(parser.iterate(error_phrase).get(doc)); + bool b; + ASSERT_SUCCESS( doc.get_bool().get(b)); + ASSERT_FALSE(b); + TEST_SUCCEED(); + } + + bool issue1981_failure() { + auto error_phrase = R"(falseA)"_padded; + TEST_START(); + ondemand::parser parser; + ondemand::document doc; + ASSERT_SUCCESS(parser.iterate(error_phrase).get(doc)); + bool b; + ASSERT_ERROR( doc.get_bool().get(b), INCORRECT_TYPE); + TEST_SUCCEED(); + } + bool replacement_char() { auto fun_phrase = R"( ["I \u2665 Unicode. Even broken \ud800 Unicode." ])"_padded; std::string_view expected_fun = "I \xe2\x99\xa5 Unicode. Even broken \xef\xbf\xbd Unicode."; @@ -570,6 +593,8 @@ namespace misc_tests { bool run() { return + issue1981_success() && + issue1981_failure() && replacement_char() && wobbly_tests() && issue_uffff() && diff --git a/tests/test_macros.h b/tests/test_macros.h index 7806ee9d5..7df60dc0b 100644 --- a/tests/test_macros.h +++ b/tests/test_macros.h @@ -114,6 +114,7 @@ simdjson_inline bool assert_iterate_error(T &arr, simdjson::error_code expected, #define ASSERT_EQUAL(ACTUAL, EXPECTED) do { if (!::assert_equal ((ACTUAL), (EXPECTED), #ACTUAL)) { return false; } } while (0); #define ASSERT_RESULT(ACTUAL, EXPECTED) do { if (!::assert_result ((ACTUAL), (EXPECTED), #ACTUAL)) { return false; } } while (0); #define ASSERT_SUCCESS(ACTUAL) do { if (!::assert_success((ACTUAL), #ACTUAL)) { return false; } } while (0); +#define ASSERT_FAILURE(ACTUAL) do { if (::assert_success((ACTUAL), #ACTUAL)) { return false; } } while (0); #define ASSERT_ERROR(ACTUAL, EXPECTED) do { if (!::assert_error ((ACTUAL), (EXPECTED), #ACTUAL)) { return false; } } while (0); #define ASSERT_TRUE(ACTUAL) do { if (!::assert_true ((ACTUAL), #ACTUAL)) { return false; } } while (0); #define ASSERT_FALSE(ACTUAL) do { if (!::assert_false ((ACTUAL), #ACTUAL)) { return false; } } while (0);