From af3bd197c3b40c5a9f2094a0d822370b65d5a41b Mon Sep 17 00:00:00 2001 From: Wang Xiaofeng Date: Sun, 25 Oct 2026 23:31:41 +0800 Subject: [PATCH] Respect splitter field bounds in numeric conversions --- src/butil/string_splitter.h | 4 +-- src/butil/string_splitter_inl.h | 60 ++++++++++++++++++------------- test/string_splitter_unittest.cpp | 40 +++++++++++++++++++++ 3 files changed, 78 insertions(+), 26 deletions(-) diff --git a/src/butil/string_splitter.h b/src/butil/string_splitter.h index 0651e68dce..0fc8bb2722 100644 --- a/src/butil/string_splitter.h +++ b/src/butil/string_splitter.h @@ -88,7 +88,7 @@ class StringSplitter { // Cast field to specific type, and write the value into `pv'. // Returns 0 on success, -1 otherwise. - // NOTE: If separator is a digit, casting functions always return -1. + // Only the bytes in the current field are converted. inline int to_int8(int8_t *pv) const; inline int to_uint8(uint8_t *pv) const; inline int to_int(int *pv) const; @@ -143,7 +143,7 @@ class StringMultiSplitter { // Cast field to specific type, and write the value into `pv'. // Returns 0 on success, -1 otherwise. - // NOTE: If separators contains digit, casting functions always return -1. + // Only the bytes in the current field are converted. inline int to_int8(int8_t *pv) const; inline int to_uint8(uint8_t *pv) const; inline int to_int(int *pv) const; diff --git a/src/butil/string_splitter_inl.h b/src/butil/string_splitter_inl.h index ed2981fcb6..ef51b9a787 100644 --- a/src/butil/string_splitter_inl.h +++ b/src/butil/string_splitter_inl.h @@ -132,39 +132,45 @@ int StringSplitter::to_uint(unsigned int* pv) const { } int StringSplitter::to_long(long* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtol(field(), &endptr, 10); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtol(value.c_str(), &endptr, 10); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringSplitter::to_ulong(unsigned long* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtoul(field(), &endptr, 10); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtoul(value.c_str(), &endptr, 10); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringSplitter::to_longlong(long long* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtoll(field(), &endptr, 10); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtoll(value.c_str(), &endptr, 10); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringSplitter::to_ulonglong(unsigned long long* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtoull(field(), &endptr, 10); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtoull(value.c_str(), &endptr, 10); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringSplitter::to_float(float* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtof(field(), &endptr); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtof(value.c_str(), &endptr); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringSplitter::to_double(double* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtod(field(), &endptr); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtod(value.c_str(), &endptr); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } StringMultiSplitter::StringMultiSplitter ( @@ -283,39 +289,45 @@ int StringMultiSplitter::to_uint(unsigned int* pv) const { } int StringMultiSplitter::to_long(long* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtol(field(), &endptr, 10); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtol(value.c_str(), &endptr, 10); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringMultiSplitter::to_ulong(unsigned long* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtoul(field(), &endptr, 10); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtoul(value.c_str(), &endptr, 10); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringMultiSplitter::to_longlong(long long* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtoll(field(), &endptr, 10); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtoll(value.c_str(), &endptr, 10); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringMultiSplitter::to_ulonglong(unsigned long long* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtoull(field(), &endptr, 10); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtoull(value.c_str(), &endptr, 10); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringMultiSplitter::to_float(float* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtof(field(), &endptr); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtof(value.c_str(), &endptr); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } int StringMultiSplitter::to_double(double* pv) const { + const std::string value(field(), length()); char* endptr = nullptr; - *pv = strtod(field(), &endptr); - return (endptr == field() + length()) ? 0 : -1; + *pv = strtod(value.c_str(), &endptr); + return (endptr == value.c_str() + value.size()) ? 0 : -1; } void KeyValuePairsSplitter::UpdateDelimiterPosition() { diff --git a/test/string_splitter_unittest.cpp b/test/string_splitter_unittest.cpp index bac31c43e7..dc1fd64e2a 100644 --- a/test/string_splitter_unittest.cpp +++ b/test/string_splitter_unittest.cpp @@ -246,6 +246,46 @@ TEST_F(StringSplitterTest, number_list) { ASSERT_EQ(ss2.field(), str + str_len); } +template +void CheckBoundedNumericField(const Splitter& splitter) { + long l; + unsigned long ul; + long long ll; + unsigned long long ull; + float f; + double d; + EXPECT_EQ(0, splitter.to_long(&l)); + EXPECT_EQ(123, l); + EXPECT_EQ(0, splitter.to_ulong(&ul)); + EXPECT_EQ(123u, ul); + EXPECT_EQ(0, splitter.to_longlong(&ll)); + EXPECT_EQ(123, ll); + EXPECT_EQ(0, splitter.to_ulonglong(&ull)); + EXPECT_EQ(123u, ull); + EXPECT_EQ(0, splitter.to_float(&f)); + EXPECT_FLOAT_EQ(123, f); + EXPECT_EQ(0, splitter.to_double(&d)); + EXPECT_DOUBLE_EQ(123, d); +} + +TEST_F(StringSplitterTest, numeric_conversions_respect_field_end) { + // The suffix is numeric too: parsing must stop at the explicit end, + // rather than consume it and reject the bounded field afterwards. + const char text[] = "123456"; + CheckBoundedNumericField(butil::StringSplitter(text, text + 3, ',')); + CheckBoundedNumericField(butil::StringMultiSplitter(text, text + 3, ",;")); + + // These bytes have no trailing NUL. Sanitizers also check the read bound. + const std::string digits = "123"; + char* bounded = new char[digits.size()]; + memcpy(bounded, digits.data(), digits.size()); + CheckBoundedNumericField(butil::StringSplitter( + bounded, bounded + digits.size(), ',')); + CheckBoundedNumericField(butil::StringMultiSplitter( + bounded, bounded + digits.size(), ",;")); + delete[] bounded; +} + TEST_F(StringSplitterTest, cast_type) { const char* str = "-1\t123\t111\t1\t10\t11\t1.3\t3.1415926\t127\t128\t256"; int i = 0;