Fix potential invalid memory access in StringSplitter (#2996)

Author: gitccl

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;
 }

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) {