Include string annotation objects when uploading crash reports.

This extracts string annotation objects from the minidumps and includes
them as form POST key-value pairs.

This change also starts building a crashpad_handler_test binary on Mac.

Bug: crashpad:192
Change-Id: I68cbf6fda6f1e57c1e621d5e3de8717cfaea65bf
Reviewed-on: https://chromium-review.googlesource.com/749793
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

build/run_tests.py

@@ -46,16 +46,13 @@ def main(args):
 
   tests = [
       'crashpad_client_test',
+      'crashpad_handler_test',
       'crashpad_minidump_test',
       'crashpad_snapshot_test',
       'crashpad_test_test',
       'crashpad_util_test',
   ]
 
-  if sys.platform == 'win32':
-    tests.append('crashpad_handler_test')
-    tests = sorted(tests)
-
   for test in tests:
     print '-' * 80
     print test

handler/crash_report_upload_thread.cc

@@ -27,6 +27,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "client/settings.h"
+#include "handler/minidump_to_upload_parameters.h"
 #include "snapshot/minidump/process_snapshot_minidump.h"
 #include "snapshot/module_snapshot.h"
 #include "util/file/file_reader.h"
@@ -46,68 +47,6 @@ namespace crashpad {
 
 namespace {
 
-void InsertOrReplaceMapEntry(std::map<std::string, std::string>* map,
-                             const std::string& key,
-                             const std::string& value) {
-  std::string old_value;
-  if (!MapInsertOrReplace(map, key, value, &old_value)) {
-    LOG(WARNING) << "duplicate key " << key << ", discarding value "
-                 << old_value;
-  }
-}
-
-// Given a minidump file readable by |minidump_file_reader|, returns a map of
-// key-value pairs to use as HTTP form parameters for upload to a Breakpad
-// server. The map is built by combining the process simple annotations map with
-// each module’s simple annotations map. In the case of duplicate keys, the map
-// will retain the first value found for any key, and will log a warning about
-// discarded values. Each module’s annotations vector is also examined and built
-// into a single string value, with distinct elements separated by newlines, and
-// stored at the key named “list_annotations”, which supersedes any other key
-// found by that name. The client ID stored in the minidump is converted to
-// a string and stored at the key named “guid”, which supersedes any other key
-// found by that name.
-//
-// In the event of an error reading the minidump file, a message will be logged.
-std::map<std::string, std::string> BreakpadHTTPFormParametersFromMinidump(
-    FileReaderInterface* minidump_file_reader) {
-  ProcessSnapshotMinidump minidump_process_snapshot;
-  if (!minidump_process_snapshot.Initialize(minidump_file_reader)) {
-    return std::map<std::string, std::string>();
-  }
-
-  std::map<std::string, std::string> parameters =
-      minidump_process_snapshot.AnnotationsSimpleMap();
-
-  std::string list_annotations;
-  for (const ModuleSnapshot* module : minidump_process_snapshot.Modules()) {
-    for (const auto& kv : module->AnnotationsSimpleMap()) {
-      if (!parameters.insert(kv).second) {
-        LOG(WARNING) << "duplicate key " << kv.first << ", discarding value "
-                     << kv.second;
-      }
-    }
-
-    for (std::string annotation : module->AnnotationsVector()) {
-      list_annotations.append(annotation);
-      list_annotations.append("\n");
-    }
-  }
-
-  if (!list_annotations.empty()) {
-    // Remove the final newline character.
-    list_annotations.resize(list_annotations.size() - 1);
-
-    InsertOrReplaceMapEntry(&parameters, "list_annotations", list_annotations);
-  }
-
-  UUID client_id;
-  minidump_process_snapshot.ClientID(&client_id);
-  InsertOrReplaceMapEntry(&parameters, "guid", client_id.ToString());
-
-  return parameters;
-}
-
 // Calls CrashReportDatabase::RecordUploadAttempt() with |successful| set to
 // false upon destruction unless disarmed by calling Fire() or Disarm(). Fire()
 // triggers an immediate call. Armed upon construction.
@@ -359,7 +298,11 @@ CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport(
   // minidump file. This may result in its being uploaded with few or no
   // parameters, but as long as there’s a dump file, the server can decide what
   // to do with it.
-  parameters = BreakpadHTTPFormParametersFromMinidump(&minidump_file_reader);
+  ProcessSnapshotMinidump minidump_process_snapshot;
+  if (minidump_process_snapshot.Initialize(&minidump_file_reader)) {
+    parameters =
+        BreakpadHTTPFormParametersFromMinidump(&minidump_process_snapshot);
+  }
 
   if (!minidump_file_reader.SeekSet(start_offset)) {
     return UploadResult::kPermanentFailure;

handler/handler.gyp

@@ -45,6 +45,8 @@
         'mac/exception_handler_server.h',
         'mac/file_limit_annotation.cc',
         'mac/file_limit_annotation.h',
+        'minidump_to_upload_parameters.cc',
+        'minidump_to_upload_parameters.h',
         'prune_crash_reports_thread.cc',
         'prune_crash_reports_thread.h',
         'user_stream_data_source.cc',

handler/handler_test.gyp

@@ -17,6 +17,40 @@
     '../build/crashpad.gypi',
   ],
   'targets': [
+    {
+      'target_name': 'crashpad_handler_test',
+      'type': 'executable',
+      'dependencies': [
+        'crashpad_handler_test_extended_handler',
+        'handler.gyp:crashpad_handler_lib',
+        '../client/client.gyp:crashpad_client',
+        '../compat/compat.gyp:crashpad_compat',
+        '../snapshot/snapshot.gyp:crashpad_snapshot',
+        '../snapshot/snapshot_test.gyp:crashpad_snapshot_test_lib',
+        '../test/test.gyp:crashpad_gtest_main',
+        '../test/test.gyp:crashpad_test',
+        '../third_party/gtest/gtest.gyp:gtest',
+        '../third_party/mini_chromium/mini_chromium.gyp:base',
+        '../util/util.gyp:crashpad_util',
+      ],
+      'include_dirs': [
+        '..',
+      ],
+      'sources': [
+        'crashpad_handler_test.cc',
+        'minidump_to_upload_parameters_test.cc',
+      ],
+      'conditions': [
+        ['OS!="win"', {
+          'dependencies!': [
+            'crashpad_handler_test_extended_handler',
+          ],
+          'sources!': [
+            'crashpad_handler_test.cc',
+          ],
+        }],
+      ],
+    },
     {
       'target_name': 'crashpad_handler_test_extended_handler',
       'type': 'executable',
@@ -52,28 +86,6 @@
             'win/crash_other_program.cc',
           ],
         },
-        {
-          # The handler is only tested on Windows for now.
-          'target_name': 'crashpad_handler_test',
-          'type': 'executable',
-          'dependencies': [
-            'crashpad_handler_test_extended_handler',
-            'handler.gyp:crashpad_handler_lib',
-            '../client/client.gyp:crashpad_client',
-            '../compat/compat.gyp:crashpad_compat',
-            '../test/test.gyp:crashpad_gtest_main',
-            '../test/test.gyp:crashpad_test',
-            '../third_party/gtest/gtest.gyp:gtest',
-            '../third_party/mini_chromium/mini_chromium.gyp:base',
-            '../util/util.gyp:crashpad_util',
-          ],
-          'include_dirs': [
-            '..',
-          ],
-          'sources': [
-            'crashpad_handler_test.cc',
-          ],
-        },
         {
           'target_name': 'crashy_program',
           'type': 'executable',

handler/minidump_to_upload_parameters.cc

@@ -0,0 +1,86 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "handler/minidump_to_upload_parameters.h"
+
+#include "base/logging.h"
+#include "client/annotation.h"
+#include "snapshot/module_snapshot.h"
+#include "util/stdlib/map_insert.h"
+
+namespace crashpad {
+
+namespace {
+
+void InsertOrReplaceMapEntry(std::map<std::string, std::string>* map,
+                             const std::string& key,
+                             const std::string& value) {
+  std::string old_value;
+  if (!MapInsertOrReplace(map, key, value, &old_value)) {
+    LOG(WARNING) << "duplicate key " << key << ", discarding value "
+                 << old_value;
+  }
+}
+
+}  // namespace
+
+std::map<std::string, std::string> BreakpadHTTPFormParametersFromMinidump(
+    const ProcessSnapshot* process_snapshot) {
+  std::map<std::string, std::string> parameters =
+      process_snapshot->AnnotationsSimpleMap();
+
+  std::string list_annotations;
+  for (const ModuleSnapshot* module : process_snapshot->Modules()) {
+    for (const auto& kv : module->AnnotationsSimpleMap()) {
+      if (!parameters.insert(kv).second) {
+        LOG(WARNING) << "duplicate key " << kv.first << ", discarding value "
+                     << kv.second;
+      }
+    }
+
+    for (std::string annotation : module->AnnotationsVector()) {
+      list_annotations.append(annotation);
+      list_annotations.append("\n");
+    }
+
+    for (const AnnotationSnapshot& annotation : module->AnnotationObjects()) {
+      if (annotation.type != static_cast<uint16_t>(Annotation::Type::kString)) {
+        continue;
+      }
+
+      std::string value(reinterpret_cast<const char*>(annotation.value.data()),
+                        annotation.value.size());
+      std::pair<std::string, std::string> entry(annotation.name, value);
+      if (!parameters.insert(entry).second) {
+        LOG(WARNING) << "duplicate annotation name " << annotation.name
+                     << ", discarding value " << value;
+      }
+    }
+  }
+
+  if (!list_annotations.empty()) {
+    // Remove the final newline character.
+    list_annotations.resize(list_annotations.size() - 1);
+
+    InsertOrReplaceMapEntry(&parameters, "list_annotations", list_annotations);
+  }
+
+  UUID client_id;
+  process_snapshot->ClientID(&client_id);
+  InsertOrReplaceMapEntry(&parameters, "guid", client_id.ToString());
+
+  return parameters;
+}
+
+}  // namespace crashpad

handler/minidump_to_upload_parameters.h

@@ -0,0 +1,61 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef HANDLER_MINIDUMP_TO_UPLOAD_PARAMETERS_H_
+#define HANDLER_MINIDUMP_TO_UPLOAD_PARAMETERS_H_
+
+#include <map>
+#include <string>
+
+#include "snapshot/process_snapshot.h"
+
+namespace crashpad {
+
+//! \brief Given a ProcessSnapshot, returns a map of key-value pairs to use as
+//!     HTTP form parameters for upload to a Breakpad crash report colleciton
+//!     server.
+//!
+//! The map is built by combining the process simple annotations map with
+//! each module’s simple annotations map and annotation objects.
+//!
+//! In the case of duplicate simple map keys or annotation names, the map will
+//! retain the first value found for any key, and will log a warning about
+//! discarded values. The precedence rules for annotation names are: the two
+//! reserved keys discussed below, process simple annotations, module simple
+//! annotations, and module annotation objects.
+//!
+//! For annotation objects, only ones of that are Annotation::Type::kString are
+//! included.
+//!
+//! Each module’s annotations vector is also examined and built into a single
+//! string value, with distinct elements separated by newlines, and stored at
+//! the key named “list_annotations”, which supersedes any other key found by
+//! that name.
+//!
+//! The client ID stored in the minidump is converted to a string and stored at
+//! the key named “guid”, which supersedes any other key found by that name.
+//!
+//! In the event of an error reading the minidump file, a message will be
+//! logged.
+//!
+//! \param[in] process_snapshot The process snapshot from which annotations
+//!     will be extracted.
+//!
+//! \returns A string map of the annotations.
+std::map<std::string, std::string> BreakpadHTTPFormParametersFromMinidump(
+    const ProcessSnapshot* process_snapshot);
+
+}  // namespace crashpad
+
+#endif  // HANDLER_MINIDUMP_TO_UPLOAD_PARAMETERS_H_

handler/minidump_to_upload_parameters_test.cc

@@ -0,0 +1,99 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "handler/minidump_to_upload_parameters.h"
+
+#include "gtest/gtest.h"
+#include "snapshot/test/test_module_snapshot.h"
+#include "snapshot/test/test_process_snapshot.h"
+#include "util/misc/uuid.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+TEST(MinidumpToUploadParameters, PrecedenceRules) {
+  const std::string guid = "00112233-4455-6677-8899-aabbccddeeff";
+  UUID uuid;
+  ASSERT_TRUE(uuid.InitializeFromString(guid));
+
+  TestProcessSnapshot process_snapshot;
+  process_snapshot.SetClientID(uuid);
+  process_snapshot.SetAnnotationsSimpleMap({
+      {"process-1", "abcdefg"},
+      {"list_annotations", "BAD: process_annotations"},
+      {"guid", "BAD: process_annotations"},
+      {"first", "process"},
+  });
+
+  auto module_snapshot_0 = std::make_unique<TestModuleSnapshot>();
+  module_snapshot_0->SetAnnotationsVector(
+      {"list-module-0-1", "list-module-0-2"});
+  module_snapshot_0->SetAnnotationsSimpleMap({
+      {"module-0-1", "goat"},
+      {"module-0-2", "doge"},
+      {"list_annotations", "BAD: module 0"},
+      {"guid", "BAD: module 0"},
+      {"first", "BAD: module 0"},
+      {"second", "module 0"},
+  });
+  module_snapshot_0->SetAnnotationObjects({
+      {"module-0-3", 1, {'s', 't', 'a', 'r'}},
+      {"module-0-4", 0xFFFA, {0x42}},
+      {"guid", 1, {'B', 'A', 'D', '*', '0', '-', '0'}},
+      {"list_annotations", 1, {'B', 'A', 'D', '*', '0', '-', '1'}},
+      {"first", 1, {'B', 'A', 'D', '*', '0', '-', '2'}},
+  });
+  process_snapshot.AddModule(std::move(module_snapshot_0));
+
+  auto module_snapshot_1 = std::make_unique<TestModuleSnapshot>();
+  module_snapshot_1->SetAnnotationsVector(
+      {"list-module-1-1", "list-module-1-2"});
+  module_snapshot_1->SetAnnotationsSimpleMap({
+      {"module-1-1", "bear"},
+      {"list_annotations", "BAD: module 1"},
+      {"guid", "BAD: module 1"},
+      {"first", "BAD: module 1"},
+      {"second", "BAD: module 1"},
+  });
+  module_snapshot_1->SetAnnotationObjects({
+      {"module-1-3", 0xBEEF, {'a', 'b', 'c'}},
+      {"module-1-4", 1, {'m', 'o', 'o', 'n'}},
+      {"guid", 1, {'B', 'A', 'D', '*', '1', '-', '0'}},
+      {"list_annotations", 1, {'B', 'A', 'D', '*', '1', '-', '1'}},
+      {"second", 1, {'B', 'A', 'D', '*', '1', '-', '2'}},
+  });
+  process_snapshot.AddModule(std::move(module_snapshot_1));
+
+  auto upload_parameters =
+      BreakpadHTTPFormParametersFromMinidump(&process_snapshot);
+
+  EXPECT_EQ(upload_parameters.size(), 10u);
+  EXPECT_EQ(upload_parameters["process-1"], "abcdefg");
+  EXPECT_EQ(upload_parameters["first"], "process");
+  EXPECT_EQ(upload_parameters["module-0-1"], "goat");
+  EXPECT_EQ(upload_parameters["module-0-2"], "doge");
+  EXPECT_EQ(upload_parameters["module-0-3"], "star");
+  EXPECT_EQ(upload_parameters["second"], "module 0");
+  EXPECT_EQ(upload_parameters["module-1-1"], "bear");
+  EXPECT_EQ(upload_parameters["module-1-4"], "moon");
+  EXPECT_EQ(upload_parameters["list_annotations"],
+            "list-module-0-1\nlist-module-0-2\n"
+            "list-module-1-1\nlist-module-1-2");
+  EXPECT_EQ(upload_parameters["guid"], guid);
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad