Replace base::str[n]casecmp with helper functions.

Adds CompareCaseInsensitiveASCII and EqualsCaseInsensitiveASCII helper functions and removes base::strcasecmp and base::strncasecmp. This avoids the dangerous locale-sensitive behavior.

ClientIsAdvertisingSdchEncoding in sdch_browsertest had the condition inverted, but because it returned true any time the given line wasn't found, the test didn't notice.

cups_helper changed most significantly. I redid the loop to use StringPieces which saves a lot of copies. On line 82 cups_helper used to do a prefix match which I'm pretty sure it wanted a regular compare. I changed this.

render_text_harfbuzz set "<" operator was also doing a prefix comparison only, when it looks like it really just wanted to compare the strings.

file_path passed string pieces into strcasecmp which could then read off the end if they're not null terminated. This patch fixes the bug and calls the native strcasecmp which is probably the best we can do for Posix file names.

Removed additional version of the same function in net.

Adds a backwards-compat hack for crashpad which improperly uses base from a DEPS-ed in repo.

Review URL: https://codereview.chromium.org/1224553010

Cr-Commit-Position: refs/heads/master@{#338324}

base/files/file_path.cc

@@ -10,9 +10,6 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/pickle.h"
-
-// These includes are just for the *Hack functions, and should be removed
-// when those functions are removed.
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
@@ -1259,11 +1256,12 @@ int FilePath::CompareIgnoreCase(StringPieceType string1,
 
 #else  // << WIN. MACOSX | other (POSIX) >>
 
-// Generic (POSIX) implementation of file string comparison.
-// TODO(rolandsteiner) check if this is sufficient/correct.
+// Generic Posix system comparisons.
 int FilePath::CompareIgnoreCase(StringPieceType string1,
                                 StringPieceType string2) {
-  int comparison = strcasecmp(string1.data(), string2.data());
+  // Specifically need null termianted strings for this API call.
+  int comparison = strcasecmp(string1.as_string().c_str(),
+                              string2.as_string().c_str());
   if (comparison < 0)
     return -1;
   if (comparison > 0)

base/strings/string_util.cc

@@ -140,6 +140,53 @@ bool IsWprintfFormatPortable(const wchar_t* format) {
   return true;
 }
 
+template<class StringType>
+int CompareCaseInsensitiveASCIIT(BasicStringPiece<StringType> a,
+                                 BasicStringPiece<StringType> b) {
+  // Find the first characters that aren't equal and compare them.  If the end
+  // of one of the strings is found before a nonequal character, the lengths
+  // of the strings are compared.
+  size_t i = 0;
+  while (i < a.length() && i < b.length()) {
+    typename StringType::value_type lower_a = ToLowerASCII(a[i]);
+    typename StringType::value_type lower_b = ToLowerASCII(b[i]);
+    if (lower_a < lower_b)
+      return -1;
+    if (lower_a > lower_b)
+      return 1;
+    i++;
+  }
+
+  // End of one string hit before finding a different character. Expect the
+  // common case to be "strings equal" at this point so check that first.
+  if (a.length() == b.length())
+    return 0;
+
+  if (a.length() < b.length())
+    return -1;
+  return 1;
+}
+
+int CompareCaseInsensitiveASCII(base::StringPiece a, base::StringPiece b) {
+  return CompareCaseInsensitiveASCIIT<std::string>(a, b);
+}
+
+int CompareCaseInsensitiveASCII(base::StringPiece16 a, base::StringPiece16 b) {
+  return CompareCaseInsensitiveASCIIT<base::string16>(a, b);
+}
+
+bool EqualsCaseInsensitiveASCII(base::StringPiece a, base::StringPiece b) {
+  if (a.length() != b.length())
+    return false;
+  return CompareCaseInsensitiveASCIIT<std::string>(a, b) == 0;
+}
+
+bool EqualsCaseInsensitiveASCII(base::StringPiece16 a, base::StringPiece16 b) {
+  if (a.length() != b.length())
+    return false;
+  return CompareCaseInsensitiveASCIIT<base::string16>(a, b) == 0;
+}
+
 const std::string& EmptyString() {
   return EmptyStrings::GetInstance()->s;
 }

base/strings/string_util.h

@@ -21,20 +21,10 @@
 
 namespace base {
 
-// C standard-library functions like "strncasecmp" and "snprintf" that aren't
-// cross-platform are provided as "base::strncasecmp", and their prototypes
-// are listed below.  These functions are then implemented as inline calls
-// to the platform-specific equivalents in the platform-specific headers.
-
-// Compares the two strings s1 and s2 without regard to case using
-// the current locale; returns 0 if they are equal, 1 if s1 > s2, and -1 if
-// s2 > s1 according to a lexicographic comparison.
-int strcasecmp(const char* s1, const char* s2);
-
-// Compares up to count characters of s1 and s2 without regard to case using
-// the current locale; returns 0 if they are equal, 1 if s1 > s2, and -1 if
-// s2 > s1 according to a lexicographic comparison.
-int strncasecmp(const char* s1, const char* s2, size_t count);
+// C standard-library functions that aren't cross-platform are provided as
+// "base::...", and their prototypes are listed below. These functions are
+// then implemented as inline calls to the platform-specific equivalents in the
+// platform-specific headers.
 
 // Wrapper for vsnprintf that always null-terminates and always returns the
 // number of characters that would be in an untruncated formatted
@@ -56,6 +46,19 @@ inline int snprintf(char* buffer, size_t size, const char* format, ...) {
   return result;
 }
 
+// TODO(mark) http://crbug.com/472900 crashpad shouldn't use base while
+// being DEPSed in. This backwards-compat hack is provided until crashpad is
+// updated.
+#if defined(OS_WIN)
+inline int strcasecmp(const char* s1, const char* s2) {
+  return _stricmp(s1, s2);
+}
+#else  // Posix
+inline int strcasecmp(const char* string1, const char* string2) {
+  return ::strcasecmp(string1, string2);
+}
+#endif
+
 // BSD-style safe and consistent string copy functions.
 // Copies |src| to |dst|, where |dst_size| is the total allocated size of |dst|.
 // Copies at most |dst_size|-1 characters, and always NULL terminates |dst|, as
@@ -102,10 +105,13 @@ template <class Char> inline Char ToUpperASCII(Char c) {
 
 // Function objects to aid in comparing/searching strings.
 
+// DO NOT USE. tolower() will given incorrect results for non-ASCII characters.
+// Use the ASCII version, base::i18n::ToLower, or base::i18n::FoldCase.
 template<typename Char> struct CaseInsensitiveCompare {
  public:
   bool operator()(Char x, Char y) const {
     // TODO(darin): Do we really want to do locale sensitive comparisons here?
+    // ANSWER(brettw): No.
     // See http://crbug.com/24917
     return tolower(x) == tolower(y);
   }
@@ -118,6 +124,22 @@ template<typename Char> struct CaseInsensitiveCompareASCII {
   }
 };
 
+// Like strcasecmp for case-insensitive ASCII characters only. Returns:
+//   -1  (a < b)
+//    0  (a == b)
+//    1  (a > b)
+// (unlike strcasecmp which can return values greater or less than 1/-1). For
+// full Unicode support, use base::i18n::ToLower or base::i18h::FoldCase
+// and then just call the normal string operators on the result.
+BASE_EXPORT int CompareCaseInsensitiveASCII(StringPiece a, StringPiece b);
+BASE_EXPORT int CompareCaseInsensitiveASCII(StringPiece16 a, StringPiece16 b);
+
+// Equality for ASCII case-insensitive comparisons. For full Unicode support,
+// use base::i18n::ToLower or base::i18h::FoldCase and then compare with either
+// == or !=.
+BASE_EXPORT bool EqualsCaseInsensitiveASCII(StringPiece a, StringPiece b);
+BASE_EXPORT bool EqualsCaseInsensitiveASCII(StringPiece16 a, StringPiece16 b);
+
 // These threadsafe functions return references to globally unique empty
 // strings.
 //

base/strings/string_util_posix.h

@@ -20,14 +20,6 @@ inline char* strdup(const char* str) {
   return ::strdup(str);
 }
 
-inline int strcasecmp(const char* string1, const char* string2) {
-  return ::strcasecmp(string1, string2);
-}
-
-inline int strncasecmp(const char* string1, const char* string2, size_t count) {
-  return ::strncasecmp(string1, string2, count);
-}
-
 inline int vsnprintf(char* buffer, size_t size,
                      const char* format, va_list arguments) {
   return ::vsnprintf(buffer, size, format, arguments);

base/strings/string_util_unittest.cc

@@ -1056,6 +1056,26 @@ TEST(StringUtilTest, ContainsOnlyChars) {
                                   kWhitespaceUTF16));
 }
 
+TEST(StringUtilTest, CompareCaseInsensitiveASCII) {
+  EXPECT_EQ(0, CompareCaseInsensitiveASCII("", ""));
+  EXPECT_EQ(0, CompareCaseInsensitiveASCII("Asdf", "aSDf"));
+
+  // Differing lengths.
+  EXPECT_EQ(-1, CompareCaseInsensitiveASCII("Asdf", "aSDfA"));
+  EXPECT_EQ(1, CompareCaseInsensitiveASCII("AsdfA", "aSDf"));
+
+  // Differing values.
+  EXPECT_EQ(-1, CompareCaseInsensitiveASCII("AsdfA", "aSDfb"));
+  EXPECT_EQ(1, CompareCaseInsensitiveASCII("Asdfb", "aSDfA"));
+}
+
+TEST(StringUtilTest, EqualsCaseInsensitiveASCII) {
+  EXPECT_TRUE(EqualsCaseInsensitiveASCII("", ""));
+  EXPECT_TRUE(EqualsCaseInsensitiveASCII("Asdf", "aSDF"));
+  EXPECT_FALSE(EqualsCaseInsensitiveASCII("bsdf", "aSDF"));
+  EXPECT_FALSE(EqualsCaseInsensitiveASCII("Asdf", "aSDFz"));
+}
+
 class WriteIntoTest : public testing::Test {
  protected:
   static void WritesCorrectly(size_t num_chars) {

base/strings/string_util_win.h

@@ -20,14 +20,6 @@ inline char* strdup(const char* str) {
   return _strdup(str);
 }
 
-inline int strcasecmp(const char* s1, const char* s2) {
-  return _stricmp(s1, s2);
-}
-
-inline int strncasecmp(const char* s1, const char* s2, size_t count) {
-  return _strnicmp(s1, s2, count);
-}
-
 inline int vsnprintf(char* buffer, size_t size,
                      const char* format, va_list arguments) {
   int length = vsnprintf_s(buffer, size, size - 1, format, arguments);

chrome/browser/media_galleries/fileapi/media_path_filter.cc

@@ -126,15 +126,12 @@ bool MediaPathFilter::ShouldSkip(const base::FilePath& path) {
   const char win_98_recycle_bin_name[] = "RECYCLED";
   const char win_xp_recycle_bin_name[] = "RECYCLER";
   const char win_vista_recycle_bin_name[] = "$Recycle.bin";
-  if ((base::strncasecmp(base_name.c_str(),
-                         win_98_recycle_bin_name,
-                         strlen(win_98_recycle_bin_name)) == 0) ||
-      (base::strncasecmp(base_name.c_str(),
-                         win_xp_recycle_bin_name,
-                         strlen(win_xp_recycle_bin_name)) == 0) ||
-      (base::strncasecmp(base_name.c_str(),
-                         win_vista_recycle_bin_name,
-                         strlen(win_vista_recycle_bin_name)) == 0))
+  if (base::StartsWith(base_name, win_98_recycle_bin_name,
+                       base::CompareCase::INSENSITIVE_ASCII) ||
+      base::StartsWith(base_name, win_xp_recycle_bin_name,
+                       base::CompareCase::INSENSITIVE_ASCII) ||
+      base::StartsWith(base_name, win_vista_recycle_bin_name,
+                       base::CompareCase::INSENSITIVE_ASCII))
     return true;
 #endif  // defined(OS_WIN)
   return false;

chrome/browser/net/sdch_browsertest.cc

@@ -108,7 +108,7 @@ bool GetRequestHeader(const HttpRequestHeaderMap& map,
                       std::string* value) {
   for (HttpRequestHeaderMap::const_iterator it = map.begin();
        it != map.end(); ++it) {
-    if (!base::strcasecmp(it->first.c_str(), header)) {
+    if (base::EqualsCaseInsensitiveASCII(it->first, header)) {
       *value = it->second;
       return true;
     }
@@ -170,7 +170,7 @@ class SdchResponseHandler {
       return false;
     base::StringTokenizer tokenizer(value, " ,");
     while (tokenizer.GetNext()) {
-      if (base::strcasecmp(tokenizer.token().c_str(), "sdch"))
+      if (base::EqualsCaseInsensitiveASCII(tokenizer.token(), "sdch"))
         return true;
     }
     return false;

chrome/browser/printing/printing_layout_browsertest.cc

@@ -207,15 +207,15 @@ class PrintingLayoutTest : public PrintingTest<InProcessBrowserTest>,
       base::FilePath file;
       while (!(file = enumerator.Next()).empty()) {
         std::wstring ext = file.Extension();
-        if (base::strcasecmp(base::WideToUTF8(ext).c_str(), ".emf") == 0) {
+        if (base::EqualsCaseInsensitiveASCII(base::WideToUTF8(ext), ".emf")) {
           EXPECT_FALSE(found_emf) << "Found a leftover .EMF file: \"" <<
               emf_file << "\" and \"" << file.value() <<
               "\" when looking for \"" << verification_name << "\"";
           found_emf = true;
           emf_file = file.value();
           continue;
         }
-        if (base::strcasecmp(base::WideToUTF8(ext).c_str(), ".prn") == 0) {
+        if (base::EqualsCaseInsensitiveASCII(base::WideToUTF8(ext), ".prn")) {
           EXPECT_FALSE(found_prn) << "Found a leftover .PRN file: \"" <<
               prn_file << "\" and \"" << file.value() <<
               "\" when looking for \"" << verification_name << "\"";

chrome/service/cloud_print/cloud_print_connector.cc

@@ -653,7 +653,7 @@ void CloudPrintConnector::OnReceivePrinterCaps(
 
 bool CloudPrintConnector::IsSamePrinter(const std::string& name1,
                                         const std::string& name2) const {
-  return (0 == base::strcasecmp(name1.c_str(), name2.c_str()));
+  return base::EqualsCaseInsensitiveASCII(name1, name2);
 }
 
 }  // namespace cloud_print

chrome/service/cloud_print/cloud_print_proxy_backend.cc

@@ -550,8 +550,8 @@ void CloudPrintProxyBackend::Core::OnIncomingNotification(
 
   DCHECK(base::MessageLoop::current() == backend_->core_thread_.message_loop());
   VLOG(1) << "CP_CONNECTOR: Incoming notification.";
-  if (0 == base::strcasecmp(kCloudPrintPushNotificationsSource,
-                            notification.channel.c_str()))
+  if (base::EqualsCaseInsensitiveASCII(kCloudPrintPushNotificationsSource,
+                                       notification.channel))
     HandlePrinterNotification(notification.data);
 }
 

chrome/test/chromedriver/performance_logger.cc

@@ -43,7 +43,8 @@ bool IsEnabled(const PerfLoggingPrefs::InspectorDomainStatus& domain_status) {
 bool ShouldRequestTraceEvents(const std::string& command) {
   for (size_t i_domain = 0; i_domain < arraysize(kRequestTraceCommands);
        ++i_domain) {
-    if (base::strcasecmp(command.c_str(), kRequestTraceCommands[i_domain]) == 0)
+    if (base::EqualsCaseInsensitiveASCII(command,
+                                         kRequestTraceCommands[i_domain]))
       return true;
   }
   return false;

components/mime_util/mime_util.cc

@@ -221,8 +221,8 @@ net::CertificateMimeType GetCertificateMimeTypeForMimeType(
   // Don't create a map, there is only one entry in the table,
   // except on Android.
   for (size_t i = 0; i < arraysize(kSupportedCertificateTypes); ++i) {
-    if (base::strcasecmp(mime_type.c_str(),
-                         kSupportedCertificateTypes[i].mime_type) == 0) {
+    if (base::EqualsCaseInsensitiveASCII(
+            mime_type, kSupportedCertificateTypes[i].mime_type)) {
       return kSupportedCertificateTypes[i].cert_type;
     }
   }

components/policy/core/common/registry_dict_win.cc

@@ -136,7 +136,7 @@ scoped_ptr<base::Value> ConvertValue(const base::Value& value,
 
 bool CaseInsensitiveStringCompare::operator()(const std::string& a,
                                               const std::string& b) const {
-  return base::strcasecmp(a.c_str(), b.c_str()) < 0;
+  return base::CompareCaseInsensitiveASCII(a, b) < 0;
 }
 
 RegistryDict::RegistryDict() {}

content/common/service_worker/service_worker_types.h

@@ -102,7 +102,7 @@ enum ServiceWorkerFetchEventResult {
 
 struct ServiceWorkerCaseInsensitiveCompare {
   bool operator()(const std::string& lhs, const std::string& rhs) const {
-    return base::strcasecmp(lhs.c_str(), rhs.c_str()) < 0;
+    return base::CompareCaseInsensitiveASCII(lhs, rhs) < 0;
   }
 };
 

content/plugin/webplugin_proxy.cc

@@ -280,7 +280,7 @@ void WebPluginProxy::HandleURLRequest(const char* url,
                                       int notify_id,
                                       bool popups_allowed,
                                       bool notify_redirects) {
- if (!target && (0 == base::strcasecmp(method, "GET"))) {
+ if (!target && base::EqualsCaseInsensitiveASCII(method, "GET")) {
     // Please refer to https://bugzilla.mozilla.org/show_bug.cgi?id=366082
     // for more details on this.
     if (delegate_->GetQuirks() &

device/usb/usb_service_impl.cc

@@ -127,10 +127,9 @@ bool IsWinUsbInterface(const std::string& device_path) {
   }
 
   USB_LOG(DEBUG) << "Driver for " << device_path << " is " << buffer << ".";
-  if (base::strncasecmp("WinUSB", (const char*)&buffer[0], sizeof "WinUSB") ==
-      0) {
+  if (base::StartsWith(reinterpret_cast<const char*>(buffer), "WinUSB",
+                       base::CompareCase::INSENSITIVE_ASCII))
     return true;
-  }
   return false;
 }
 

extensions/browser/api/web_request/form_data_parser.cc

@@ -315,11 +315,11 @@ scoped_ptr<FormDataParser> FormDataParser::CreateFromContentTypeHeader(
     const std::string content_type(
         content_type_header->substr(0, content_type_header->find(';')));
 
-    if (base::strcasecmp(
-        content_type.c_str(), "application/x-www-form-urlencoded") == 0) {
+    if (base::EqualsCaseInsensitiveASCII(content_type,
+                                         "application/x-www-form-urlencoded")) {
       choice = URL_ENCODED;
-    } else if (base::strcasecmp(
-        content_type.c_str(), "multipart/form-data") == 0) {
+    } else if (base::EqualsCaseInsensitiveASCII(content_type,
+                                                "multipart/form-data")) {
       static const char kBoundaryString[] = "boundary=";
       size_t offset = content_type_header->find(kBoundaryString);
       if (offset == std::string::npos) {

media/video/capture/fake_video_capture_device_factory.cc

@@ -26,7 +26,7 @@ scoped_ptr<VideoCaptureDevice> FakeVideoCaptureDeviceFactory::Create(
   FakeVideoCaptureDevice::FakeVideoCaptureDeviceType fake_vcd_type;
   if (option.empty())
     fake_vcd_type = FakeVideoCaptureDevice::USING_OWN_BUFFERS;
-  else if (base:: strcasecmp(option.c_str(), "triplanar") == 0)
+  else if (base::EqualsCaseInsensitiveASCII(option, "triplanar"))
     fake_vcd_type = FakeVideoCaptureDevice::USING_OWN_BUFFERS_TRIPLANAR;
   else
     fake_vcd_type = FakeVideoCaptureDevice::USING_CLIENT_BUFFERS;

mojo/services/network/http_connection_impl.cc

@@ -375,11 +375,12 @@ void HttpConnectionImpl::OnFinishedReadingResponseBody(
       //
       // TODO(yzshen): Consider adding to net::HttpServerResponseInfo a simple
       // setter for body which doesn't fiddle with headers.
-      if (base::strcasecmp(header.name.data(),
-                           net::HttpRequestHeaders::kContentLength) == 0) {
+      base::StringPiece name_piece(header.name.data(), header.name.size());
+      if (base::EqualsCaseInsensitiveASCII(
+              name_piece, net::HttpRequestHeaders::kContentLength)) {
         continue;
-      } else if (base::strcasecmp(header.name.data(),
-                                  net::HttpRequestHeaders::kContentType) == 0) {
+      } else if (base::EqualsCaseInsensitiveASCII(
+                     name_piece, net::HttpRequestHeaders::kContentType)) {
         content_type = header.value;
         continue;
       }