Fixing issue 1742 (#1743)

* Fix for issue 1742.

* Some additional tests.
This commit is contained in:
Daniel Lemire
2021-10-27 19:25:23 -04:00
committed by GitHub
parent c0d18452fc
commit 35b4a48e99
5 changed files with 141 additions and 20 deletions
@@ -53,17 +53,6 @@ inline void json_iterator::rewind() noexcept {
SIMDJSON_PUSH_DISABLE_WARNINGS
SIMDJSON_DISABLE_STRICT_OVERFLOW_WARNING
simdjson_warn_unused simdjson_really_inline error_code json_iterator::skip_child(depth_t parent_depth) noexcept {
/***
* WARNING:
* Inside an object, a string value is a depth of +1 compared to the object. Yet a key
* is at the same depth as the object.
* But json_iterator cannot easily tell whether we are pointing at a key or a string value.
* Instead, it assumes that if you are pointing at a string, then it is a value, not a key.
* To be clear...
* the following code assumes that we are *not* pointing at a key. If we are then a bug
* will follow. Unfortunately, it is not possible for the json_iterator its to make this
* check.
*/
if (depth() <= parent_depth) { return SUCCESS; }
switch (*return_current_and_advance()) {
// TODO consider whether matching braces is a requirement: if non-matching braces indicates
@@ -91,19 +80,18 @@ simdjson_warn_unused simdjson_really_inline error_code json_iterator::skip_child
if (at_end()) { return report_error(INCOMPLETE_ARRAY_OR_OBJECT, "Missing [ or { at start"); }
#endif // SIMDJSON_CHECK_EOF
break;
/*case '"':
case '"':
if(*peek() == ':') {
// we are at a key!!! This is
// only possible if someone searched
// for a key in an object and the key
// was not found but our code then
// decided the consume the separating
// comma before returning.
// We are at a key!!!
// This might happen if you just started an object and you skip it immediately.
// Performance note: it would be nice to get rid of this check as it is somewhat
// expensive.
// https://github.com/simdjson/simdjson/issues/1742
logger::log_value(*this, "key");
advance(); // eat up the ':'
return_current_and_advance(); // eat up the ':'
break; // important!!!
}
simdjson_fallthrough;*/
simdjson_fallthrough;
// Anything else must be a scalar value
default:
// For the first scalar, we will have incremented depth already, so we decrement it here.
@@ -114,6 +114,13 @@ simdjson_really_inline simdjson_result<size_t> value::count_elements() & noexcep
iter.move_at_start();
return answer;
}
simdjson_really_inline simdjson_result<size_t> value::count_fields() & noexcept {
simdjson_result<size_t> answer;
auto a = get_object();
answer = a.count_fields();
iter.move_at_start();
return answer;
}
simdjson_really_inline simdjson_result<value> value::at(size_t index) noexcept {
auto a = get_array();
return a.at(index);
@@ -211,6 +218,10 @@ simdjson_really_inline simdjson_result<size_t> simdjson_result<SIMDJSON_IMPLEMEN
if (error()) { return error(); }
return first.count_elements();
}
simdjson_really_inline simdjson_result<size_t> simdjson_result<SIMDJSON_IMPLEMENTATION::ondemand::value>::count_fields() & noexcept {
if (error()) { return error(); }
return first.count_fields();
}
simdjson_really_inline simdjson_result<SIMDJSON_IMPLEMENTATION::ondemand::value> simdjson_result<SIMDJSON_IMPLEMENTATION::ondemand::value>::at(size_t index) noexcept {
if (error()) { return error(); }
return first.at(index);
+16
View File
@@ -244,6 +244,21 @@ public:
* safe to continue.
*/
simdjson_really_inline simdjson_result<size_t> count_elements() & noexcept;
/**
* This method scans the object and counts the number of key-value pairs.
* The count_fields method should always be called before you have begun
* iterating through the object: it is expected that you are pointing at
* the beginning of the object.
* The runtime complexity is linear in the size of the object. After
* calling this function, if successful, the object is 'rewinded' at its
* beginning as if it had never been accessed. If the JSON is malformed (e.g.,
* there is a missing comma), then an error is returned and it is no longer
* safe to continue.
*
* To check that an object is empty, it is more performant to use
* the is_empty() method on the object instance.
*/
simdjson_really_inline simdjson_result<size_t> count_fields() & noexcept;
/**
* Get the value at the given index in the array. This function has linear-time complexity.
* This function should only be called once as the array iterator is not reset between each call.
@@ -557,6 +572,7 @@ public:
simdjson_really_inline operator bool() noexcept(false);
#endif
simdjson_really_inline simdjson_result<size_t> count_elements() & noexcept;
simdjson_really_inline simdjson_result<size_t> count_fields() & noexcept;
simdjson_really_inline simdjson_result<SIMDJSON_IMPLEMENTATION::ondemand::value> at(size_t index) noexcept;
simdjson_really_inline simdjson_result<SIMDJSON_IMPLEMENTATION::ondemand::array_iterator> begin() & noexcept;
simdjson_really_inline simdjson_result<SIMDJSON_IMPLEMENTATION::ondemand::array_iterator> end() & noexcept;
+31
View File
@@ -423,6 +423,36 @@ namespace array_tests {
ASSERT_TRUE(is_empty);
return true;
}
bool issue1742() {
TEST_START();
auto json = R"( {
"code": 0,
"method": "subscribe",
"result": {
"instrument_name": "DAI_USDC",
"subscription": "trade.DAI_USDC",
"channel": "trade",
"data": [
[1,2,3,4]
]
}
} )"_padded;
ondemand::parser parser;
ondemand::document doc;
ASSERT_SUCCESS(parser.iterate(json).get(doc));
simdjson::ondemand::array data;
ASSERT_SUCCESS(doc["result"]["data"].get_array().get(data));
for (auto d : data) {
simdjson::ondemand::array arr;
ASSERT_SUCCESS(d.get_array().get(arr));
size_t count;
ASSERT_SUCCESS(arr.count_elements().get(count));
ASSERT_EQUAL(count, 4);
}
TEST_SUCCEED();
}
bool value_to_array(simdjson::ondemand::value val) {
ondemand::json_type t;
ASSERT_SUCCESS(val.type().get(t));
@@ -780,6 +810,7 @@ namespace array_tests {
bool run() {
return
issue1742() &&
empty_rewind_convoluted() &&
empty_rewind() &&
iterate_empty_array_count() &&
+75
View File
@@ -808,6 +808,79 @@ namespace object_tests {
TEST_SUCCEED();
}
bool issue1742() {
TEST_START();
auto json = R"( {
"code": 0,
"method": "subscribe",
"result": {
"instrument_name": "DAI_USDC",
"subscription": "trade.DAI_USDC",
"channel": "trade",
"data": [
{
"dataTime": 1635315736764,
"d": 1928673075051012900,
"s": "SELL",
"p": 1.0004,
"q": 0.01,
"t": 1635315736763,
"i": "DAI_USDC"
}
]
}
} )"_padded;
ondemand::parser parser;
ondemand::document doc;
ASSERT_SUCCESS(parser.iterate(json).get(doc));
simdjson::ondemand::array data;
ASSERT_SUCCESS(doc["result"]["data"].get_array().get(data));
for (auto d : data) {
simdjson::ondemand::object obj;
ASSERT_SUCCESS(d.get_object().get(obj));
size_t count;
ASSERT_SUCCESS(obj.count_fields().get(count));
ASSERT_EQUAL(count, 7);
}
TEST_SUCCEED();
}
bool issue1742_value() {
TEST_START();
auto json = R"( {
"code": 0,
"method": "subscribe",
"result": {
"instrument_name": "DAI_USDC",
"subscription": "trade.DAI_USDC",
"channel": "trade",
"data": [
{
"dataTime": 1635315736764,
"d": 1928673075051012900,
"s": "SELL",
"p": 1.0004,
"q": 0.01,
"t": 1635315736763,
"i": "DAI_USDC"
}
]
}
} )"_padded;
ondemand::parser parser;
ondemand::document doc;
ASSERT_SUCCESS(parser.iterate(json).get(doc));
simdjson::ondemand::array data;
ASSERT_SUCCESS(doc["result"]["data"].get_array().get(data));
for (auto d : data) {
size_t count;
ASSERT_SUCCESS(d.count_fields().get(count));
ASSERT_EQUAL(count, 7);
}
TEST_SUCCEED();
}
bool iterate_basic_object_count() {
TEST_START();
auto json = R"( {"a":-55, "b":3.23, "c":100000000000000000000, "d":true, "e":null} )"_padded;
@@ -915,6 +988,8 @@ namespace object_tests {
bool run() {
return
issue1742() &&
issue1742_value() &&
issue1723() &&
value_search_unescaped_key() &&
missing_key_continue() &&