Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions be/src/util/jsonb_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,11 @@ class JsonbToJson {
break;
}
case JsonbType::T_String: {
// getBlobLen() is the stored payload length. length() would drop a
// trailing NUL that belongs to the value, so it cannot be used here;
// the writer stores strings by their exact length and never pads.
string_to_json(val->unpack<JsonbStringVal>()->getBlob(),
val->unpack<JsonbStringVal>()->length());
val->unpack<JsonbStringVal>()->getBlobLen());
break;
}
case JsonbType::T_Binary: {
Expand Down Expand Up @@ -162,7 +165,9 @@ class JsonbToJson {
return;
}
char char_buffer[16];
for (const char* ptr = str; ptr != str + len && *ptr; ++ptr) {
// A JSON string may legally contain U+0000, so the loop must be bounded by
// the length only; the NUL itself is escaped as \u0000 by the default branch.
for (const char* ptr = str; ptr != str + len; ++ptr) {

@github-actions github-actions Bot Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve the stored length for a trailing U+0000

This loop is length-bounded now, but intern_json() still calls it with JsonbStringVal::length() (lines 110-111). That helper returns strnlen(payload, size) whenever the last stored byte is zero. As a result, a legal JSON string whose decoded payload is the two bytes a,0 reaches this loop with len == 1 and still renders as just a; a value whose decoded payload is only the zero byte becomes empty. All new tests put b after the NUL, so they miss this branch. Please use the authoritative payload length here (for example, getBlobLen() in the T_String case, while handling any typed padding at its type-aware producer) and add trailing/only-NUL coverage.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 67d0df8.

intern_json() now passes getBlobLen() for the T_String case, so the stored
payload length reaches the length-bounded loop unchanged. JsonbWriter writes a
string with writeString(str, len) and patches the size field with the exact
number of bytes written, so it never emits NUL padding and getBlobLen() is the
authoritative length here; the direct string path in vjson_each.cpp already
relies on the same helper.

New coverage for the branch you pointed at:

  • JsonbParserTest.ParseJsonWithTrailingNulInString ({"key":"a\u0000"})
  • JsonbParserTest.ParseJsonWithOnlyNulInString ({"key":"\u0000"})
  • JsonbParserTest.ParseJsonWithTrailingNulInNestedArray ({"key":["a\u0000","\u0000"]})
  • regression json_each_text_trailing_nul and explode_json_array_string_trailing_nul

The regression output regenerated on a local cluster now shows
["a\u0000","\u0000"] (length 20) and ["a\u0000"] (length 11) where the old
code returned ["a",""] and ["a"].

Note that JsonbStringVal::length() has the same trimming behaviour for its
other callers (the string equality check in jsonb_document.h and two spots in
function_jsonb.cpp). Those are separate semantics from JSON textualization, so
I left them out of this fix rather than widening the diff.

if ((unsigned char)*ptr > 31 && *ptr != '\"' && *ptr != '\\') {
os_.put(*ptr);
} else {
Expand Down
37 changes: 37 additions & 0 deletions be/test/util/jsonb_parser_simd_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,4 +400,41 @@ TEST_F(JsonbParserTest, ParseJsonWithToLongKey) {
EXPECT_FALSE(st.ok());
std::cout << st.msg() << std::endl;
}

TEST_F(JsonbParserTest, ParseJsonWithEscapedNulInString) {
std::string_view json_with_nul = R"({"key":"a\u0000b"})";
std::string_view expected_json_with_nul = R"({"key":"a\u0000b"})";
EXPECT_EQ(parse_json_and_check(json_with_nul, expected_json_with_nul), Status::OK());
}

TEST_F(JsonbParserTest, ParseJsonWithEscapedNulInNestedArray) {
std::string_view json_with_nul = R"({"key":["a\u0000b","c"]})";
std::string_view expected_json_with_nul = R"({"key":["a\u0000b","c"]})";
EXPECT_EQ(parse_json_and_check(json_with_nul, expected_json_with_nul), Status::OK());
}

TEST_F(JsonbParserTest, ParseJsonWithTrailingNulInString) {
std::string_view json_with_nul = R"({"key":"a\u0000"})";
std::string_view expected_json_with_nul = R"({"key":"a\u0000"})";
EXPECT_EQ(parse_json_and_check(json_with_nul, expected_json_with_nul), Status::OK());
}

TEST_F(JsonbParserTest, ParseJsonWithOnlyNulInString) {
std::string_view json_with_nul = R"({"key":"\u0000"})";
std::string_view expected_json_with_nul = R"({"key":"\u0000"})";
EXPECT_EQ(parse_json_and_check(json_with_nul, expected_json_with_nul), Status::OK());
}

TEST_F(JsonbParserTest, ParseJsonWithTrailingNulInNestedArray) {
std::string_view json_with_nul = R"({"key":["a\u0000","\u0000"]})";
std::string_view expected_json_with_nul = R"({"key":["a\u0000","\u0000"]})";
EXPECT_EQ(parse_json_and_check(json_with_nul, expected_json_with_nul), Status::OK());
}

TEST_F(JsonbParserTest, ParseJsonWithEscapedNulInKey) {
std::string_view json_with_nul = R"({"a\u0000b":1})";
std::string_view expected_json_with_nul = R"({"a\u0000b":1})";
EXPECT_EQ(parse_json_and_check(json_with_nul, expected_json_with_nul), Status::OK());
}

} // namespace doris
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !json_each_text_nested_array --
x 12 ["a\\u0000b"]

-- !json_each_text_nested_object --
x 16 {"y":"a\\u0000b"}

-- !json_each_text_trailing_nul --
x 20 ["a\\u0000","\\u0000"]

-- !explode_json_array_string_direct --
3 610062

-- !explode_json_array_string_nested --
12 ["a\\u0000b"]

-- !explode_json_array_string_trailing_nul --
11 ["a\\u0000"]

Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// 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_text_embedded_nul") {
// unhex('5C') is a backslash, so every JSON literal below really contains
// \u0000, the legal escape for U+0000 inside a JSON string. Rendering such
// a nested value back to text must not stop at the NUL.

qt_json_each_text_nested_array '''
SELECT k, length(v), v
FROM (SELECT 1) d
LATERAL VIEW json_each_text(concat('{"x":["a', unhex('5C'), 'u0000b"]}')) t AS k, v
'''

qt_json_each_text_nested_object '''
SELECT k, length(v), v
FROM (SELECT 1) d
LATERAL VIEW json_each_text(concat('{"x":{"y":"a', unhex('5C'), 'u0000b"}}')) t AS k, v
'''

// A NUL at the very end of a string used to be trimmed by the stored-length
// helper, so keep a dedicated case for it.
qt_json_each_text_trailing_nul '''
SELECT k, length(v), v
FROM (SELECT 1) d
LATERAL VIEW json_each_text(concat('{"x":["a', unhex('5C'), 'u0000","', unhex('5C'), 'u0000"]}')) t AS k, v
'''

// A direct string element is copied by length, so it keeps the raw NUL byte;
// compare it through hex() to keep the expected output printable.
qt_explode_json_array_string_direct '''
SELECT length(c), hex(c)
FROM (SELECT 1) d
LATERAL VIEW explode_json_array_string(concat('["a', unhex('5C'), 'u0000b"]')) t AS c
'''

qt_explode_json_array_string_nested '''
SELECT length(c), c
FROM (SELECT 1) d
LATERAL VIEW explode_json_array_string(concat('[["a', unhex('5C'), 'u0000b"]]')) t AS c
'''

qt_explode_json_array_string_trailing_nul '''
SELECT length(c), c
FROM (SELECT 1) d
LATERAL VIEW explode_json_array_string(concat('[["a', unhex('5C'), 'u0000"]]')) t AS c
'''
}
Loading