Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce requiring API key to initialise notifier #280

Merged
merged 9 commits into from
May 24, 2018
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
Changelog
=========

## 5.X.X (TBD)

* Enforce requiring API key to initialise notifier [#280](https://github.com/bugsnag/bugsnag-cocoa/pull/280)

## 5.15.5 (25 Apr 2018)

### Bug Fixes
Expand Down
168 changes: 102 additions & 66 deletions Source/Bugsnag.m
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,13 @@ + (void)startBugsnagWithApiKey:(NSString *)apiKey {

+ (void)startBugsnagWithConfiguration:(BugsnagConfiguration *)configuration {
@synchronized(self) {
bsg_g_bugsnag_notifier =
[[BugsnagNotifier alloc] initWithConfiguration:configuration];
[bsg_g_bugsnag_notifier start];
if ([configuration hasValidApiKey]) {
bsg_g_bugsnag_notifier =
[[BugsnagNotifier alloc] initWithConfiguration:configuration];
[bsg_g_bugsnag_notifier start];
} else {
bsg_log_err(@"Bugsnag not initialized - a valid API key must be supplied.");
}
}
}

Expand All @@ -73,75 +77,87 @@ + (BugsnagNotifier *)notifier {
}

+ (void)notify:(NSException *)exception {
[self.notifier notifyException:exception
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;
}];
if ([self bugsnagStarted]) {
[self.notifier notifyException:exception
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;
}];
}
}

+ (void)notify:(NSException *)exception block:(BugsnagNotifyBlock)block {
[[self notifier] notifyException:exception
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;

if (block) {
block(report);
}
}];
if ([self bugsnagStarted]) {
[[self notifier] notifyException:exception
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;

if (block) {
block(report);
}
}];
}
}

+ (void)notifyError:(NSError *)error {
[self.notifier notifyError:error
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;
}];
if ([self bugsnagStarted]) {
[self.notifier notifyError:error
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;
}];
}
}

+ (void)notifyError:(NSError *)error block:(BugsnagNotifyBlock)block {
[[self notifier] notifyError:error
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;

if (block) {
block(report);
}
}];
if ([self bugsnagStarted]) {
[[self notifier] notifyError:error
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;

if (block) {
block(report);
}
}];
}
}

+ (void)notify:(NSException *)exception withData:(NSDictionary *)metaData {

[[self notifier]
notifyException:exception
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;
report.metaData = [metaData
BSG_mergedInto:[self.notifier.configuration
.metaData toDictionary]];
}];
if ([self bugsnagStarted]) {
[[self notifier]
notifyException:exception
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;
report.metaData = [metaData
BSG_mergedInto:[self.notifier.configuration
.metaData toDictionary]];
}];
}
}

+ (void)notify:(NSException *)exception
withData:(NSDictionary *)metaData
atSeverity:(NSString *)severity {

[[self notifier]
notifyException:exception
atSeverity:BSGParseSeverity(severity)
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;
report.metaData = [metaData
BSG_mergedInto:[self.notifier.configuration
.metaData toDictionary]];
report.severity = BSGParseSeverity(severity);
}];
if ([self bugsnagStarted]) {
[[self notifier]
notifyException:exception
atSeverity:BSGParseSeverity(severity)
block:^(BugsnagCrashReport *_Nonnull report) {
report.depth += 2;
report.metaData = [metaData
BSG_mergedInto:[self.notifier.configuration
.metaData toDictionary]];
report.severity = BSGParseSeverity(severity);
}];
}
}

+ (void)internalClientNotify:(NSException *_Nonnull)exception
withData:(NSDictionary *_Nullable)metaData
block:(BugsnagNotifyBlock _Nullable)block {
[self.notifier internalClientNotify:exception
withData:metaData
block:block];
if ([self bugsnagStarted]) {
[self.notifier internalClientNotify:exception
withData:metaData
block:block];
}
}

+ (void)addAttribute:(NSString *)attributeName
Expand All @@ -161,7 +177,7 @@ + (void)clearTabWithName:(NSString *)tabName {
}

