Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Commit

Permalink
Enforce that onBeforeNotifyCallbacks can't mess with handledState
Browse files Browse the repository at this point in the history
Also detects if any callback changed "severity" and update
"defaultSeverity" accordingly.
  • Loading branch information
bengourley committed Sep 8, 2017
1 parent 3d97df2 commit c042167
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
18 changes: 18 additions & 0 deletions lib/notification.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ function Notification(bugsnagErrors, options, handledState) {

Notification.prototype.deliver = function(cb, originalError) {

// save handledState properties, because the user shouldn't be able to set them
var before = {
severity: this.events[0].severity,
defaultSeverity: this.events[0].defaultSeverity,
unhandled: this.events[0].unhandled,
severityReason: this.events[0].severityReason
}

// run before notify callbacks
var shouldNotify = true;
Configuration.beforeNotifyCallbacks.forEach(function (callback) {
Expand All @@ -139,6 +147,16 @@ Notification.prototype.deliver = function(cb, originalError) {
return;
}

// reinstate handledState properties
this.events[0].defaultSeverity = before.defaultSeverity
this.events[0].unhandled = before.unhandled
this.events[0].severityReason = before.severityReason

// but do one last check to see if severity was changed as a result of the callback(s)
if (before.severity !== this.events[0].severity) {
this.events[0].defaultSeverity = false;
}

this._deliver(cb);
};

Expand Down
18 changes: 18 additions & 0 deletions test/notification.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,24 @@ describe("Notification", function() {

Configuration.beforeNotifyCallbacks = [];
});

it("should not be able to modify handledState", function () {
Bugsnag.onBeforeNotify(function (notification, error) {
notification.events[0].unhandled = true;
notification.events[0].defaultSeverity = false;
});
Bugsnag.notify(new Error('breaky'));
deliverStub.firstCall.thisValue.events[0].unhandled.should.equal(false);
deliverStub.firstCall.thisValue.events[0].defaultSeverity.should.equal(true);
});

it("should cause defaultSeverity=false if any callback changes severity", function () {
Bugsnag.onBeforeNotify(function (notification, error) {
notification.events[0].severity = "error";
});
Bugsnag.notify(new Error('breaky'));
deliverStub.firstCall.thisValue.events[0].defaultSeverity.should.equal(false);
})
});

describe("autoNotify", function() {
Expand Down

0 comments on commit c042167

Please sign in to comment.