Skip to content

Respect splitter field bounds in numeric conversions #3660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wasphin wants to merge 1 commit into
base: master
Choose a base branch
from
+78 −26
Open

Respect splitter field bounds in numeric conversions #3660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/butil/string_splitter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
60 changes: 36 additions & 24 deletions src/butil/string_splitter_inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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() {
Expand Down
40 changes: 40 additions & 0 deletions test/string_splitter_unittest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,46 @@ TEST_F(StringSplitterTest, number_list) {
ASSERT_EQ(ss2.field(), str + str_len);
}

template <typename Splitter>
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;
Expand Down
Loading