+ (BOOL)bugsnagStarted {
if (self.notifier == nil) {
if (!self.notifier.started) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this handle the notifier being nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as nil will be coerced to false.

bsg_log_err(@"Ensure you have started Bugsnag with startWithApiKey: "
@"before calling any other Bugsnag functions.");

Expand All @@ -171,31 +187,43 @@ + (BOOL)bugsnagStarted {
}

+ (void)leaveBreadcrumbWithMessage:(NSString *)message {
[self leaveBreadcrumbWithBlock:^(BugsnagBreadcrumb *_Nonnull crumbs) {
crumbs.metadata = @{BSGKeyMessage : message};
}];
if ([self bugsnagStarted]) {
[self leaveBreadcrumbWithBlock:^(BugsnagBreadcrumb *_Nonnull crumbs) {
crumbs.metadata = @{BSGKeyMessage: message};
}];
}
}

+ (void)leaveBreadcrumbWithBlock:
(void (^_Nonnull)(BugsnagBreadcrumb *_Nonnull))block {
[self.notifier addBreadcrumbWithBlock:block];
if ([self bugsnagStarted]) {
[self.notifier addBreadcrumbWithBlock:block];
}
}

+ (void)leaveBreadcrumbForNotificationName:
(NSString *_Nonnull)notificationName {
[self.notifier crumbleNotification:notificationName];
if ([self bugsnagStarted]) {
[self.notifier crumbleNotification:notificationName];
}
}

+ (void)setBreadcrumbCapacity:(NSUInteger)capacity {
self.notifier.configuration.breadcrumbs.capacity = capacity;
if ([self bugsnagStarted]) {
self.notifier.configuration.breadcrumbs.capacity = capacity;
}
}

+ (void)clearBreadcrumbs {
[self.notifier clearBreadcrumbs];
if ([self bugsnagStarted]) {
[self.notifier clearBreadcrumbs];
}
}

+ (void)startSession {
[self.notifier startSession];
if ([self bugsnagStarted]) {
[self.notifier startSession];
}
}

+ (NSDateFormatter *)payloadDateFormatter {
Expand All @@ -209,23 +237,31 @@ + (NSDateFormatter *)payloadDateFormatter {
}

+ (void)setSuspendThreadsForUserReported:(BOOL)suspendThreadsForUserReported {
[[BSG_KSCrash sharedInstance]
setSuspendThreadsForUserReported:suspendThreadsForUserReported];
if ([self bugsnagStarted]) {
[[BSG_KSCrash sharedInstance]
setSuspendThreadsForUserReported:suspendThreadsForUserReported];
}
}

+ (void)setReportWhenDebuggerIsAttached:(BOOL)reportWhenDebuggerIsAttached {
[[BSG_KSCrash sharedInstance]
setReportWhenDebuggerIsAttached:reportWhenDebuggerIsAttached];
if ([self bugsnagStarted]) {
[[BSG_KSCrash sharedInstance]
setReportWhenDebuggerIsAttached:reportWhenDebuggerIsAttached];
}
}

+ (void)setThreadTracingEnabled:(BOOL)threadTracingEnabled {
[[BSG_KSCrash sharedInstance] setThreadTracingEnabled:threadTracingEnabled];
if ([self bugsnagStarted]) {
[[BSG_KSCrash sharedInstance] setThreadTracingEnabled:threadTracingEnabled];
}
}

+ (void)setWriteBinaryImagesForUserReported:
(BOOL)writeBinaryImagesForUserReported {
[[BSG_KSCrash sharedInstance]
setWriteBinaryImagesForUserReported:writeBinaryImagesForUserReported];
if ([self bugsnagStarted]) {
[[BSG_KSCrash sharedInstance]
setWriteBinaryImagesForUserReported:writeBinaryImagesForUserReported];
}
}

@end
Expand Down
2 changes: 2 additions & 0 deletions Source/BugsnagConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,4 +193,6 @@ BugsnagBreadcrumbs *breadcrumbs;
@property(nullable) NSString *codeBundleId;
@property(nullable) NSString *notifierType;

- (BOOL)hasValidApiKey;

@end
22 changes: 22 additions & 0 deletions Source/BugsnagConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#import "BSG_RFC3339DateTool.h"
#import "BugsnagUser.h"
#import "BugsnagSessionTracker.h"
#import "BugsnagLogger.h"

static NSString *const kHeaderApiPayloadVersion = @"Bugsnag-Payload-Version";
static NSString *const kHeaderApiKey = @"Bugsnag-Api-Key";
Expand Down Expand Up @@ -199,6 +200,22 @@ - (void)setAppVersion:(NSString *)newVersion {
}
}

@synthesize apiKey = _apiKey;

- (NSString *)apiKey {
return _apiKey;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we remove this as it is just the default generated by @synthesize?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it needs to be specified since setApiKey is overridden.


- (void)setApiKey:(NSString *)apiKey {
if ([apiKey length] > 0) {
[self willChangeValueForKey:NSStringFromSelector(@selector(apiKey))];
_apiKey = apiKey;
[self didChangeValueForKey:NSStringFromSelector(@selector(apiKey))];
} else {
bsg_log_err(@"Attempted to override non-null API key with nil - ignoring.");
}
}

@synthesize shouldAutoCaptureSessions = _shouldAutoCaptureSessions;

- (BOOL)shouldAutoCaptureSessions {
Expand Down Expand Up @@ -231,4 +248,9 @@ - (NSDictionary *)sessionApiHeaders {
kHeaderApiSentAt: [BSG_RFC3339DateTool stringFromDate:[NSDate new]]
};
}

- (BOOL)hasValidApiKey {
return [_apiKey length] > 0;
}

@end
1 change: 1 addition & 0 deletions Source/BugsnagNotifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
@property(nonatomic, readwrite, retain) NSLock *_Nonnull metaDataLock;

@property(nonatomic) BSGConnectivity *_Nonnull networkReachable;
@property(readonly) BOOL started;

- (instancetype _Nonnull)initWithConfiguration:
(BugsnagConfiguration *_Nonnull)configuration;
Expand Down
1 change: 1 addition & 0 deletions Source/BugsnagNotifier.m
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ - (void)start {

// notification not received in time on initial startup, so trigger manually
[self willEnterForeground:self];
_started = YES;
}

- (void)watchLifecycleEvents:(NSNotificationCenter *)center {
Expand Down
27 changes: 27 additions & 0 deletions Tests/BugsnagConfigurationTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,31 @@ - (void)testUser {

}

- (void)testApiKeySetter {
BugsnagConfiguration *config = [BugsnagConfiguration new];
XCTAssertEqualObjects(@"", config.apiKey);

config.apiKey = @"test";
XCTAssertEqualObjects(@"test", config.apiKey);

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wnonnull"
config.apiKey = nil;
#pragma clang diagnostic pop

XCTAssertEqualObjects(@"test", config.apiKey);
}

- (void)testHasValidApiKey {
BugsnagConfiguration *config = nil;
XCTAssertFalse([config hasValidApiKey]);

config = [BugsnagConfiguration new];
XCTAssertFalse([config hasValidApiKey]);

config.apiKey = @"5adf89e0aaa";
XCTAssertTrue([config hasValidApiKey]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config.apiKey = "" should also be false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's covered by L122, as config defaults to @""

}


@end