This is an automated email from the ASF dual-hosted git repository.
Mryange 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 0451443dd09 [fix](function) Enforce input boundaries in IP functions
(#68160)
0451443dd09 is described below
commit 0451443dd09b7af8e815686e1ccc312a767c3b68
Author: Mryange <[email protected]>
AuthorDate: Sun Sep 20 10:30:35 2026 +0800
[fix](function) Enforce input boundaries in IP functions (#68160)
IP functions did not consistently respect the explicit boundaries of
`ColumnString` values. IPv4 and IPv6 text conversions accepted an
otherwise valid address followed by an embedded NUL byte and trailing
data because unbounded parser overloads stopped at the first NUL. In
addition, `is_ipv4_compat` and `is_ipv4_mapped` discarded each binary
string's length and unconditionally read 16 bytes, so short values could
consume bytes from the following row and produce results that changed
with filtering, while oversized values were accepted based only on their
first 16 bytes. Root cause: both paths reduced length-aware string
values to raw pointers before parsing or inspection. This change uses
bounded parsers throughout, removes the unsafe null-terminated parser
overloads, maps parsed IPv4 values directly to IPv6, and requires binary
IPv6 inputs to be exactly 16 bytes before inspecting them. Unit coverage
includes throw, default, nullable, vector, constant, short, valid, and
oversized input paths.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
be/src/exec/common/format_ip.h | 29 -----------
be/src/exprs/function/function_ip.h | 76 +++++++++------------------
be/test/core/column/column_ip_test.cpp | 5 +-
be/test/exprs/function/function_ip_test.cpp | 80 +++++++++++++++++++++++++++++
4 files changed, 107 insertions(+), 83 deletions(-)
diff --git a/be/src/exec/common/format_ip.h b/be/src/exec/common/format_ip.h
index 1863db1aa61..fa309a321bb 100644
--- a/be/src/exec/common/format_ip.h
+++ b/be/src/exec/common/format_ip.h
@@ -179,21 +179,6 @@ inline bool parse_ipv4_whole(const char* src, const char*
end, unsigned char* ds
return parse_ipv4(src, end, dst) == end;
}
-/// returns pointer to the right after parsed sequence or null on failed
parsing
-inline const char* parse_ipv4(const char* src, unsigned char* dst) {
- if (parse_ipv4(
- src, []() { return false; }, dst)) {
- return src;
- }
- return nullptr;
-}
-
-/// returns true if whole null-terminated string was parsed successfully
-inline bool parse_ipv4_whole(const char* src, unsigned char* dst) {
- const char* end = parse_ipv4(src, dst);
- return end != nullptr && *end == '\0';
-}
-
/// integer logarithm, return ceil(log(value, base)) (the smallest integer
greater or equal than log(value, base)
inline constexpr UInt32 int_log(const UInt32 value, const UInt32 base, const
bool carry) {
return value >= base ? 1 + int_log(value / base, base, value % base ||
carry)
@@ -480,18 +465,4 @@ inline bool parse_ipv6_whole(const char* src, const char*
end, unsigned char* ds
return parse_ipv6(src, end, dst) == end;
}
-/// returns pointer to the right after parsed sequence or null on failed
parsing
-inline const char* parse_ipv6(const char* src, unsigned char* dst) {
- if (parse_ipv6(
- src, []() { return false; }, dst))
- return src;
- return nullptr;
-}
-
-/// returns true if whole null-terminated string was parsed successfully
-inline bool parse_ipv6_whole(const char* src, unsigned char* dst) {
- const char* end = parse_ipv6(src, dst);
- return end != nullptr && *end == '\0';
-}
-
} // namespace doris
diff --git a/be/src/exprs/function/function_ip.h
b/be/src/exprs/function/function_ip.h
index f0cf8c86712..359c84037b6 100644
--- a/be/src/exprs/function/function_ip.h
+++ b/be/src/exprs/function/function_ip.h
@@ -43,6 +43,7 @@
#include "core/field.h"
#include "core/types.h"
#include "core/value/ip_address_cidr.h"
+#include "core/value/ipv4_value.h"
#include "exec/common/endian.h"
#include "exec/common/format_ip.h"
#include "exec/common/ipv6_to_binary.h"
@@ -135,8 +136,8 @@ public:
/// Since IPExceptionMode means wider scope, we use more specific name here.
enum class IPConvertExceptionMode : uint8_t { Throw, Default, Null };
-static inline bool try_parse_ipv4(const char* pos, Int64& result_value) {
- return parse_ipv4_whole(pos, reinterpret_cast<unsigned
char*>(&result_value));
+static inline bool try_parse_ipv4(const char* begin, const char* end, Int64&
result_value) {
+ return parse_ipv4_whole(begin, end, reinterpret_cast<unsigned
char*>(&result_value));
}
template <IPConvertExceptionMode exception_mode, typename ToColumn>
@@ -156,10 +157,6 @@ ColumnPtr convert_to_ipv4(ColumnPtr column, const
PaddedPODArray<UInt8>* null_ma
auto col_res = ToColumn::create(column_size, 0);
auto& vec_res = col_res->get_data();
- const ColumnString::Chars& vec_src = column_string->get_chars();
- const ColumnString::Offsets& offsets_src = column_string->get_offsets();
- size_t prev_offset = 0;
-
for (size_t i = 0; i < vec_res.size(); ++i) {
if (null_map && (*null_map)[i]) {
if constexpr (exception_mode == IPConvertExceptionMode::Throw) {
@@ -169,17 +166,13 @@ ColumnPtr convert_to_ipv4(ColumnPtr column, const
PaddedPODArray<UInt8>* null_ma
"like '0.0.0.0' first");
}
vec_res[i] = 0;
- prev_offset = offsets_src[i];
if constexpr (exception_mode == IPConvertExceptionMode::Null) {
(*vec_null_map_to)[i] = true;
}
continue;
}
- const char* src_start = reinterpret_cast<const
char*>(&vec_src[prev_offset]);
- size_t src_length = (i < vec_res.size() - 1) ? (offsets_src[i] -
prev_offset)
- : (vec_src.size() -
prev_offset);
- std::string src(src_start, src_length);
- bool parse_result = try_parse_ipv4(src.c_str(), vec_res[i]);
+ const auto src = column_string->get_data_at(i);
+ bool parse_result = try_parse_ipv4(src.begin(), src.end(), vec_res[i]);
if (!parse_result) {
if constexpr (exception_mode == IPConvertExceptionMode::Throw) {
@@ -191,8 +184,6 @@ ColumnPtr convert_to_ipv4(ColumnPtr column, const
PaddedPODArray<UInt8>* null_ma
vec_res[i] = 0;
}
}
-
- prev_offset = offsets_src[i];
}
if constexpr (exception_mode == IPConvertExceptionMode::Null) {
@@ -384,15 +375,6 @@ ColumnPtr convert_to_ipv6(const StringColumnType&
string_column,
auto col_res = column_create(column_size);
auto& vec_res = get_vector(col_res, column_size);
- using Chars = typename StringColumnType::Chars;
- const Chars& vec_src = string_column.get_chars();
-
- size_t src_offset = 0;
-
- /// ColumnString contains not null terminated strings. But functions
parseIPv6, parseIPv4 expect null terminated string.
- /// TODO fix this - now parseIPv6/parseIPv4 accept end iterator, so can be
parsed in-place
- std::string string_buffer;
-
int offset_inc = 1;
ColumnString* column_string = nullptr;
if constexpr (std::is_same_v<ToColumn, ColumnString>) {
@@ -401,20 +383,11 @@ ColumnPtr convert_to_ipv6(const StringColumnType&
string_column,
}
for (size_t out_offset = 0, i = 0; i < column_size; out_offset +=
offset_inc, ++i) {
- char src_ipv4_buf[sizeof("::ffff:") + IPV4_MAX_TEXT_LENGTH + 1] =
"::ffff:";
- size_t src_next_offset = src_offset;
-
- const char* src_value = nullptr;
+ const auto src = string_column.get_data_at(i);
+ const char* src_value = src.begin();
+ const char* src_end = src.end();
auto* res_value = reinterpret_cast<unsigned
char*>(&vec_res[out_offset]);
- if constexpr (std::is_same_v<StringColumnType, ColumnString>) {
- src_value = reinterpret_cast<const char*>(&vec_src[src_offset]);
- src_next_offset = string_column.get_offsets()[i];
-
- string_buffer.assign(src_value, src_next_offset - src_offset);
- src_value = string_buffer.c_str();
- }
-
if (null_map && (*null_map)[i]) {
if (exception_mode == IPConvertExceptionMode::Throw) {
throw Exception(
@@ -431,7 +404,6 @@ ColumnPtr convert_to_ipv6(const StringColumnType&
string_column,
DCHECK(column_string != nullptr);
column_string->get_offsets().push_back((i + 1) *
IPV6_BINARY_LENGTH);
}
- src_offset = src_next_offset;
continue;
}
@@ -442,13 +414,13 @@ ColumnPtr convert_to_ipv6(const StringColumnType&
string_column,
/// If the source IP address is parsable as an IPv4 address, then
transform it into a valid IPv6 address.
/// Keeping it simple by just prefixing `::ffff:` to the IPv4 address
to represent it as a valid IPv6 address.
- size_t string_length = src_next_offset - src_offset;
+ size_t string_length = src.size;
if (string_length != 0) {
- if (try_parse_ipv4(src_value, dummy_result)) {
- strncat(src_ipv4_buf, src_value, sizeof(src_ipv4_buf) -
strlen(src_ipv4_buf) - 1);
- parse_result = parse_ipv6_whole(src_ipv4_buf, res_value);
+ if (try_parse_ipv4(src_value, src_end, dummy_result)) {
+ map_ipv4_to_ipv6(static_cast<IPv4>(dummy_result), res_value);
+ parse_result = true;
} else {
- parse_result = parse_ipv6_whole(src_value, res_value);
+ parse_result = parse_ipv6_whole(src_value, src_end, res_value);
}
}
@@ -479,7 +451,6 @@ ColumnPtr convert_to_ipv6(const StringColumnType&
string_column,
(*vec_null_map_to)[i] = true;
}
}
- src_offset = src_next_offset;
}
if constexpr (exception_mode == IPConvertExceptionMode::Null) {
@@ -1018,8 +989,8 @@ public:
auto& col_res_data = col_res->get_data();
for (size_t i = 0; i < col_size; ++i) {
- auto ipv4_in = col_in->get_data_at(i);
- if (is_ipv4_compat(reinterpret_cast<const UInt8*>(ipv4_in.data))) {
+ const auto address = col_in->get_data_at(i);
+ if (is_ipv4_compat(address)) {
col_res_data[i] = 1;
}
}
@@ -1029,9 +1000,10 @@ public:
}
private:
- static bool is_ipv4_compat(const UInt8* address) {
- return (LittleEndian::Load64(address) == 0) &&
(LittleEndian::Load32(address + 8) == 0) &&
- (LittleEndian::Load32(address + 12) != 0);
+ static bool is_ipv4_compat(const StringRef& address) {
+ return address.size == IPV6_BINARY_LENGTH &&
(LittleEndian::Load64(address.data) == 0) &&
+ (LittleEndian::Load32(address.data + 8) == 0) &&
+ (LittleEndian::Load32(address.data + 12) != 0);
}
};
@@ -1058,8 +1030,8 @@ public:
auto& col_res_data = col_res->get_data();
for (size_t i = 0; i < col_size; ++i) {
- auto ipv4_in = col_in->get_data_at(i);
- if (is_ipv4_mapped(reinterpret_cast<const UInt8*>(ipv4_in.data))) {
+ const auto address = col_in->get_data_at(i);
+ if (is_ipv4_mapped(address)) {
col_res_data[i] = 1;
}
}
@@ -1069,9 +1041,9 @@ public:
}
private:
- static bool is_ipv4_mapped(const UInt8* address) {
- return (LittleEndian::Load64(address) == 0) &&
- ((LittleEndian::Load64(address + 8) & 0x00000000FFFFFFFFULL) ==
+ static bool is_ipv4_mapped(const StringRef& address) {
+ return address.size == IPV6_BINARY_LENGTH &&
(LittleEndian::Load64(address.data) == 0) &&
+ ((LittleEndian::Load64(address.data + 8) &
0x00000000FFFFFFFFULL) ==
0x00000000FFFF0000ULL);
}
};
diff --git a/be/test/core/column/column_ip_test.cpp
b/be/test/core/column/column_ip_test.cpp
index 871a0a2641a..82706a8fbb0 100644
--- a/be/test/core/column/column_ip_test.cpp
+++ b/be/test/core/column/column_ip_test.cpp
@@ -303,8 +303,9 @@ TEST_F(ColumnIPTest, IPv4ValueFromStringTest) {
TEST_F(ColumnIPTest, IPv4Parse) {
std::string ipv4_str = "127.0.0.1";
Int64 result_value = 0;
- ASSERT_EQ(try_parse_ipv4(ipv4_str.data(), result_value), true);
+ ASSERT_EQ(try_parse_ipv4(ipv4_str.data(), ipv4_str.data() +
ipv4_str.size(), result_value),
+ true);
ASSERT_EQ(2130706433, result_value);
};
-} // namespace doris
\ No newline at end of file
+} // namespace doris
diff --git a/be/test/exprs/function/function_ip_test.cpp
b/be/test/exprs/function/function_ip_test.cpp
index bfb367c9da8..a3f1f431de0 100644
--- a/be/test/exprs/function/function_ip_test.cpp
+++ b/be/test/exprs/function/function_ip_test.cpp
@@ -39,6 +39,86 @@ TEST(FunctionIpTest, IPAddressVariantTypeTest) {
EXPECT_TRUE(ipv6_zero.is_v6());
}
+TEST(FunctionIpTest, StringToNumRejectsEmbeddedNullTail) {
+ std::string invalid_ipv4 = "192.168.0.1";
+ invalid_ipv4.push_back('\0');
+ invalid_ipv4.append("tail");
+
+ std::string invalid_ipv6 = "2001:db8::1";
+ invalid_ipv6.push_back('\0');
+ invalid_ipv6.append("tail");
+
+ const DataSet ipv4_error_data = {{{invalid_ipv4}, int64_t {0}}};
+ const DataSet ipv4_default_data = {{{invalid_ipv4}, int64_t {0}}};
+ const DataSet ipv4_null_data = {{{invalid_ipv4}, Null()}};
+ const DataSet ipv6_error_data = {{{invalid_ipv6}, std::string {}}};
+ const DataSet ipv6_default_data = {{{invalid_ipv6},
std::string(IPV6_BINARY_LENGTH, '\0')},
+ {{invalid_ipv4},
std::string(IPV6_BINARY_LENGTH, '\0')}};
+ const DataSet ipv6_null_data = {{{invalid_ipv6}, Null()}, {{invalid_ipv4},
Null()}};
+
+ for (const auto& input_types : {InputTypeSet {PrimitiveType::TYPE_VARCHAR},
+ InputTypeSet {Consted
{PrimitiveType::TYPE_VARCHAR}}}) {
+ auto status = check_function<DataTypeInt64>("ipv4_string_to_num",
input_types,
+ ipv4_error_data, -1, -1,
true);
+ EXPECT_EQ(status.code(), ErrorCode::INVALID_ARGUMENT);
+
+ status = check_function<DataTypeString>("ipv6_string_to_num",
input_types, ipv6_error_data,
+ -1, -1, true);
+ EXPECT_EQ(status.code(), ErrorCode::INVALID_ARGUMENT);
+ }
+
+ const InputTypeSet input_types = {PrimitiveType::TYPE_VARCHAR};
+
check_function_all_arg_comb<DataTypeInt64>("ipv4_string_to_num_or_default",
input_types,
+ ipv4_default_data);
+ check_function_all_arg_comb<DataTypeInt64,
true>("ipv4_string_to_num_or_null", input_types,
+ ipv4_null_data);
+ check_function_all_arg_comb<DataTypeInt64, true>("inet_aton", input_types,
ipv4_null_data);
+
check_function_all_arg_comb<DataTypeString>("ipv6_string_to_num_or_default",
input_types,
+ ipv6_default_data);
+ check_function_all_arg_comb<DataTypeString,
true>("ipv6_string_to_num_or_null", input_types,
+ ipv6_null_data);
+ check_function_all_arg_comb<DataTypeString, true>("inet6_aton",
input_types, ipv6_null_data);
+}
+
+TEST(FunctionIpTest, StringToIPv6AcceptsLongIPv4Spellings) {
+ std::string mapped_ipv4_zero(IPV6_BINARY_LENGTH, '\0');
+ mapped_ipv4_zero[10] = static_cast<char>(0xff);
+ mapped_ipv4_zero[11] = static_cast<char>(0xff);
+
+ for (const auto& [input, input_type] :
+ {std::pair {std::string("0000.0000.0000.0"), InputTypeSet
{PrimitiveType::TYPE_VARCHAR}},
+ std::pair {std::string("0000.0000.0000.0000"),
+ InputTypeSet {Consted {PrimitiveType::TYPE_VARCHAR}}}}) {
+ const DataSet data = {{{input}, mapped_ipv4_zero}};
+ static_cast<void>(check_function<DataTypeString>("ipv6_string_to_num",
input_type, data));
+ }
+}
+
+TEST(FunctionIpTest, IPv4CompatAndMappedRequireIPv6BinaryLength) {
+ const std::string ipv4_address = {static_cast<char>(0xc0),
static_cast<char>(0xa8), '\0',
+ static_cast<char>(0x01)};
+
+ const std::string compat_prefix(IPV6_BINARY_LENGTH - IPV4_BINARY_LENGTH,
'\0');
+ const std::string compat_address = compat_prefix + ipv4_address;
+ const DataSet compat_data = {{{compat_prefix}, uint8_t {0}},
+ {{ipv4_address}, uint8_t {0}},
+ {{compat_address}, uint8_t {1}},
+ {{compat_address + '\0'}, uint8_t {0}}};
+
+ const std::string mapped_marker = {static_cast<char>(0xff),
static_cast<char>(0xff)};
+ const std::string mapped_prefix(IPV6_BINARY_LENGTH - IPV4_BINARY_LENGTH -
mapped_marker.size(),
+ '\0');
+ const std::string mapped_address = mapped_prefix + mapped_marker +
ipv4_address;
+ const DataSet mapped_data = {{{mapped_prefix}, uint8_t {0}},
+ {{mapped_marker}, uint8_t {0}},
+ {{mapped_address}, uint8_t {1}},
+ {{mapped_address + '\0'}, uint8_t {0}}};
+
+ const InputTypeSet input_types = {PrimitiveType::TYPE_VARCHAR};
+ check_function_all_arg_comb<DataTypeUInt8, true>("is_ipv4_compat",
input_types, compat_data);
+ check_function_all_arg_comb<DataTypeUInt8, true>("is_ipv4_mapped",
input_types, mapped_data);
+}
+
TEST(FunctionIpTest, FunctionIsIPAddressInRangeTest) {
std::string func_name = "is_ip_address_in_range";
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
