Author: Alex Rousskov <rousskov@measurement-factory.com>

Check for NULL and empty strings before calling str*cmp().

These checks are necessary to ensure consistent comparison results (important
for sorting and searching) and to avoid segfaults on NULL buffers (because
termedBuf() may return NULL instead of the expected "0-terminated buffer").

diff --git a/src/SquidString.h b/src/SquidString.h
index 657cd7dfcc079c11911f3400d90c10edfe8059d7..c8d33b0314c17f2679138470454e90e4dbbc2853 100644 (file)
--- a/src/SquidString.h
+++ b/src/SquidString.h
@@ -167,6 +167,8 @@ private:
     void allocBuffer(size_type sz);
     void setBuffer(char *buf, size_type sz);
 
+    _SQUID_INLINE_ bool nilCmp(bool, bool, int &) const;
+
     /* never reference these directly! */
     size_type size_; /* buffer size; 64K limit */
 

diff --git a/src/String.cci b/src/String.cci
index 085f383517e5557fc7813dcd970d88455f4e89bb..050de32bba8022a8472b1ad7d585bbe6bb835887 100644 (file)
--- a/src/String.cci
+++ b/src/String.cci
@@ -88,19 +88,31 @@ String::operator [](unsigned int aPos) const
 }
 
 
-int
-String::cmp (char const *aString) const
+/// compare NULL and empty strings because str*cmp() may fail on NULL strings
+/// and because we need to return consistent results for strncmp(count == 0).
+bool
+String::nilCmp(const bool thisIsNilOrEmpty, const bool otherIsNilOrEmpty, int &result) const
 {
-    /* strcmp fails on NULLS */
+    if (!thisIsNilOrEmpty && !otherIsNilOrEmpty)
+        return false; // result does not matter
 
-    if (size() == 0 && (aString == NULL || aString[0] == '\0'))
-        return 0;
+    if (thisIsNilOrEmpty && otherIsNilOrEmpty)
+        result = 0;
+    else if (thisIsNilOrEmpty)
+        result = -1;
+    else // otherIsNilOrEmpty
+        result = +1;
+
+    return true;
+}
 
-    if (size() == 0)
-        return -1;
 
-    if (aString == NULL || aString[0] == '\0')
-        return 1;
+int
+String::cmp (char const *aString) const
+{
+    int result = 0;
+    if (nilCmp(!size(), (!aString || !*aString), result))
+        return result;
 
     return strcmp(termedBuf(), aString);
 }
@@ -108,19 +120,9 @@ String::cmp (char const *aString) const
 int
 String::cmp (char const *aString, String::size_type count) const
 {
-    /* always the same at length 0 */
-
-    if (count == 0)
-        return 0;
-
-    if (size() == 0 && (aString == NULL || aString[0] == '\0'))
-        return 0;
-
-    if (size() == 0)
-        return -1;
-
-    if (aString == NULL || aString[0] == '\0')
-        return 1;
+    int result = 0;
+    if (nilCmp((!size() || !count), (!aString || !*aString || !count), result))
+        return result;
 
     return strncmp(termedBuf(), aString, count);
 }
@@ -128,16 +130,9 @@ String::cmp (char const *aString, String::size_type count) const
 int
 String::cmp (String const &aString) const
 {
-    /* strcmp fails on NULLS */
-
-    if (size() == 0 && aString.size() == 0)
-        return 0;
-
-    if (size() == 0)
-        return -1;
-
-    if (aString.size() == 0)
-        return 1;
+    int result = 0;
+    if (nilCmp(!size(), !aString.size(), result))
+        return result;
 
     return strcmp(termedBuf(), aString.termedBuf());
 }
@@ -145,12 +140,20 @@ String::cmp (String const &aString) const
 int
 String::caseCmp(char const *aString) const
 {
+    int result = 0;
+    if (nilCmp(!size(), (!aString || !*aString), result))
+        return result;
+
     return strcasecmp(termedBuf(), aString);
 }
 
 int
 String::caseCmp(char const *aString, String::size_type count) const
 {
+    int result = 0;
+    if (nilCmp((!size() || !count), (!aString || !*aString || !count), result))
+        return result;
+
     return strncasecmp(termedBuf(), aString, count);
 }