------------------------------------------------------------ revno: 9189 revision-id: amosjeffries@squid-cache.org-20100901075447-forii2klfwibdo1i parent: amosjeffries@squid-cache.org-20100731142846-lg1oecplw4vwrgo3 committer: Amos Jeffries branch nick: SQUID_3_0 timestamp: Wed 2010-09-01 01:54:47 -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-20100901075447-\ # forii2klfwibdo1i # target_branch: http://www.squid-cache.org/bzr/squid3/branches\ # /SQUID_3_0/ # testament_sha1: e7f7fabcaa516a40cb0d055e7f7c4624a7d2cf54 # timestamp: 2010-09-01 07:55:26 +0000 # source_branch: http://www.squid-cache.org/bzr/squid3/branches\ # /SQUID_3_0 # base_revision_id: amosjeffries@squid-cache.org-20100731142846-\ # lg1oecplw4vwrgo3 # # Begin patch === modified file 'src/SquidString.h' --- src/SquidString.h 2008-02-27 17:47:59 +0000 +++ src/SquidString.h 2010-09-01 07:54:47 +0000 @@ -127,6 +127,8 @@ #endif private: + _SQUID_INLINE_ bool nilCmp(bool, bool, int &) const; + /* never reference these directly! */ unsigned short int size_; /* buffer size; 64K limit */ === modified file 'src/String.cci' --- src/String.cci 2008-06-13 05:13:47 +0000 +++ src/String.cci 2010-09-01 07:54:47 +0000 @@ -73,19 +73,31 @@ return strrchr(buf(), (ch)); } +/// 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(buf(), aString); } @@ -93,19 +105,9 @@ 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 @@ 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 @@ 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); }