------------------------------------------------------------ revno: 10090 revision-id: amosjeffries@squid-cache.org-20100901074826-a10vnuc1o4txo20i parent: squidadm@squid-cache.org-20100831062658-7ekcy87grpp24xmr committer: Amos Jeffries branch nick: SQUID_3_1 timestamp: Wed 2010-09-01 01:48:26 -0600 message: Author: Alex Rousskov 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"). ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: amosjeffries@squid-cache.org-20100901074826-\ # a10vnuc1o4txo20i # target_branch: http://www.squid-cache.org/bzr/squid3/trunk/ # testament_sha1: 55350f2ccd1a553e0461a3a08d7089f719b1585f # timestamp: 2010-09-01 07:51:30 +0000 # source_branch: http://www.squid-cache.org/bzr/squid3/branches\ # /SQUID_3_1 # base_revision_id: squidadm@squid-cache.org-20100831062658-\ # 7ekcy87grpp24xmr # # Begin patch === modified file 'src/SquidString.h' --- src/SquidString.h 2009-06-05 23:47:35 +0000 +++ src/SquidString.h 2010-09-01 07:48:26 +0000 @@ -167,6 +167,8 @@ 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 */ === modified file 'src/String.cci' --- src/String.cci 2009-11-21 11:01:43 +0000 +++ src/String.cci 2010-09-01 07:48:26 +0000 @@ -88,19 +88,31 @@ } +/// 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 +{ + if (!thisIsNilOrEmpty && !otherIsNilOrEmpty) + return false; // result does not matter + + if (thisIsNilOrEmpty && otherIsNilOrEmpty) + result = 0; + else if (thisIsNilOrEmpty) + result = -1; + else // otherIsNilOrEmpty + result = +1; + + return true; +} + + int String::cmp (char const *aString) const { - /* strcmp fails on NULLS */ - - 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(), (!aString || !*aString), result)) + return result; return strcmp(termedBuf(), aString); } @@ -108,19 +120,9 @@ 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 @@ 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 @@ 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); }