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 a0394c8a79183411e2750cc138990d011ad63fd7..f37e3d1b864f700d8764f32b547613180d4b5833 100644 (file)
--- a/src/SquidString.h
+++ b/src/SquidString.h
@@ -127,6 +127,8 @@ public:
 #endif
 
 private:
+    _SQUID_INLINE_ bool nilCmp(bool, bool, int &) const;
+
     /* never reference these directly! */
     unsigned short int size_; /* buffer size; 64K limit */
 

diff --git a/src/String.cci b/src/String.cci
index 7dd7ee328a99f22ba45e3cdfa462918cfe5ae142..085b94e6f829535dc42179c64a6529948dc5c75e 100644 (file)
--- a/src/String.cci
+++ b/src/String.cci
@@ -73,19 +73,31 @@ String::rpos(char const ch) const
     return strrchr(buf(), (ch));
 }
 
-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(buf(), aString);
 }
@@ -93,19 +105,9 @@ String::cmp (char const *aString) const
 int
 String::cmp (char const *aString, size_t 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(buf(), aString, count);
 }
@@ -113,16 +115,9 @@ String::cmp (char const *aString, size_t 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(buf(), aString.buf());
 }
@@ -130,12 +125,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(buf(), aString);
 }
 
 int
 String::caseCmp(char const *aString, size_t count) const
 {
+    int result = 0;
+    if (nilCmp((!size() || !count), (!aString || !*aString || !count), result))
+        return result;
+
     return strncasecmp(buf(), aString, count);
 }