From: amosjeffries <>
Date: Sun, 20 May 2007 14:29:44 +0000 (+0000)
Subject: Fix 1-off bug left after strnlen bug removal.
X-Git-Tag: SQUID_3_0_PRE7~250
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6b7483bd0d99e80ea0fa765d0df1cf8c2447bf52;p=thirdparty%2Fsquid.git

Fix 1-off bug left after strnlen bug removal.

Also:
 * Harden memory allocations to SqString.
 * Added more unit tests for strings.
---

diff --git a/src/SqString.cc b/src/SqString.cc
index eb0e6a5d4b..32b4d8ec57 100644
--- a/src/SqString.cc
+++ b/src/SqString.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: SqString.cc,v 1.5 2007/05/20 04:22:06 amosjeffries Exp $
+ * $Id: SqString.cc,v 1.6 2007/05/20 08:29:44 amosjeffries Exp $
  *
  * DEBUG: section 67    String
  * AUTHOR: Duane Wessels
@@ -40,16 +40,19 @@
 void
 SqString::initBuf(size_t sz)
 {
+    size_t bsz;
     PROF_start(StringInitBuf);
     clear();
-    buf_ = (char *)memAllocString(sz, &sz);
     assert(sz < 65536);
-    size_ = sz;
+    buf_ = (char *)memAllocString(sz, &bsz);
+    assert(bsz < 65536);
+    assert(bsz >= sz);
+    size_ = bsz;
     PROF_stop(StringInitBuf);
 }
 
 void
-SqString::limitInit(const char *str, int len)
+SqString::limitInit(const char *str, unsigned int len)
 {
     PROF_start(StringLimitInit);
     assert(this && str);
@@ -185,15 +188,16 @@ SqString::append(const char *str, int len)
     if(len < 1 || str == NULL)
         return;
 
-    if (len_ + len < size_) {
+    if ( (len_ + len +1) < size_) {
         operator[](len_+len) = '\0';
         xmemcpy(buf_+len_, str, len);
         len_ += len;
     } else {
         unsigned int ssz = len_ + len;
         unsigned int bsz = len_ + len + 1;
-        char* tmp = (char *)memAllocString(ssz, &bsz);
+        char* tmp = (char *)memAllocString(bsz, &bsz);
         assert(bsz < 65536);
+        assert(bsz > ssz);
 
         if (buf_)
             xmemcpy(tmp, buf_, len_);
@@ -201,7 +205,7 @@ SqString::append(const char *str, int len)
         if (len)
             xmemcpy(tmp + len_, str, len);
 
-        tmp[ssz + 1] = '\0';
+        tmp[ssz] = '\0';
 
         clear();
 
@@ -232,7 +236,7 @@ SqString::append (char chr)
 void
 SqString::append(SqString const &old)
 {
-    append (old.c_str(), old.len_);
+    append (old.c_str(), old.size());
 }
 
 const char&
diff --git a/src/SqString.h b/src/SqString.h
index 8362b0c117..c330bb44c0 100644
--- a/src/SqString.h
+++ b/src/SqString.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: SqString.h,v 1.2 2007/05/18 16:56:18 amosjeffries Exp $
+ * $Id: SqString.h,v 1.3 2007/05/20 08:29:44 amosjeffries Exp $
  *
  * DEBUG: section 67    String
  * AUTHOR: Duane Wessels
@@ -135,7 +135,7 @@ public:
 
 #endif
 
-    void limitInit(const char *str, int len);
+    void limitInit(const char *str, unsigned int len);
 private:
     void initBuf(size_t sz);
     void init (char const *);
diff --git a/src/tests/testString.cc b/src/tests/testString.cc
index 083693ae36..58baa4f902 100644
--- a/src/tests/testString.cc
+++ b/src/tests/testString.cc
@@ -123,7 +123,55 @@ testString::testAppend()
     cStr.append("rld\0 untroubled by things such as null termination", 10);
     CPPUNIT_ASSERT( !cStr.empty() );
     CPPUNIT_ASSERT_EQUAL( 18, cStr.size() );
-    CPPUNIT_ASSERT_EQUAL( (string)"hello world\0 untr", cStr );
+    CPPUNIT_ASSERT( memcmp("hello world", cStr.c_str(), 11) == 0 );
+    CPPUNIT_ASSERT( memcmp("hello world\0", cStr.c_str(), 12) == 0 );
+    CPPUNIT_ASSERT( memcmp("hello world\0 untro", cStr.c_str(), 18) == 0 );
+    CPPUNIT_ASSERT( memcmp("hello world\0 untro\0", cStr.c_str(), 19) == 0 );
+}
+
+void
+testString::testAccess()
+{
+    string test;
+    test = "123456789a"; // to get a predictable length buffer.
+
+    CPPUNIT_ASSERT_EQUAL( test.size(), 10 );
+
+/* FIXME: flow checks do not seem to catch assert() sent from within code. */
+      /* underflow handling test: _should_ fail with core dump. */
+      /* this SHOULD be impossible due to unsigned type of parameter. */
+//    CPPUNIT_ASSERT_ASSERTION_FAIL( CPPUNIT_ASSERT( test[-1] ) );
+
+      /* overflow handling test: _should_ fail with core dump. */
+//    CPPUNIT_ASSERT_ASSERTION_FAIL( CPPUNIT_ASSERT( test[test.size()+10] ) );
+
+      /* [] access method (read and write) */
+    CPPUNIT_ASSERT( test[0]  == '1' );
+    CPPUNIT_ASSERT( test[9]  == 'a' );
+    CPPUNIT_ASSERT( test[10] == '\0' );
+
+    test.append('T');
+    CPPUNIT_ASSERT( test[10]  ==  'T' );
+    CPPUNIT_ASSERT( test[11]  == '\0' );
+    CPPUNIT_ASSERT_EQUAL((string)"123456789aT", test);
+
+      /* Random access inside buffer. */
+    test[5] = 't';
+    CPPUNIT_ASSERT( test[5]   ==  't' );
+    CPPUNIT_ASSERT( test[11]  == '\0' );
+    CPPUNIT_ASSERT_EQUAL((string)"12345t789aT", test);
+
+      /* border case at last position of string */
+    test[9] = 'E';
+    CPPUNIT_ASSERT( test[9]   ==  'E' );
+    CPPUNIT_ASSERT( test[11]  == '\0' );
+    CPPUNIT_ASSERT_EQUAL((string)"12345t789ET", test);
+
+      /* border case at EOS position */
+    test[11] = 'F';
+    CPPUNIT_ASSERT( test[11]  == 'F' );
+    CPPUNIT_ASSERT( test[12]  == '\0' );
+    CPPUNIT_ASSERT_EQUAL((string)"12345t789ETF", test);
 }
 
 void
diff --git a/src/tests/testString.h b/src/tests/testString.h
index 69cdd42b09..52951a2fef 100644
--- a/src/tests/testString.h
+++ b/src/tests/testString.h
@@ -20,6 +20,7 @@ class testString : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST( testBooleans );
     CPPUNIT_TEST( testAppend );
     CPPUNIT_TEST( testAssignments );
+    CPPUNIT_TEST( testAccess );
     CPPUNIT_TEST( testCstrMethods );
     CPPUNIT_TEST( testSearch );
     CPPUNIT_TEST_SUITE_END();
@@ -36,6 +37,7 @@ protected:
     void testBooleans();
     void testAppend();
     void testAssignments();
+    void testAccess();
     void testCstrMethods();
     void testSearch();
 };