This is an automated email from the ASF dual-hosted git repository.

wwbmmm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git


The following commit(s) were added to refs/heads/master by this push:
     new 10f8b24b Fix potential invalid memory access in StringSplitter (#2996)
10f8b24b is described below

commit 10f8b24b96ba85314d1c37ee30bf99ea88b8944a
Author: Chen Chuanle <60637740+git...@users.noreply.github.com>
AuthorDate: Mon Jun 16 10:07:04 2025 +0800

    Fix potential invalid memory access in StringSplitter (#2996)
---
 src/butil/string_splitter_inl.h   | 16 +++++++-------
 test/string_splitter_unittest.cpp | 45 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/src/butil/string_splitter_inl.h b/src/butil/string_splitter_inl.h
index 79f3f924..ae035f13 100644
--- a/src/butil/string_splitter_inl.h
+++ b/src/butil/string_splitter_inl.h
@@ -47,9 +47,9 @@ void StringSplitter::init() {
     // Find the starting _head and _tail.
     if (__builtin_expect(_head != NULL, 1)) {
         if (_empty_field_action == SKIP_EMPTY_FIELD) {
-            for (; _sep == *_head && not_end(_head); ++_head) {}
+            for (; not_end(_head) && *_head == _sep; ++_head) {}
         }
-        for (_tail = _head; *_tail != _sep && not_end(_tail); ++_tail) {}
+        for (_tail = _head; not_end(_tail) && *_tail != _sep; ++_tail) {}
     } else {
         _tail = NULL;
     }
@@ -60,11 +60,11 @@ StringSplitter& StringSplitter::operator++() {
         if (not_end(_tail)) {
             ++_tail;
             if (_empty_field_action == SKIP_EMPTY_FIELD) {
-                for (; _sep == *_tail && not_end(_tail); ++_tail) {}
+                for (; not_end(_tail) && *_tail == _sep; ++_tail) {}
             }
         }
         _head = _tail;
-        for (; *_tail != _sep && not_end(_tail); ++_tail) {}
+        for (; not_end(_tail) && *_tail != _sep; ++_tail) {}
     }
     return *this;
 }
@@ -189,9 +189,9 @@ StringMultiSplitter::StringMultiSplitter (
 void StringMultiSplitter::init() {
     if (__builtin_expect(_head != NULL, 1)) {
         if (_empty_field_action == SKIP_EMPTY_FIELD) {
-            for (; is_sep(*_head) && not_end(_head); ++_head) {}
+            for (; not_end(_head) && is_sep(*_head); ++_head) {}
         }
-        for (_tail = _head; !is_sep(*_tail) && not_end(_tail); ++_tail) {}
+        for (_tail = _head; not_end(_tail) && !is_sep(*_tail); ++_tail) {}
     } else {
         _tail = NULL;
     }
@@ -202,11 +202,11 @@ StringMultiSplitter& StringMultiSplitter::operator++() {
         if (not_end(_tail)) {
             ++_tail;
             if (_empty_field_action == SKIP_EMPTY_FIELD) {
-                for (; is_sep(*_tail) && not_end(_tail); ++_tail) {}
+                for (; not_end(_tail) && is_sep(*_tail); ++_tail) {}
             }
         }
         _head = _tail;
-        for (; !is_sep(*_tail) && not_end(_tail); ++_tail) {}
+        for (; not_end(_tail) && !is_sep(*_tail); ++_tail) {}
     }
     return *this;
 }
diff --git a/test/string_splitter_unittest.cpp 
b/test/string_splitter_unittest.cpp
index 7296198c..c3b66a24 100644
--- a/test/string_splitter_unittest.cpp
+++ b/test/string_splitter_unittest.cpp
@@ -354,6 +354,51 @@ TEST_F(StringSplitterTest, split_limit_len) {
     ASSERT_FALSE(ss3);
 }
 
+TEST_F(StringSplitterTest, non_null_terminated_string) {
+    const char str[] = "  a non  null   terminated  string   ";
+    const size_t len = strlen(str);
+    char* buf = new char[len];
+    memcpy(buf, str, len);
+
+    butil::StringSplitter ss(buf, buf + len, ' ');
+
+    // "a"
+    ASSERT_TRUE(ss != NULL);
+    ASSERT_EQ(1ul, ss.length());
+    ASSERT_EQ(ss.field(), buf + 2);
+
+    // "non"
+    ++ss;
+    ASSERT_TRUE(ss != NULL);
+    ASSERT_EQ(3ul, ss.length());
+    ASSERT_EQ(ss.field(), buf + 4);
+
+    // "null"
+    ++ss;
+    ASSERT_TRUE(ss != NULL);
+    ASSERT_EQ(4ul, ss.length());
+    ASSERT_EQ(ss.field(), buf + 9);
+
+    // "terminated"
+    ++ss;
+    ASSERT_TRUE(ss != NULL);
+    ASSERT_EQ(10ul, ss.length());
+    ASSERT_EQ(ss.field(), buf + 16);
+
+    // "string"
+    ++ss;
+    ASSERT_TRUE(ss != NULL);
+    ASSERT_EQ(6ul, ss.length());
+    ASSERT_EQ(ss.field(), buf + 28);
+
+    ++ss;
+    ASSERT_FALSE(ss);
+    ASSERT_EQ(0ul, ss.length());
+    ASSERT_EQ(ss.field(), buf + len);
+
+    delete[] buf;
+}
+
 TEST_F(StringSplitterTest, key_value_pairs_splitter_sanity) {
     std::string kvstr = 
"key1=value1&&&key2=value2&key3=value3&===&key4=&=&=value5";
     for (int i = 0 ; i < 3; ++i) {


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org

Reply via email to