Fix some bugs in the handling of dynamic crash keys.

Fix some bugs in the handling of dynamic crash keys.

- The check to see if the dynamic key slots were all in use
  was comparing the wrong value.
  
- The lengths of the key and value being set wasn't being
  validated. The lower-level code capturing the values
  subsequently terminate the process if a key or value is
  too long for the CustomInfoEntry record.
  
R= cpu@chromium.org, rsesek@chromium.org
BUG=77656
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195196

[Roger McFarlane (Chromium), 2013-04-16 04:15:13]

PTAL?

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

https://codereview.chromium.org/14262014/diff/2001/chrome/app/breakpad_win.cc
File chrome/app/breakpad_win.cc (right):

https://codereview.chromium.org/14262014/diff/2001/chrome/app/breakpad_win.cc...
chrome/app/breakpad_win.cc:768: 0,
google_breakpad::CustomInfoEntry::kValueMaxLength - 1));
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]

https://codereview.chromium.org/14262014/diff/2001/chrome/app/breakpad_win.cc
File chrome/app/breakpad_win.cc (right):

https://codereview.chromium.org/14262014/diff/2001/chrome/app/breakpad_win.cc...
chrome/app/breakpad_win.cc:768: 0,
google_breakpad::CustomInfoEntry::kValueMaxLength - 1));
On 2013/04/16 17:54:29, rsesek wrote:
> 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.


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

[Roger McFarlane (Chromium), 2013-04-17 14:59:58]

ping?

[Robert Sesek, 2013-04-17 15:40:18]

lgtm

[commit-bot: I haz the power, 2013-04-18 15:03:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rogerm@chromium.org/14262014/2001

[commit-bot: I haz the power, 2013-04-18 15:03:42]

Presubmit check for 14262014-2001 failed and returned exit status 1.

INFO:root:Found 1 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/app/breakpad_win.cc

[Roger McFarlane (Chromium), 2013-04-18 15:09:39]

Hey Eric,

I think Carlos is nominating you to be an OWNER in this directory. But from what
I see, you're not yet in the OWNERS file, modulo sync and codesearch update
latencies.

[cpu_(ooo_6.6-7.5), 2013-04-18 17:33:05]

lgtm,

I still want the opinion of eroman since my brain tells me added the routines
that raise the exception.

[eroman, 2013-04-18 21:04:43]

lgtm

[commit-bot: I haz the power, 2013-04-18 21:19:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rogerm@chromium.org/14262014/2001

[commit-bot: I haz the power, 2013-04-19 14:57:14]

Change committed as 195196