From 15267b2209f6b7233e46d9145cb440fbe536db67 Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Tue, 31 Aug 2010 21:44:05 -0600
Subject: [PATCH] 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").
---
 src/SquidString.h |  2 ++
 src/String.cci    | 67 +++++++++++++++++++++++++----------------------
 2 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/src/SquidString.h b/src/SquidString.h
index 60da24a978..84674e222f 100644
--- 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 2875a71237..4dd09556a8 100644
--- 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);
 }
 
-- 
2.47.2