Fix some bugs in the handling of dynamic crash keys.

chrome/app/breakpad_win.cc

Index: chrome/app/breakpad_win.cc
diff --git a/chrome/app/breakpad_win.cc b/chrome/app/breakpad_win.cc
index e5e7be2e580216ff91c2f10fc80cf9ad3e59457b..990e67bfcb32e513a609b5115dc3ed14092c297d 100644
--- a/chrome/app/breakpad_win.cc
+++ b/chrome/app/breakpad_win.cc
@@ -759,12 +759,24 @@ extern "C" void __declspec(dllexport) __cdecl SetNumberOfViews(
 
 void SetCrashKeyValue(const base::StringPiece& key,
                       const base::StringPiece& value) {
-  std::string key_string = key.as_string();
-
+  // CustomInfoEntry limits the length of key and value. If they exceed
+  // their maximum length the underlying string handling functions raise
+  // an exception and prematurely trigger a crash. Truncate here.
+  base::StringPiece safe_key(key.substr(
+      0, google_breakpad::CustomInfoEntry::kNameMaxLength  - 1));
+  base::StringPiece safe_value(value.substr(
+      0, google_breakpad::CustomInfoEntry::kValueMaxLength - 1));

[Robert Sesek, 2013/04/16 17:54:29]

The crash_logging system should be doing this truncation for you. Though I
wouldn't object to doing it here as well for extra sanity.

[Roger McFarlane (Chromium), 2013/04/16 18:18:23]

What I've observed is that if you get down to entry->set(...) below and the
strings don't fit into the space allocated in CustomInfoEntry then you get a
crash because:

  - CustomInfoEntry::set* calls WindowsStringUtils::safe_wcscpy
  - WindowsStringUtils::safe_wcscpy calls wcscpy_s
  - wcscpy_s invokes the invalid parameter handler
  - the invalid parameter handler calls back into breakpad
  - breakpad triggers a crash

+
+  // Keep a copy of the safe key as a std::string, we'll reuse it later.
+  std::string key_string(safe_key.begin(), safe_key.end());
+
+  // If we already have a value for this key, update it; otherwise, insert
+  // the new value if we have not exhausted the pre-allocated slots for dynamic
+  // entries.
   DynamicEntriesMap::iterator it = g_dynamic_entries->find(key_string);
   google_breakpad::CustomInfoEntry* entry = NULL;
   if (it == g_dynamic_entries->end()) {
-    if (g_dynamic_keys_offset >= g_dynamic_entries_count)
+    if (g_dynamic_entries->size() >= g_dynamic_entries_count)
       return;
     entry = &(*g_custom_entries)[g_dynamic_keys_offset++];
     g_dynamic_entries->insert(std::make_pair(key_string, entry));
@@ -772,7 +784,7 @@ void SetCrashKeyValue(const base::StringPiece& key,
     entry = it->second;
   }
 
-  entry->set(UTF8ToWide(key).data(), UTF8ToWide(value).data());
+  entry->set(UTF8ToWide(safe_key).data(), UTF8ToWide(safe_value).data());
 }
 
 extern "C" void __declspec(dllexport) __cdecl SetCrashKeyValuePair(