This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 82e3954c39a [fix](be) Reject malformed JSON tokens during casts
(#68448)
82e3954c39a is described below
commit 82e3954c39a4d4ee85f75e464f0e6f32cba12c9a
Author: Jerry Hu <[email protected]>
AuthorDate: Mon Sep 28 09:40:54 2026 +0800
[fix](be) Reject malformed JSON tokens during casts (#68448)
### What problem does this PR solve?
Issue Number: None
Problem Summary:
CAST from strings to JSON accepted incomplete `null` literals and
malformed number tokens because the parser trusted simdjson token
classification and the large-number fallback without validating the
complete raw token. As a result, inputs such as `nul`, `[nul]`, `01`,
and `1.` could be normalized and persisted as valid JSON values.
This change validates `null` tokens, applies the JSON number grammar
before accepting large-number fallback errors, and verifies that the
parser consumed the complete document. It preserves the existing
fallback behavior for syntactically valid numbers outside the binary64
or int128 range, such as `9.6E400`, while continuing to support valid
integers larger than 64 bits and surrounding JSON whitespace.
### Release note
Fix CAST from strings to JSON to reject malformed JSON tokens instead of
normalizing them.
### Check List (For Author)
- Test
- [x] Regression test
- `test_json_cast_validation`
- `test_json_type_cast`
- [x] Unit Test
- `./run-be-ut.sh --run --filter='JsonbParserTest.*'` (60 tests passed,
ASAN_UT)
- [x] Manual test
- `./build.sh --be --fe` (ASAN)
- Verified `1e9999` and a 400-digit integer remain accepted as JSON
- `build-support/clang-format.sh`
- `build-support/check-format.sh`
- `build-support/check-build-hygiene.sh`
- Behavior changed:
- [x] Yes. Malformed JSON text now returns SQL NULL in non-strict casts
and raises an error in strict casts instead of being normalized and
persisted; syntactically valid oversized numbers retain their existing
fallback behavior.
- Does this need documentation?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
---
be/src/util/jsonb_parser_simd.h | 230 ++++++++++++++-------
be/test/util/jsonb_parser_simd_test.cpp | 35 +++-
.../datatype_p0/json/test_json_cast_validation.out | 23 +++
.../json/test_json_cast_validation.groovy | 80 +++++++
4 files changed, 297 insertions(+), 71 deletions(-)
diff --git a/be/src/util/jsonb_parser_simd.h b/be/src/util/jsonb_parser_simd.h
index 43f12f024ed..48e2a0cd1af 100644
--- a/be/src/util/jsonb_parser_simd.h
+++ b/be/src/util/jsonb_parser_simd.h
@@ -59,8 +59,8 @@
#pragma once
#include <simdjson.h>
-#include <cmath>
#include <limits>
+#include <string_view>
#include "common/status.h"
#include "util/jsonb_document.h"
@@ -70,17 +70,85 @@
namespace doris {
using int128_t = __int128;
struct JsonbParser {
+private:
+ static bool is_json_whitespace(char c) {
+ return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+ }
+
+ static std::string_view trim_json_whitespace(std::string_view token) {
+ while (!token.empty() && is_json_whitespace(token.front())) {
+ token.remove_prefix(1);
+ }
+ while (!token.empty() && is_json_whitespace(token.back())) {
+ token.remove_suffix(1);
+ }
+ return token;
+ }
+
+ static bool is_valid_json_number(std::string_view token) {
+ size_t pos = 0;
+ if (pos < token.size() && token[pos] == '-') {
+ ++pos;
+ }
+ if (pos == token.size()) {
+ return false;
+ }
+
+ if (token[pos] == '0') {
+ ++pos;
+ } else if (token[pos] >= '1' && token[pos] <= '9') {
+ do {
+ ++pos;
+ } while (pos < token.size() && token[pos] >= '0' && token[pos] <=
'9');
+ } else {
+ return false;
+ }
+
+ if (pos < token.size() && token[pos] == '.') {
+ ++pos;
+ const size_t fraction_start = pos;
+ while (pos < token.size() && token[pos] >= '0' && token[pos] <=
'9') {
+ ++pos;
+ }
+ if (pos == fraction_start) {
+ return false;
+ }
+ }
+
+ if (pos < token.size() && (token[pos] == 'e' || token[pos] == 'E')) {
+ ++pos;
+ if (pos < token.size() && (token[pos] == '+' || token[pos] ==
'-')) {
+ ++pos;
+ }
+ const size_t exponent_start = pos;
+ while (pos < token.size() && token[pos] >= '0' && token[pos] <=
'9') {
+ ++pos;
+ }
+ if (pos == exponent_start) {
+ return false;
+ }
+ }
+
+ return pos == token.size();
+ }
+
// According to https://github.com/simdjson/simdjson/pull/2139
// For numbers larger than 64 bits, we can obtain the raw_json_token and
parse it ourselves.
// This allows handling numbers larger than 64 bits, such as int128.
// For example, try to parse a 18446744073709551616, this number is just 1
greater than the maximum value of uint64_t, and simdjson will return a
NUMBER_ERROR
// If try to parse a 18446744073709551616231231, it is obviously a large
integer, at this time simdjson will return a BIGINT_ERROR
- static bool parse_number_success(simdjson::error_code error_code) {
- return error_code == simdjson::error_code::SUCCESS ||
- error_code == simdjson::error_code::NUMBER_ERROR ||
- error_code == simdjson::error_code::BIGINT_ERROR;
+ static bool parse_number_success(simdjson::error_code error_code,
std::string_view raw_token) {
+ if (error_code == simdjson::error_code::SUCCESS) {
+ return true;
+ }
+ if (error_code != simdjson::error_code::NUMBER_ERROR &&
+ error_code != simdjson::error_code::BIGINT_ERROR) {
+ return false;
+ }
+ return is_valid_json_number(raw_token);
}
+public:
// parse a UTF-8 JSON string with length
// will reset writer before parse
static Status parse(const char* pch, size_t len, JsonbWriter& writer) {
@@ -92,6 +160,7 @@ struct JsonbParser {
simdjson::ondemand::parser simdjson_parser;
simdjson::padded_string json_str {pch, len};
simdjson::ondemand::document doc =
simdjson_parser.iterate(json_str);
+ bool root_number_validated_from_raw_token = false;
// simdjson process top level primitive types specially
// so some repeated code here
@@ -102,9 +171,7 @@ struct JsonbParser {
break;
}
case simdjson::ondemand::json_type::null: {
- if (writer.writeNull() == 0) {
- return Status::InvalidArgument("writeNull failed");
- }
+ RETURN_IF_ERROR(write_null(doc, writer));
break;
}
case simdjson::ondemand::json_type::boolean: {
@@ -120,16 +187,26 @@ struct JsonbParser {
case simdjson::ondemand::json_type::number: {
simdjson::ondemand::number num;
simdjson::error_code res = doc.get_number().get(num);
- if (!parse_number_success(res)) {
+ const auto raw_token =
trim_json_whitespace(doc.raw_json_token());
+ if (!parse_number_success(res, raw_token)) {
return Status::InvalidArgument(fmt::format("simdjson
get_number failed: {}",
simdjson::error_message(res)));
}
// simdjson get_number() returns a number object, which can be
- RETURN_IF_ERROR(
- write_number(num, doc.get_number_type(),
doc.raw_json_token(), writer));
+ RETURN_IF_ERROR(write_number(num, doc.get_number_type(),
raw_token, writer));
+ if (res != simdjson::error_code::SUCCESS) {
+ const auto document =
trim_json_whitespace(std::string_view {pch, len});
+ if (raw_token != document) {
+ return Status::InvalidArgument("JSON document was not
fully consumed");
+ }
+ root_number_validated_from_raw_token = true;
+ }
break;
}
}
+ if (!root_number_validated_from_raw_token && !doc.at_end()) {
+ return Status::InvalidArgument("JSON document was not fully
consumed");
+ }
} catch (simdjson::simdjson_error& e) {
return Status::InvalidArgument(fmt::format("simdjson parse
exception: {}", e.what()));
}
@@ -137,15 +214,34 @@ struct JsonbParser {
}
private:
+ template <typename JsonValue>
+ static Status write_null(JsonValue& value, JsonbWriter& writer) {
+ if (!value.is_null()) {
+ return Status::InvalidArgument("Invalid null literal");
+ }
+ if (writer.writeNull() == 0) {
+ return Status::InvalidArgument("writeNull failed");
+ }
+ return Status::OK();
+ }
+
+ static Status parse_number(simdjson::ondemand::value& value, JsonbWriter&
writer) {
+ simdjson::ondemand::number num;
+ const auto result = value.get_number().get(num);
+ const auto raw_token = trim_json_whitespace(value.raw_json_token());
+ if (!parse_number_success(result, raw_token)) {
+ return Status::InvalidArgument(
+ fmt::format("simdjson get_number failed: {}",
simdjson::error_message(result)));
+ }
+ return write_number(num, value.get_number_type(), raw_token, writer);
+ }
+
// parse json, recursively if necessary, by simdjson
// and serialize to binary format by writer
static Status parse(simdjson::ondemand::value value, JsonbWriter& writer) {
switch (value.type()) {
case simdjson::ondemand::json_type::null: {
- if (writer.writeNull() == 0) {
- return Status::InvalidArgument("writeNull failed");
- }
- break;
+ return write_null(value, writer);
}
case simdjson::ondemand::json_type::boolean: {
if (writer.writeBool(value.get_bool()) == 0) {
@@ -158,16 +254,7 @@ private:
break;
}
case simdjson::ondemand::json_type::number: {
- simdjson::ondemand::number num;
- auto res = value.get_number().get(num);
- if (!parse_number_success(res)) {
- return Status::InvalidArgument(fmt::format("simdjson
get_number failed: {}",
-
simdjson::error_message(res)));
- }
-
- RETURN_IF_ERROR(
- write_number(num, value.get_number_type(),
value.raw_json_token(), writer));
- break;
+ return parse_number(value, writer);
}
case simdjson::ondemand::json_type::object: {
if (!writer.writeStartObject()) {
@@ -196,7 +283,6 @@ private:
if (!writer.writeEndObject()) {
return Status::InvalidArgument("writeEndObject failed");
- break;
}
break;
@@ -224,6 +310,50 @@ private:
return Status::OK();
}
+ static Status write_floating_number(double number, std::string_view
raw_string,
+ JsonbWriter& writer) {
+ // When a double exceeds the precision that can be represented by a
double type in
+ // simdjson, it gets converted to 0. The correct approach is to
truncate the value instead.
+ if (number == 0) {
+ StringParser::ParseResult result;
+ number = StringParser::string_to_float<double>(raw_string.data(),
raw_string.size(),
+ &result);
+ if (result != StringParser::PARSE_SUCCESS) {
+ return Status::InvalidArgument("invalid number, raw string is:
" +
+ std::string(raw_string));
+ }
+ }
+ if (writer.writeDouble(number) == 0) {
+ return Status::InvalidArgument("writeDouble failed");
+ }
+ return Status::OK();
+ }
+
+ static Status write_big_integer(std::string_view raw_string, JsonbWriter&
writer) {
+ StringParser::ParseResult result;
+ auto value = StringParser::string_to_int<int128_t>(raw_string.data(),
raw_string.size(),
+ &result);
+ if (result == StringParser::PARSE_SUCCESS) {
+ if (!writer.writeInt128(value)) {
+ return Status::InvalidArgument("writeInt128 failed");
+ }
+ return Status::OK();
+ }
+
+ // JSON text can represent integers beyond int128. Preserve the
existing fallback to
+ // double, even though the conversion may lose precision.
+ double double_value =
StringParser::string_to_float<double>(raw_string.data(),
+
raw_string.size(), &result);
+ if (result != StringParser::PARSE_SUCCESS) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_string));
+ }
+ if (!writer.writeDouble(double_value)) {
+ return Status::InvalidArgument("writeDouble failed");
+ }
+ return Status::OK();
+ }
+
static Status write_string(std::string_view str, JsonbWriter& writer) {
// start writing string
if (!writer.writeStartString()) {
@@ -259,24 +389,7 @@ private:
switch (num_type) {
case simdjson::ondemand::number_type::floating_point_number: {
- double number = num.get_double();
- // When a double exceeds the precision that can be represented by
a double type in simdjson, it gets converted to 0.
- // The correct approach, should be to truncate the double value
instead.
- if (number == 0) {
- StringParser::ParseResult result;
- number =
StringParser::string_to_float<double>(raw_string.data(), raw_string.size(),
- &result);
- if (result != StringParser::PARSE_SUCCESS) {
- return Status::InvalidArgument("invalid number, raw string
is: " +
- std::string(raw_string));
- }
- }
-
- if (writer.writeDouble(number) == 0) {
- return Status::InvalidArgument("writeDouble failed");
- }
-
- break;
+ return write_floating_number(num.get_double(), raw_string, writer);
}
case simdjson::ondemand::number_type::signed_integer:
case simdjson::ondemand::number_type::unsigned_integer: {
@@ -301,36 +414,13 @@ private:
if (!success) {
return Status::InvalidArgument("writeInt failed");
}
- break;
+ return Status::OK();
}
case simdjson::ondemand::number_type::big_integer: {
- StringParser::ParseResult result;
- auto val =
StringParser::string_to_int<int128_t>(raw_string.data(), raw_string.size(),
- &result);
- if (result != StringParser::PARSE_SUCCESS) {
- // If the string exceeds the range of int128_t, it will
attempt to convert it to double.
- // This may result in loss of precision, but for JSON,
exchanging data as plain text between different systems may inherently cause
precision loss.
- // try parse as double
- double double_val = StringParser::string_to_float<double>(
- raw_string.data(), raw_string.size(), &result);
- if (result != StringParser::PARSE_SUCCESS) {
- // if both parse failed, return error
- return Status::InvalidArgument("invalid number, raw string
is: " +
- std::string(raw_string));
- }
- if (!writer.writeDouble(double_val)) {
- return Status::InvalidArgument("writeDouble failed");
- }
- } else {
- // as int128_t
- if (!writer.writeInt128(val)) {
- return Status::InvalidArgument("writeInt128 failed");
- }
- }
- break;
+ return write_big_integer(raw_string, writer);
}
}
- return Status::OK();
+ return Status::InvalidArgument("unknown number type");
}
};
} // namespace doris
diff --git a/be/test/util/jsonb_parser_simd_test.cpp
b/be/test/util/jsonb_parser_simd_test.cpp
index 8f5b418a193..8ccbaf46519 100644
--- a/be/test/util/jsonb_parser_simd_test.cpp
+++ b/be/test/util/jsonb_parser_simd_test.cpp
@@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.
+#include <string_view>
+
#include "common/status.h"
#include "core/value/jsonb_value.h"
#include "gtest/gtest.h"
@@ -221,6 +223,10 @@ TEST_F(JsonbParserTest, ParseJsonWithLongInt2) {
std::string_view json_with_long_int = R"(19389892839283982938923)";
std::string_view expected_json_with_long_int =
R"(19389892839283982938923)";
EXPECT_EQ(parse_json_and_check(json_with_long_int,
expected_json_with_long_int), Status::OK());
+
+ std::string_view json_with_whitespace = " \n19389892839283982938923\t";
+ EXPECT_EQ(parse_json_and_check(json_with_whitespace,
expected_json_with_long_int),
+ Status::OK());
}
TEST_F(JsonbParserTest, ParseJsonWithLongInt3) {
@@ -255,6 +261,33 @@ TEST_F(JsonbParserTest, ParseJsonWithInvalidNumberFormat) {
EXPECT_FALSE(parse_json_and_check(json_with_invalid_number,
json_with_invalid_number));
}
+TEST_F(JsonbParserTest, RejectInvalidTokens) {
+ constexpr std::string_view invalid_json[] = {"nul", "[nul]",
R"({"x":nul})", "01", "-01",
+ "1.", "[01]", "[-01]",
"[1.]", "1e+",
+ "{}x", "[]x", "{} {}",
"[] []", "nullx"};
+
+ for (const auto json : invalid_json) {
+ JsonBinaryValue jsonb_val;
+ EXPECT_FALSE(jsonb_val.from_json_string(json.data(),
json.size()).ok()) << json;
+ }
+}
+
+TEST_F(JsonbParserTest, PreserveValidNumbersBeyondDoubleRange) {
+ constexpr std::string_view oversized_floating_point_json[] = {"9.6E400",
"1e9999", "[9.6E400]",
+
R"({"value":1e9999})"};
+
+ for (const auto json : oversized_floating_point_json) {
+ JsonBinaryValue jsonb_val;
+ const auto status = jsonb_val.from_json_string(json.data(),
json.size());
+ EXPECT_TRUE(status.ok()) << status;
+ }
+
+ const std::string oversized_integer(400, '9');
+ JsonBinaryValue jsonb_val;
+ const auto status = jsonb_val.from_json_string(oversized_integer);
+ EXPECT_TRUE(status.ok()) << status;
+}
+
TEST_F(JsonbParserTest, ParseJsonWithInvalidBoolean) {
std::string_view json_with_invalid_boolean = R"({"invalid_bool": True})";
EXPECT_FALSE(parse_json_and_check(json_with_invalid_boolean,
json_with_invalid_boolean));
@@ -437,4 +470,4 @@ TEST_F(JsonbParserTest, ParseJsonWithEscapedNulInKey) {
EXPECT_EQ(parse_json_and_check(json_with_nul, expected_json_with_nul),
Status::OK());
}
-} // namespace doris
\ No newline at end of file
+} // namespace doris
diff --git
a/regression-test/data/datatype_p0/json/test_json_cast_validation.out
b/regression-test/data/datatype_p0/json/test_json_cast_validation.out
new file mode 100644
index 00000000000..b5d2a614a38
--- /dev/null
+++ b/regression-test/data/datatype_p0/json/test_json_cast_validation.out
@@ -0,0 +1,23 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !non_strict --
+1 null false
+2 \N true
+3 \N true
+4 \N true
+5 \N true
+6 \N true
+7 \N true
+8 18446744073709551616 false
+
+-- !persisted --
+1 null false
+2 \N true
+3 \N true
+4 \N true
+5 \N true
+6 \N true
+7 \N true
+8 18446744073709551616 false
+
+-- !strict_insert_atomic --
+0
diff --git
a/regression-test/suites/datatype_p0/json/test_json_cast_validation.groovy
b/regression-test/suites/datatype_p0/json/test_json_cast_validation.groovy
new file mode 100644
index 00000000000..a2031804315
--- /dev/null
+++ b/regression-test/suites/datatype_p0/json/test_json_cast_validation.groovy
@@ -0,0 +1,80 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_json_cast_validation", "p0") {
+ sql "drop table if exists test_json_cast_validation_src"
+ sql """
+ create table test_json_cast_validation_src (
+ id int,
+ s varchar(30)
+ ) duplicate key(id)
+ distributed by hash(id) buckets 1
+ properties("replication_num" = "1")
+ """
+ sql """
+ insert into test_json_cast_validation_src values
+ (1, 'null'),
+ (2, 'nul'),
+ (3, '[nul]'),
+ (4, '{\"x\":nul}'),
+ (5, '01'),
+ (6, '-01'),
+ (7, '1.'),
+ (8, '18446744073709551616')
+ """
+
+ sql "set enable_strict_cast = false"
+ order_qt_non_strict """
+ select id, cast(s as json), cast(s as json) is null
+ from test_json_cast_validation_src
+ order by id
+ """
+
+ sql "drop table if exists test_json_cast_validation_sink"
+ sql """
+ create table test_json_cast_validation_sink (
+ id int,
+ j json
+ ) duplicate key(id)
+ distributed by hash(id) buckets 1
+ properties("replication_num" = "1")
+ """
+ sql """
+ insert into test_json_cast_validation_sink
+ select id, cast(s as json) from test_json_cast_validation_src
+ """
+ order_qt_persisted """
+ select id, j, j is null
+ from test_json_cast_validation_sink
+ order by id
+ """
+
+ sql "truncate table test_json_cast_validation_sink"
+ sql "set enable_strict_cast = true"
+ test {
+ sql "select cast(s as json) from test_json_cast_validation_src where
id = 2"
+ exception "Failed to parse json string"
+ }
+ test {
+ sql """
+ insert into test_json_cast_validation_sink
+ select id, cast(s as json) from test_json_cast_validation_src
+ """
+ exception "Failed to parse json string"
+ }
+ qt_strict_insert_atomic "select count(*) from
test_json_cast_validation_sink"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
