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 60da24a9786994732efca0f442b5c21421948ded..84674e222fa739bf362933c63d280300b6e52a71 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 2875a71237346e6be10be67c0b4462abd742a822..4dd09556a8526c6d24fedf14d7a89dc5d145547c 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);
 }