From 657489e387ca3f687be4bb159a9f27eeced7afa3 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Fri, 23 Jul 2021 22:27:46 -0400 Subject: [PATCH] More fixes. --- .../generic/ondemand/raw_json_string-inl.h | 6 ++++-- .../generic/ondemand/raw_json_string.h | 9 ++++++--- .../generic/ondemand/value_iterator-inl.h | 18 ++++++++++++------ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/include/simdjson/generic/ondemand/raw_json_string-inl.h b/include/simdjson/generic/ondemand/raw_json_string-inl.h index ff12a7420..73045a738 100644 --- a/include/simdjson/generic/ondemand/raw_json_string-inl.h +++ b/include/simdjson/generic/ondemand/raw_json_string-inl.h @@ -52,9 +52,11 @@ simdjson_really_inline bool raw_json_string::is_free_from_unescaped_quote(const } -simdjson_really_inline bool raw_json_string::unsafe_is_equal(size_t length, std::string_view target) const noexcept { +simdjson_really_inline bool raw_json_string::unsafe_is_equal(size_t max_key_length_including_final_quote, std::string_view target) const noexcept { // If we are going to call memcmp, then we must know something about the length of the raw_json_string. - return (length >= target.size()) && (raw()[target.size()] == '"') && !memcmp(raw(), target.data(), target.size()); + if(max_key_length_including_final_quote <= target.size()) { return false; } + // It is now safe to read in [0, target.size()]. + return (raw()[target.size()] == '"') && (memcmp(raw(), target.data(), target.size()) == 0); } simdjson_really_inline bool raw_json_string::unsafe_is_equal(std::string_view target) const noexcept { diff --git a/include/simdjson/generic/ondemand/raw_json_string.h b/include/simdjson/generic/ondemand/raw_json_string.h index 53df5d413..6c5307e82 100644 --- a/include/simdjson/generic/ondemand/raw_json_string.h +++ b/include/simdjson/generic/ondemand/raw_json_string.h @@ -53,8 +53,11 @@ public: * This compares the current instance to the std::string_view target: returns true if * they are byte-by-byte equal (no escaping is done) on target.size() characters, * and if the raw_json_string instance has a quote character at byte index target.size(). - * We never read more than length + 1 bytes in the raw_json_string instance. - * If length is smaller than target.size(), this will return false. + * We never read more than max_key_length_including_final_quote bytes in the raw_json_string instance. + * If max_key_length_including_final_quote is smaller than target.size() + 1, this will return false. + * + * max_key_length_including_final_quote is the maximal key length in bytes, not including + * the leading quote. * * The std::string_view instance may contain any characters. However, the caller * is responsible for setting length so that length bytes may be read in the @@ -63,7 +66,7 @@ public: * Performance: the comparison may be done using memcmp which may be efficient * for long strings. */ - simdjson_really_inline bool unsafe_is_equal(size_t length, std::string_view target) const noexcept; + simdjson_really_inline bool unsafe_is_equal(size_t max_key_length_including_final_quote, std::string_view target) const noexcept; /** * This compares the current instance to the std::string_view target: returns true if diff --git a/include/simdjson/generic/ondemand/value_iterator-inl.h b/include/simdjson/generic/ondemand/value_iterator-inl.h index 621090562..c0777f9ee 100644 --- a/include/simdjson/generic/ondemand/value_iterator-inl.h +++ b/include/simdjson/generic/ondemand/value_iterator-inl.h @@ -135,7 +135,9 @@ simdjson_warn_unused simdjson_really_inline simdjson_result value_iterator while (has_value) { // Get the key and colon, stopping at the value. raw_json_string actual_key; - size_t max_key_length = _json_iter->peek_length() - 2; // -2 for the two quotes + // We know, for sure, that _json_iter->peek_length() is at least '1', but it could + // be just one, so subtracting by more than 1 is unsafe. + size_t max_key_length_including_final_quote = _json_iter->peek_length() - 1; // -1 for one quote // field_key() advances the pointer and checks that '"' is found (corresponding to a key). // The depth is left unchanged by field_key(). if ((error = field_key().get(actual_key) )) { abandon(); return error; }; @@ -143,7 +145,7 @@ simdjson_warn_unused simdjson_really_inline simdjson_result value_iterator // key and the value. It will also increment the depth by one. if ((error = field_value() )) { abandon(); return error; } // If it matches, stop and return - if (actual_key.unsafe_is_equal(max_key_length, key)) { + if (actual_key.unsafe_is_equal(max_key_length_including_final_quote, key)) { logger::log_event(*this, "match", key, -2); // If we return here, then we return while pointing at the ':' that we just checked. return true; @@ -258,7 +260,9 @@ simdjson_warn_unused simdjson_really_inline simdjson_result value_iterator // Get the key and colon, stopping at the value. raw_json_string actual_key; - size_t max_key_length = _json_iter->peek_length() - 2; // -2 for the two quotes + // We know, for sure, that _json_iter->peek_length() is at least '1', but it could + // be just one, so subtracting by more than 1 is unsafe. + size_t max_key_length_including_final_quote = _json_iter->peek_length() - 1; // -1 for one quote // field_key() advances the pointer and checks that '"' is found (corresponding to a key). // The depth is left unchanged by field_key(). if ((error = field_key().get(actual_key) )) { abandon(); return error; }; @@ -267,7 +271,7 @@ simdjson_warn_unused simdjson_really_inline simdjson_result value_iterator if ((error = field_value() )) { abandon(); return error; } // If it matches, stop and return - if (actual_key.unsafe_is_equal(max_key_length, key)) { + if (actual_key.unsafe_is_equal(max_key_length_including_final_quote, key)) { logger::log_event(*this, "match", key, -2); // If we return here, then we return while pointing at the ':' that we just checked. return true; @@ -300,7 +304,9 @@ simdjson_warn_unused simdjson_really_inline simdjson_result value_iterator // Get the key and colon, stopping at the value. raw_json_string actual_key; - size_t max_key_length = _json_iter->peek_length() - 2; // -2 for the two quotes + // We know, for sure, that _json_iter->peek_length() is at least '1', but it could + // be just one, so subtracting by more than 1 is unsafe. + size_t max_key_length_including_final_quote = _json_iter->peek_length() - 1; // -1 for one quote // field_key() advances the pointer and checks that '"' is found (corresponding to a key). // The depth is left unchanged by field_key(). error = field_key().get(actual_key); SIMDJSON_ASSUME(!error); @@ -309,7 +315,7 @@ simdjson_warn_unused simdjson_really_inline simdjson_result value_iterator error = field_value(); SIMDJSON_ASSUME(!error); // If it matches, stop and return - if (actual_key.unsafe_is_equal(max_key_length, key)) { + if (actual_key.unsafe_is_equal(max_key_length_including_final_quote, key)) { logger::log_event(*this, "match", key, -2); // If we return here, then we return while pointing at the ':' that we just checked. return true;