From 2eb356a68d981b87329cdbaf1ce46380b2362296 Mon Sep 17 00:00:00 2001 From: nealsid Date: Wed, 3 Jun 2009 21:51:33 +0000 Subject: Support custom URL parameters. Added unit tests for Breakpad. Added a way to specify server parameters in app plist file, as well. R=stuartmorgan, jeremy A=nealsid git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@346 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/mac/Breakpad.xcodeproj/project.pbxproj | 34 ++-- src/client/mac/Framework/Breakpad.h | 110 +++++++---- src/client/mac/Framework/Breakpad.mm | 106 ++++++++--- src/client/mac/sender/crash_report_sender.h | 21 +-- src/client/mac/sender/crash_report_sender.m | 75 ++++++-- src/client/mac/testapp/Controller.m | 7 +- src/client/mac/testapp/Info.plist | 7 + src/client/mac/tests/BreakpadFramework_Test.mm | 216 ++++++++++++++++++++++ 8 files changed, 481 insertions(+), 95 deletions(-) create mode 100644 src/client/mac/tests/BreakpadFramework_Test.mm (limited to 'src') diff --git a/src/client/mac/Breakpad.xcodeproj/project.pbxproj b/src/client/mac/Breakpad.xcodeproj/project.pbxproj index 7d8e5b8d..97659d32 100644 --- a/src/client/mac/Breakpad.xcodeproj/project.pbxproj +++ b/src/client/mac/Breakpad.xcodeproj/project.pbxproj @@ -30,6 +30,8 @@ 33880C800F9E097100817F82 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 33880C7E0F9E097100817F82 /* InfoPlist.strings */; }; 4084699D0F5D9CF900FDCA37 /* crash_report_sender.icns in Resources */ = {isa = PBXBuildFile; fileRef = 4084699C0F5D9CF900FDCA37 /* crash_report_sender.icns */; }; 8DC2EF570486A6940098B216 /* Cocoa.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 1058C7B1FEA5585E11CA2CBB /* Cocoa.framework */; }; + F91AF5D00FD60393009D8BE2 /* BreakpadFramework_Test.mm in Sources */ = {isa = PBXBuildFile; fileRef = F91AF5CF0FD60393009D8BE2 /* BreakpadFramework_Test.mm */; }; + F91AF6210FD60784009D8BE2 /* Breakpad.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 8DC2EF5B0486A6940098B216 /* Breakpad.framework */; }; F9286B3A0F7EB25800A4DCC8 /* InspectorMain.mm in Sources */ = {isa = PBXBuildFile; fileRef = F9286B390F7EB25800A4DCC8 /* InspectorMain.mm */; }; F92C53B80ECCE7B3009BE4BA /* Inspector.mm in Sources */ = {isa = PBXBuildFile; fileRef = F92C53B70ECCE7B3009BE4BA /* Inspector.mm */; }; F92C554C0ECCF534009BE4BA /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0867D69BFE84028FC02AAC07 /* Foundation.framework */; }; @@ -105,6 +107,13 @@ /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ + F91AF6370FD60A74009D8BE2 /* PBXContainerItemProxy */ = { + isa = PBXContainerItemProxy; + containerPortal = 0867D690FE84028FC02AAC07 /* Project object */; + proxyType = 1; + remoteGlobalIDString = 8DC2EF4F0486A6940098B216; + remoteInfo = Breakpad; + }; F92C564D0ECD10E5009BE4BA /* PBXContainerItemProxy */ = { isa = PBXContainerItemProxy; containerPortal = 0867D690FE84028FC02AAC07 /* Project object */; @@ -133,13 +142,6 @@ remoteGlobalIDString = F93803BD0F80820F004D428B; remoteInfo = generator_test; }; - F93DE2FD0F82C3C900608B94 /* PBXContainerItemProxy */ = { - isa = PBXContainerItemProxy; - containerPortal = 0867D690FE84028FC02AAC07 /* Project object */; - proxyType = 1; - remoteGlobalIDString = F93DE2D00F82A67300608B94; - remoteInfo = minidump_file_writer_unittest; - }; F93DE36F0F82CC1300608B94 /* PBXContainerItemProxy */ = { isa = PBXContainerItemProxy; containerPortal = 0867D690FE84028FC02AAC07 /* Project object */; @@ -228,6 +230,7 @@ 33880C7F0F9E097100817F82 /* English */ = {isa = PBXFileReference; fileEncoding = 10; lastKnownFileType = text.plist.strings; name = English; path = sender/English.lproj/InfoPlist.strings; sourceTree = ""; }; 4084699C0F5D9CF900FDCA37 /* crash_report_sender.icns */ = {isa = PBXFileReference; lastKnownFileType = image.icns; name = crash_report_sender.icns; path = sender/crash_report_sender.icns; sourceTree = ""; }; 8DC2EF5B0486A6940098B216 /* Breakpad.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Breakpad.framework; sourceTree = BUILT_PRODUCTS_DIR; }; + F91AF5CF0FD60393009D8BE2 /* BreakpadFramework_Test.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = BreakpadFramework_Test.mm; path = tests/BreakpadFramework_Test.mm; sourceTree = ""; }; F9286B380F7EB25800A4DCC8 /* Inspector.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = Inspector.h; path = crash_generation/Inspector.h; sourceTree = ""; }; F9286B390F7EB25800A4DCC8 /* InspectorMain.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = InspectorMain.mm; path = crash_generation/InspectorMain.mm; sourceTree = ""; }; F92C53540ECCE349009BE4BA /* Inspector */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = Inspector; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -375,6 +378,7 @@ isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; files = ( + F91AF6210FD60784009D8BE2 /* Breakpad.framework in Frameworks */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -564,6 +568,7 @@ children = ( F9C77DE00F7DD7E30045F7DB /* SimpleStringDictionaryTest.h */, F9C77DE10F7DD7E30045F7DB /* SimpleStringDictionaryTest.mm */, + F91AF5CF0FD60393009D8BE2 /* BreakpadFramework_Test.mm */, ); name = tests; sourceTree = ""; @@ -754,8 +759,8 @@ ); dependencies = ( F93DE2FC0F82C3C600608B94 /* PBXTargetDependency */, - F93DE2FE0F82C3C900608B94 /* PBXTargetDependency */, F93DE3700F82CC1300608B94 /* PBXTargetDependency */, + F91AF6380FD60A74009D8BE2 /* PBXTargetDependency */, ); name = UnitTests; productName = UnitTests; @@ -871,7 +876,7 @@ ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/sh; - shellScript = "# Run the unit tests in this test bundle.\n\"${SYSTEM_DEVELOPER_DIR}/Tools/RunUnitTests\"\n\necho running minidump generator tests...\n\"${BUILT_PRODUCTS_DIR}/generator_test\"\necho Running minidump file writer tests...\n\"${BUILT_PRODUCTS_DIR}/minidump_file_writer_unittest\"\necho Running exception handler tests...\n\"${BUILT_PRODUCTS_DIR}/handler_test\"\n"; + shellScript = "# Run the unit tests in this test bundle.\n\"${SYSTEM_DEVELOPER_DIR}/Tools/RunUnitTests\"\n\necho running minidump generator tests...\n\"${BUILT_PRODUCTS_DIR}/generator_test\"\necho Running exception handler tests...\n\"${BUILT_PRODUCTS_DIR}/handler_test\"\n"; }; /* End PBXShellScriptBuildPhase section */ @@ -991,12 +996,18 @@ F9C77DE40F7DD82F0045F7DB /* SimpleStringDictionary.mm in Sources */, F9C77DE20F7DD7E30045F7DB /* SimpleStringDictionaryTest.mm in Sources */, F9C77E130F7DDF810045F7DB /* GTMSenTestCase.m in Sources */, + F91AF5D00FD60393009D8BE2 /* BreakpadFramework_Test.mm in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; /* End PBXSourcesBuildPhase section */ /* Begin PBXTargetDependency section */ + F91AF6380FD60A74009D8BE2 /* PBXTargetDependency */ = { + isa = PBXTargetDependency; + target = 8DC2EF4F0486A6940098B216 /* Breakpad */; + targetProxy = F91AF6370FD60A74009D8BE2 /* PBXContainerItemProxy */; + }; F92C564E0ECD10E5009BE4BA /* PBXTargetDependency */ = { isa = PBXTargetDependency; target = F92C563B0ECD10B3009BE4BA /* breakpadUtilities */; @@ -1017,11 +1028,6 @@ target = F93803BD0F80820F004D428B /* generator_test */; targetProxy = F93DE2FB0F82C3C600608B94 /* PBXContainerItemProxy */; }; - F93DE2FE0F82C3C900608B94 /* PBXTargetDependency */ = { - isa = PBXTargetDependency; - target = F93DE2D00F82A67300608B94 /* minidump_file_writer_unittest */; - targetProxy = F93DE2FD0F82C3C900608B94 /* PBXContainerItemProxy */; - }; F93DE3700F82CC1300608B94 /* PBXTargetDependency */ = { isa = PBXTargetDependency; target = F93DE32B0F82C55600608B94 /* handler_test */; diff --git a/src/client/mac/Framework/Breakpad.h b/src/client/mac/Framework/Breakpad.h index 88be8162..ab4d6220 100644 --- a/src/client/mac/Framework/Breakpad.h +++ b/src/client/mac/Framework/Breakpad.h @@ -81,15 +81,15 @@ extern "C" { #define BREAKPAD_REQUEST_EMAIL "BreakpadRequestEmail" #define BREAKPAD_EMAIL "BreakpadEmail" #define BREAKPAD_SERVER_TYPE "BreakpadServerType" -// TODO(nealsid) find a better way to support server-specific -// parameters without having to rebuild Breakpad -#define BREAKPAD_BUILD_ID "BreakpadBuildID" +#define BREAKPAD_SERVER_PARAMETER_DICT "BreakpadServerParameters" // The keys below are NOT user supplied, and are used internally. -#define BREAKPAD_PROCESS_START_TIME "BreakpadProcStartTime" -#define BREAKPAD_PROCESS_UP_TIME "BreakpadProcessUpTime" -#define BREAKPAD_PROCESS_CRASH_TIME "BreakpadProcessCrashTime" -#define BREAKPAD_LOGFILE_KEY_PREFIX "BreakpadAppLogFile" +#define BREAKPAD_PROCESS_START_TIME "BreakpadProcStartTime" +#define BREAKPAD_PROCESS_UP_TIME "BreakpadProcessUpTime" +#define BREAKPAD_PROCESS_CRASH_TIME "BreakpadProcessCrashTime" +#define BREAKPAD_LOGFILE_KEY_PREFIX "BreakpadAppLogFile" +#define BREAKPAD_SERVER_PARAMETER_PREFIX "BreakpadServerParameterPrefix_" + // Optional user-defined function to dec to decide if we should handle // this crash or forward it along. // Return true if you want Breakpad to handle it. @@ -98,7 +98,8 @@ extern "C" { // (which means the next exception handler will take the exception) typedef bool (*BreakpadFilterCallback)(int exception_type, int exception_code, - mach_port_t crashing_thread); + mach_port_t crashing_thread, + void *context); // Create a new BreakpadRef object and install it as an exception // handler. The |parameters| will typically be the contents of your @@ -179,8 +180,15 @@ typedef bool (*BreakpadFilterCallback)(int exception_type, // other types, see the function in // crash_report_sender.m that maps parameters to // URL parameters. Defaults to 'google'. -// BREAKPAD_BUILD_ID A string parameter indicating build id. -// Optional. +// +// BREAKPAD_SERVER_PARAMETER_DICT A plist dictionary of static +// parameters that are uploaded to the +// server. The parameters are sent as +// is to the crash server. Their +// content isn't added to the minidump +// but pass as URL parameters when +// uploading theminidump to the crash +// server. //============================================================================= // The BREAKPAD_PRODUCT, BREAKPAD_VERSION and BREAKPAD_URL are // required to have non-NULL values. By default, the BREAKPAD_PRODUCT @@ -209,17 +217,31 @@ typedef bool (*BreakpadFilterCallback)(int exception_type, // completeness. They are calculated by Breakpad during initialization & // crash-dump generation. // -// BREAKPAD_PROCESS_START_TIME The time the process started. +// BREAKPAD_PROCESS_START_TIME The time the process started. +// +// BREAKPAD_PROCESS_CRASH_TIME The time the process crashed. +// +// BREAKPAD_PROCESS_UP_TIME The total time the process has been +// running. This parameter is not set +// until the crash-dump-generation phase. // -// BREAKPAD_PROCESS_CRASH_TIME The time the process crashed. +// BREAKPAD_LOGFILE_KEY_PREFIX Used to find out which parameters in the +// parameter dictionary correspond to log +// file paths. // -// BREAKPAD_PROCESS_UP_TIME The total time the process has been running. -// This parameter is not set until the -// crash-dump-generation phase. +// BREAKPAD_SERVER_PARAMETER_PREFIX This prefix is used by Breakpad +// internally, because Breakpad uses +// the same dictionary internally to +// track both its internal +// configuration parameters and +// parameters meant to be uploaded +// to the server. This string is +// used internally by Breakpad to +// prefix user-supplied parameter +// names so those can be sent to the +// server without leaking Breakpad's +// internal values. // -// BREAKPAD_LOGFILE_KEY_PREFIX Used to find out which parameters in the -// parameter dictionary correspond to log file -// paths. // Returns a new BreakpadRef object on success, NULL otherwise. BreakpadRef BreakpadCreate(NSDictionary *parameters); @@ -227,27 +249,49 @@ BreakpadRef BreakpadCreate(NSDictionary *parameters); // Uninstall and release the data associated with |ref|. void BreakpadRelease(BreakpadRef ref); -// Clients may set an optional callback which gets called when a crash occurs. -// The callback function should return |true| if we should handle the crash, -// generate a crash report, etc. or |false| if we should ignore it and forward -// the crash (normally to CrashReporter) +// Clients may set an optional callback which gets called when a crash +// occurs. The callback function should return |true| if we should +// handle the crash, generate a crash report, etc. or |false| if we +// should ignore it and forward the crash (normally to CrashReporter). +// Context is a pointer to arbitrary data to make the callback with. void BreakpadSetFilterCallback(BreakpadRef ref, - BreakpadFilterCallback callback); + BreakpadFilterCallback callback, + void *context); -// User defined key and value string storage -// All set keys will be uploaded with the minidump if a crash occurs -// Keys and Values are limited to 255 bytes (256 - 1 for terminator). -// NB this is BYTES not GLYPHS. -// Anything longer than 255 bytes will be truncated. Note that the string is -// converted to UTF8 before truncation, so any multibyte character that -// straddles the 255 byte limit will be mangled. -// -// A maximum number of 64 key/value pairs are supported. An assert() will fire -// if more than this number are set. +// User defined key and value string storage. Generally this is used +// to configure Breakpad's internal operation, such as whether the +// crash_sender should prompt the user, or the filesystem location for +// the minidump file. See Breakpad.h for some parameters that can be +// set. Anything longer than 255 bytes will be truncated. Note that +// the string is converted to UTF8 before truncation, so any multibyte +// character that straddles the 255(256 - 1 for terminator) byte limit +// will be mangled. +// +// A maximum number of 64 key/value pairs are supported. An assert() +// will fire if more than this number are set. Unfortunately, right +// now, the same dictionary is used for both Breakpad's parameters AND +// the Upload parameters. +// +// TODO (nealsid): Investigate how necessary this is if we don't +// automatically upload parameters to the server anymore. +// TODO (nealsid): separate server parameter dictionary from the +// dictionary used to configure Breakpad, and document limits for each +// independently. void BreakpadSetKeyValue(BreakpadRef ref, NSString *key, NSString *value); NSString *BreakpadKeyValue(BreakpadRef ref, NSString *key); void BreakpadRemoveKeyValue(BreakpadRef ref, NSString *key); +// You can use this method to specify parameters that will be uploaded +// to the crash server. They will be automatically encoded as +// necessary. Note that as mentioned above there are limits on both +// the number of keys and their length. +void BreakpadAddUploadParameter(BreakpadRef ref, NSString *key, + NSString *value); + +// This method will remove a previously-added parameter from the +// upload parameter set. +void BreakpadRemoveUploadParameter(BreakpadRef ref, NSString *key); + // Add a log file for Breakpad to read and send upon crash dump void BreakpadAddLogFile(BreakpadRef ref, NSString *logPathname); diff --git a/src/client/mac/Framework/Breakpad.mm b/src/client/mac/Framework/Breakpad.mm index ad4a1cda..e2b87141 100644 --- a/src/client/mac/Framework/Breakpad.mm +++ b/src/client/mac/Framework/Breakpad.mm @@ -154,8 +154,9 @@ class Breakpad { void GenerateAndSendReport(); - void SetFilterCallback(BreakpadFilterCallback callback) { + void SetFilterCallback(BreakpadFilterCallback callback, void *context) { filter_callback_ = callback; + filter_callback_context_ = context; } private: @@ -163,7 +164,8 @@ class Breakpad { : handler_(NULL), config_params_(NULL), send_and_exit_(true), - filter_callback_(NULL) { + filter_callback_(NULL), + filter_callback_context_(NULL) { inspector_path_[0] = 0; } @@ -194,8 +196,7 @@ class Breakpad { bool send_and_exit_; // Exit after sending, if true BreakpadFilterCallback filter_callback_; - - unsigned int logFileCounter; + void *filter_callback_context_; }; #pragma mark - @@ -412,8 +413,10 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) { [parameters objectForKey:@BREAKPAD_VENDOR]; NSString *dumpSubdirectory = [parameters objectForKey:@BREAKPAD_DUMP_DIRECTORY]; - NSString *buildId = - [parameters objectForKey:@BREAKPAD_BUILD_ID]; + + NSDictionary *serverParameters = + [parameters objectForKey:@BREAKPAD_SERVER_PARAMETER_DICT]; + // These may have been set above as user prefs, which take priority. if (!skipConfirm) { skipConfirm = [parameters objectForKey:@BREAKPAD_SKIP_CONFIRM]; @@ -521,10 +524,19 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) { dumpSubdirectory = @""; } - // The product and version are required values. - if (![product length] || ![version length]) { - DEBUGLOG(stderr, - "Missing required product or version subdirectory keys\n"); + // The product, version, and URL are required values. + if (![product length]) { + DEBUGLOG(stderr, "Missing required product key.\n"); + return false; + } + + if (![version length]) { + DEBUGLOG(stderr, "Missing required version key.\n"); + return false; + } + + if (![urlStr length]) { + DEBUGLOG(stderr, "Missing required URL key.\n"); return false; } @@ -555,8 +567,6 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) { dictionary.SetKeyValue(BREAKPAD_DUMP_DIRECTORY, [dumpSubdirectory UTF8String]); - dictionary.SetKeyValue(BREAKPAD_BUILD_ID, - [buildId UTF8String]); struct timeval tv; gettimeofday(&tv, NULL); char timeStartedString[32]; @@ -579,6 +589,15 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) { [reportEmail UTF8String]); } + if (serverParameters) { + // For each key-value pair, call BreakpadAddUploadParameter() + NSEnumerator *keyEnumerator = [serverParameters keyEnumerator]; + NSString *aParameter; + while (aParameter = [keyEnumerator nextObject]) { + BreakpadAddUploadParameter(this, aParameter, + [serverParameters objectForKey:aParameter]); + } + } return true; } @@ -622,7 +641,8 @@ bool Breakpad::HandleException(int exception_type, if (filter_callback_) { bool should_handle = filter_callback_(exception_type, exception_code, - crashing_thread); + crashing_thread, + filter_callback_context_); if (!should_handle) return false; } @@ -769,7 +789,7 @@ BreakpadRef BreakpadCreate(NSDictionary *parameters) { [pool release]; return (BreakpadRef)breakpad; - } catch(...) { // don't let exception leave this C API + } catch(...) { // don't let exceptions leave this C API if (gKeyValueAllocator) { gKeyValueAllocator->~ProtectedMemoryAllocator(); gKeyValueAllocator = NULL; @@ -817,7 +837,7 @@ void BreakpadRelease(BreakpadRef ref) { pthread_mutex_destroy(&gDictionaryMutex); } - } catch(...) { // don't let exception leave this C API + } catch(...) { // don't let exceptions leave this C API fprintf(stderr, "BreakpadRelease() : error\n"); } } @@ -833,11 +853,51 @@ void BreakpadSetKeyValue(BreakpadRef ref, NSString *key, NSString *value) { breakpad->SetKeyValue(key, value); } - } catch(...) { // don't let exception leave this C API + } catch(...) { // don't let exceptions leave this C API + fprintf(stderr, "BreakpadSetKeyValue() : error\n"); + } +} + +void BreakpadAddUploadParameter(BreakpadRef ref, + NSString *key, + NSString *value) { + // The only difference, internally, between an upload parameter and + // a key value one that is set with BreakpadSetKeyValue is that we + // prepend the keyname with a special prefix. This informs the + // crash sender that the parameter should be sent along with the + // POST of the crash dump upload. + try { + Breakpad *breakpad = (Breakpad *)ref; + + if (breakpad && key && gKeyValueAllocator) { + ProtectedMemoryLocker locker(&gDictionaryMutex, gKeyValueAllocator); + + NSString *prefixedKey = [@BREAKPAD_SERVER_PARAMETER_PREFIX + stringByAppendingString:key]; + breakpad->SetKeyValue(prefixedKey, value); + } + } catch(...) { // don't let exceptions leave this C API fprintf(stderr, "BreakpadSetKeyValue() : error\n"); } } +void BreakpadRemoveUploadParameter(BreakpadRef ref, + NSString *key) { + try { + // Not called at exception time + Breakpad *breakpad = (Breakpad *)ref; + + if (breakpad && key && gKeyValueAllocator) { + ProtectedMemoryLocker locker(&gDictionaryMutex, gKeyValueAllocator); + + NSString *prefixedKey = [NSString stringWithFormat:@"%@%@", + @BREAKPAD_SERVER_PARAMETER_PREFIX, key]; + breakpad->RemoveKeyValue(prefixedKey); + } + } catch(...) { // don't let exceptions leave this C API + fprintf(stderr, "BreakpadRemoveKeyValue() : error\n"); + } +} //============================================================================= NSString *BreakpadKeyValue(BreakpadRef ref, NSString *key) { NSString *value = nil; @@ -852,7 +912,7 @@ NSString *BreakpadKeyValue(BreakpadRef ref, NSString *key) { ProtectedMemoryLocker locker(&gDictionaryMutex, gKeyValueAllocator); value = breakpad->KeyValue(key); - } catch(...) { // don't let exception leave this C API + } catch(...) { // don't let exceptions leave this C API fprintf(stderr, "BreakpadKeyValue() : error\n"); } @@ -870,7 +930,7 @@ void BreakpadRemoveKeyValue(BreakpadRef ref, NSString *key) { breakpad->RemoveKeyValue(key); } - } catch(...) { // don't let exception leave this C API + } catch(...) { // don't let exceptions leave this C API fprintf(stderr, "BreakpadRemoveKeyValue() : error\n"); } } @@ -887,14 +947,15 @@ void BreakpadGenerateAndSendReport(BreakpadRef ref) { breakpad->GenerateAndSendReport(); gBreakpadAllocator->Protect(); } - } catch(...) { // don't let exception leave this C API + } catch(...) { // don't let exceptions leave this C API fprintf(stderr, "BreakpadGenerateAndSendReport() : error\n"); } } //============================================================================= void BreakpadSetFilterCallback(BreakpadRef ref, - BreakpadFilterCallback callback) { + BreakpadFilterCallback callback, + void *context) { try { Breakpad *breakpad = (Breakpad *)ref; @@ -903,9 +964,9 @@ void BreakpadSetFilterCallback(BreakpadRef ref, // share the dictionary mutex here (we really don't need a mutex) ProtectedMemoryLocker locker(&gDictionaryMutex, gBreakpadAllocator); - breakpad->SetFilterCallback(callback); + breakpad->SetFilterCallback(callback, context); } - } catch(...) { // don't let exception leave this C API + } catch(...) { // don't let exceptions leave this C API fprintf(stderr, "BreakpadSetFilterCallback() : error\n"); } } @@ -933,5 +994,4 @@ void BreakpadAddLogFile(BreakpadRef ref, NSString *logPathname) { } BreakpadSetKeyValue(ref, logFileKey, logPathname); - } diff --git a/src/client/mac/sender/crash_report_sender.h b/src/client/mac/sender/crash_report_sender.h index b3169538..f62e7613 100644 --- a/src/client/mac/sender/crash_report_sender.h +++ b/src/client/mac/sender/crash_report_sender.h @@ -69,15 +69,20 @@ extern NSString *const kDefaultServerType; NSMutableDictionary *parameters_; // Key value pairs of data (STRONG) NSData *minidumpContents_; // The data in the minidump (STRONG) NSData *logFileData_; // An NSdata for the tar, - // bz2'd log file + // bz2'd log file. NSMutableDictionary *serverDictionary_; // The dictionary mapping a // server type name to a - // dictionary of URL - // parameter names + // dictionary of server + // parameter names. NSMutableDictionary *socorroDictionary_; // The dictionary for - // Socorro + // Socorro. NSMutableDictionary *googleDictionary_; // The dictionary for - // Google + // Google. + NSMutableDictionary *extraServerVars_; // A dictionary containing + // extra key/value pairs + // that are uploaded to the + // crash server with the + // minidump. } // Stops the modal panel with an NSAlertDefaultReturn value. This is the action @@ -95,9 +100,6 @@ extern NSString *const kDefaultServerType; - (BOOL)control:(NSControl *)control textView:(NSTextView *)textView doCommandBySelector:(SEL)commandSelector; -// Helper method to set HTTP parameters based on server type -- (BOOL)setPostParametersFromDictionary:(NSMutableDictionary *)crashParameters; - // Accessors to make bindings work - (NSString *)commentsValue; - (void)setCommentsValue:(NSString *)value; @@ -105,7 +107,4 @@ extern NSString *const kDefaultServerType; - (NSString *)emailValue; - (void)setEmailValue:(NSString *)value; -// Initialization helper to create dictionaries mapping Breakpad -// parameters to URL parameters -- (void)createServerParameterDictionaries; @end diff --git a/src/client/mac/sender/crash_report_sender.m b/src/client/mac/sender/crash_report_sender.m index 43d1da08..5d76290c 100644 --- a/src/client/mac/sender/crash_report_sender.m +++ b/src/client/mac/sender/crash_report_sender.m @@ -115,7 +115,7 @@ NSString *const kDefaultServerType = @"google"; NSRect newFrame = NSMakeRect(oldFrame.origin.x, oldFrame.origin.y, NSWidth(oldFrame), newSize.height); [self setFrame:newFrame]; - + return newSize.height - NSHeight(oldFrame); } @@ -217,7 +217,24 @@ NSString *const kDefaultServerType = @"google"; // Returns a dictionary that can be used to map Breakpad parameter names to // URL parameter names. -- (NSDictionary *)dictionaryForServerType:(NSString *)serverType; +- (NSMutableDictionary *)dictionaryForServerType:(NSString *)serverType; + +// Helper method to set HTTP parameters based on server type. This is +// called right before the upload - crashParameters will contain, on exit, +// URL parameters that should be sent with the minidump. +- (BOOL)populateServerDictionary:(NSMutableDictionary *)crashParameters; + +// Initialization helper to create dictionaries mapping Breakpad +// parameters to URL parameters +- (void)createServerParameterDictionaries; + +// Accessor method for the URL parameter dictionary +- (NSMutableDictionary *)urlParameterDictionary; + +// This method adds a key/value pair to the dictionary that +// will be uploaded to the crash server. +- (void)addServerParameter:(id)value forKey:(NSString *)key; + @end @implementation Reporter @@ -308,7 +325,24 @@ NSString *const kDefaultServerType = @"google"; id value = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; - [parameters_ setObject:value ? value : data forKey:key]; + // If the keyname is prefixed by BREAKPAD_SERVER_PARAMETER_PREFIX + // that indicates that it should be uploaded to the server along + // with the minidump, so we treat it specially. + if ([key hasPrefix:@BREAKPAD_SERVER_PARAMETER_PREFIX]) { + NSString *urlParameterKey = + [key substringFromIndex:[@BREAKPAD_SERVER_PARAMETER_PREFIX length]]; + if ([urlParameterKey length]) { + if (value) { + [self addServerParameter:value + forKey:urlParameterKey]; + } else { + [self addServerParameter:data + forKey:urlParameterKey]; + } + } + } else { + [parameters_ setObject:(value ? value : data) forKey:key]; + } [value release]; } @@ -743,6 +777,7 @@ doCommandBySelector:(SEL)commandSelector { serverDictionary_ = [[NSMutableDictionary alloc] init]; socorroDictionary_ = [[NSMutableDictionary alloc] init]; googleDictionary_ = [[NSMutableDictionary alloc] init]; + extraServerVars_ = [[NSMutableDictionary alloc] init]; [serverDictionary_ setObject:socorroDictionary_ forKey:kSocorroServerType]; [serverDictionary_ setObject:googleDictionary_ forKey:kGoogleServerType]; @@ -752,8 +787,6 @@ doCommandBySelector:(SEL)commandSelector { [googleDictionary_ setObject:@"comments" forKey:@BREAKPAD_COMMENTS]; [googleDictionary_ setObject:@"prod" forKey:@BREAKPAD_PRODUCT]; [googleDictionary_ setObject:@"ver" forKey:@BREAKPAD_VERSION]; - // TODO: just for testing, google's server doesn't support it - [googleDictionary_ setObject:@"buildid" forKey:@BREAKPAD_BUILD_ID]; [socorroDictionary_ setObject:@"Comments" forKey:@BREAKPAD_COMMENTS]; [socorroDictionary_ setObject:@"CrashTime" @@ -766,21 +799,23 @@ doCommandBySelector:(SEL)commandSelector { forKey:@BREAKPAD_PRODUCT]; [socorroDictionary_ setObject:@"ProductName" forKey:@BREAKPAD_PRODUCT]; - [socorroDictionary_ setObject:@"BuildID" - forKey:@BREAKPAD_BUILD_ID]; } -- (NSDictionary *)dictionaryForServerType:(NSString *)serverType { +- (NSMutableDictionary *)dictionaryForServerType:(NSString *)serverType { if (serverType == nil || [serverType length] == 0) { return [serverDictionary_ objectForKey:kDefaultServerType]; } return [serverDictionary_ objectForKey:serverType]; } -// Helper method to set HTTP parameters based on server type -- (BOOL)setPostParametersFromDictionary:(NSMutableDictionary *)crashParameters { +- (NSMutableDictionary *)urlParameterDictionary { NSString *serverType = [parameters_ objectForKey:@BREAKPAD_SERVER_TYPE]; - NSDictionary *urlParameterNames = [self dictionaryForServerType:serverType]; + return [self dictionaryForServerType:serverType]; + +} + +- (BOOL)populateServerDictionary:(NSMutableDictionary *)crashParameters { + NSDictionary *urlParameterNames = [self urlParameterDictionary]; id key; NSEnumerator *enumerator = [parameters_ keyEnumerator]; @@ -802,16 +837,31 @@ doCommandBySelector:(SEL)commandSelector { forKey:urlParameter]; } } + + // Now, add the parameters that were added by the application. + enumerator = [extraServerVars_ keyEnumerator]; + + while ((key = [enumerator nextObject])) { + NSString *urlParameterName = (NSString *)key; + NSString *urlParameterValue = + [extraServerVars_ objectForKey:urlParameterName]; + [crashParameters setObject:urlParameterValue + forKey:urlParameterName]; + } return YES; } +- (void)addServerParameter:(id)value forKey:(NSString *)key { + [extraServerVars_ setObject:value forKey:key]; +} + //============================================================================= - (void)report { NSURL *url = [NSURL URLWithString:[parameters_ objectForKey:@BREAKPAD_URL]]; HTTPMultipartUpload *upload = [[HTTPMultipartUpload alloc] initWithURL:url]; NSMutableDictionary *uploadParameters = [NSMutableDictionary dictionary]; - if (![self setPostParametersFromDictionary:uploadParameters]) { + if (![self populateServerDictionary:uploadParameters]) { return; } @@ -886,6 +936,7 @@ doCommandBySelector:(SEL)commandSelector { [googleDictionary_ release]; [socorroDictionary_ release]; [serverDictionary_ release]; + [extraServerVars_ release]; [super dealloc]; } diff --git a/src/client/mac/testapp/Controller.m b/src/client/mac/testapp/Controller.m index e548c917..a324f147 100644 --- a/src/client/mac/testapp/Controller.m +++ b/src/client/mac/testapp/Controller.m @@ -234,6 +234,9 @@ NSLog(@"Shouldn't find BreakpadKeyValue (key2)"); } + BreakpadAddUploadParameter(breakpad_, + @"MeaningOfLife", + @"42"); [NSThread detachNewThreadSelector:@selector(anotherThread) toTarget:self withObject:nil]; @@ -244,8 +247,8 @@ // automated testing. if ([args boolForKey:@"autocrash"]) { BreakpadSetKeyValue(breakpad_, - @BREAKPAD_SKIP_CONFIRM, - @"YES"); + @BREAKPAD_SKIP_CONFIRM, + @"YES"); [self causeCrash]; } diff --git a/src/client/mac/testapp/Info.plist b/src/client/mac/testapp/Info.plist index 2794c44b..6094ec6c 100644 --- a/src/client/mac/testapp/Info.plist +++ b/src/client/mac/testapp/Info.plist @@ -42,6 +42,13 @@ YES BreakpadVendor Foo Bar Corp, Incorporated, LTD, LLC + BreakpadServerParameters + + Param1 + Value1 + Param2 + Value2 + LSUIElement 1 diff --git a/src/client/mac/tests/BreakpadFramework_Test.mm b/src/client/mac/tests/BreakpadFramework_Test.mm new file mode 100644 index 00000000..cd163630 --- /dev/null +++ b/src/client/mac/tests/BreakpadFramework_Test.mm @@ -0,0 +1,216 @@ +// Copyright (c) 2009, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +// BreakpadFramework_Test.mm +// Test case file for Breakpad.h/mm. +// + +#import "GTMSenTestCase.h" +#import "Breakpad.h" + +#include + +@interface BreakpadFramework_Test : GTMTestCase { + @private + int last_exception_code_; + int last_exception_type_; + mach_port_t last_exception_thread_; + // We're not using Obj-C BOOL because we need to interop with + // Breakpad's callback. + bool shouldHandleException_; +} + +// This method is used by a callback used by test cases to determine +// whether to return true or false to Breakpad when handling an +// exception. +- (bool)shouldHandleException; +// This method returns a minimal dictionary that has what +// Breakpad needs to initialize. +- (NSMutableDictionary *)breakpadInitializationDictionary; +// This method is used by the exception handling callback +// to communicate to test cases the properites of the last +// exception. +- (void)setLastExceptionType:(int)type andCode:(int)code + andThread:(mach_port_t)thread; +@end + +// Callback for Breakpad exceptions +bool myBreakpadCallback(int exception_type, + int exception_code, + mach_port_t crashing_thread, + void *context); + +bool myBreakpadCallback(int exception_type, + int exception_code, + mach_port_t crashing_thread, + void *context) { + BreakpadFramework_Test *testCaseClass = + (BreakpadFramework_Test *)context; + [testCaseClass setLastExceptionType:exception_type + andCode:exception_code + andThread:crashing_thread]; + bool shouldHandleException = + [testCaseClass shouldHandleException]; + NSLog(@"Callback returning %d", shouldHandleException); + return shouldHandleException; +} +const int kNoLastExceptionCode = -1; +const int kNoLastExceptionType = -1; +const mach_port_t kNoLastExceptionThread = MACH_PORT_NULL; + +@implementation BreakpadFramework_Test +- (void) initializeExceptionStateVariables { + last_exception_code_ = kNoLastExceptionCode; + last_exception_type_ = kNoLastExceptionType; + last_exception_thread_ = kNoLastExceptionThread; +} + +- (NSMutableDictionary *)breakpadInitializationDictionary { + NSMutableDictionary *breakpadParams = + [NSMutableDictionary dictionaryWithCapacity:3]; + + [breakpadParams setObject:@"UnitTests" forKey:@BREAKPAD_PRODUCT]; + [breakpadParams setObject:@"1.0" forKey:@BREAKPAD_VERSION]; + [breakpadParams setObject:@"http://staging" forKey:@BREAKPAD_URL]; + return breakpadParams; +} + +- (bool)shouldHandleException { + return shouldHandleException_; +} + +- (void)setLastExceptionType:(int)type + andCode:(int)code + andThread:(mach_port_t)thread { + last_exception_type_ = type; + last_exception_code_ = code; + last_exception_thread_ = thread; +} + +// Test that the parameters mark required actually enable Breakpad to +// be initialized. +- (void)testBreakpadInstantiationWithRequiredParameters { + BreakpadRef b = BreakpadCreate([self breakpadInitializationDictionary]); + STAssertNotNULL(b, @"BreakpadCreate failed with required parameters"); + BreakpadRelease(b); +} + +// Test that Breakpad fails to initialize cleanly when required +// parameters are not present. +- (void)testBreakpadInstantiationWithoutRequiredParameters { + NSMutableDictionary *breakpadDictionary = + [self breakpadInitializationDictionary]; + + // Skip setting version, so that BreakpadCreate fails. + [breakpadDictionary removeObjectForKey:@BREAKPAD_VERSION]; + BreakpadRef b = BreakpadCreate(breakpadDictionary); + STAssertNULL(b, @"BreakpadCreate did not fail when missing a required" + " parameter!"); + + breakpadDictionary = [self breakpadInitializationDictionary]; + // Now test with no product + [breakpadDictionary removeObjectForKey:@BREAKPAD_PRODUCT]; + b = BreakpadCreate(breakpadDictionary); + STAssertNULL(b, @"BreakpadCreate did not fail when missing a required" + " parameter!"); + + breakpadDictionary = [self breakpadInitializationDictionary]; + // Now test with no URL + [breakpadDictionary removeObjectForKey:@BREAKPAD_URL]; + b = BreakpadCreate(breakpadDictionary); + STAssertNULL(b, @"BreakpadCreate did not fail when missing a required" + " parameter!"); + BreakpadRelease(b); +} + +// Test to ensure that when we call BreakpadAddUploadParameter, +// it's added to the dictionary correctly(this test depends on +// some internal details of Breakpad, namely, the special prefix +// that it uses to figure out which key/value pairs to upload). +- (void)testAddingBreakpadServerVariable { + NSMutableDictionary *breakpadDictionary = + [self breakpadInitializationDictionary]; + + BreakpadRef b = BreakpadCreate(breakpadDictionary); + STAssertNotNULL(b, @"BreakpadCreate failed with valid dictionary!"); + + BreakpadAddUploadParameter(b, + @"key", + @"value"); + + // Test that it did not add the key/value directly, e.g. without + // prepending the key with the prefix. + STAssertNil(BreakpadKeyValue(b, @"key"), + @"AddUploadParameter added key directly to dictionary" + " instead of prepending it!"); + + NSString *prependedKeyname = + [@BREAKPAD_SERVER_PARAMETER_PREFIX stringByAppendingString:@"key"]; + + STAssertEqualStrings(BreakpadKeyValue(b, prependedKeyname), + @"value", + @"Calling BreakpadAddUploadParameter did not prepend " + "key name"); + BreakpadRelease(b); +} + +// Test that when we do on-demand minidump generation, +// the exception code/type/thread are set properly. +- (void)testFilterCallbackReturnsFalse { + NSMutableDictionary *breakpadDictionary = + [self breakpadInitializationDictionary]; + + BreakpadRef b = BreakpadCreate(breakpadDictionary); + STAssertNotNULL(b, @"BreakpadCreate failed with valid dictionary!"); + BreakpadSetFilterCallback(b, &myBreakpadCallback, self); + + // This causes the callback to return false, meaning + // Breakpad won't take the exception + shouldHandleException_ = false; + + [self initializeExceptionStateVariables]; + STAssertEquals(last_exception_type_, kNoLastExceptionType, + @"Last exception type not initialized correctly."); + STAssertEquals(last_exception_code_, kNoLastExceptionCode, + @"Last exception code not initialized correctly."); + STAssertEquals(last_exception_thread_, kNoLastExceptionThread, + @"Last exception thread is not initialized correctly."); + + // Cause Breakpad's exception handler to be invoked. + BreakpadGenerateAndSendReport(b); + + STAssertEquals(last_exception_type_, 0, + @"Last exception type is not 0 for on demand"); + STAssertEquals(last_exception_code_, 0, + @"Last exception code is not 0 for on demand"); + STAssertEquals(last_exception_thread_, (mach_port_t)0, + @"Last exception thread is not 0 for on demand"); +} + +@end -- cgit v1.2.1