Skip to content

Commit

Permalink
Fix CSP bug in iOS9 (#358)
Browse files Browse the repository at this point in the history
In iOS9 Safari, a request that violates the Content Security Policy (CSP) rules causes a `DOMException` to be thrown. Since v4.7.0, bugsnag calls `startSession()` synchronously upon startup, which means that any thrown error in that tick of the event loop results in the libary (and anything else in that synchronous execution context) to be unusable. The behaviour of iOS Safari was fixed in version 10, but since we support 9, this needs a workaround.

The xml-http-request transport mechanism already wrapped request.send() with a try/catch, but the error in this instance was coming from the request.open() call. The try/catch block was moved out out to include all of the synchronous code inside sendReport()/sendSession().
  • Loading branch information
bengourley authored Jun 18, 2018
1 parent 8811c44 commit 11921b2
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 45 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Bugsnag error reporter for JavaScript
![11.02kB](https://img.shields.io/badge/size-11.02kB-green.svg)
![11.03kB](https://img.shields.io/badge/size-11.03kB-green.svg)
[![Documentation](https://img.shields.io/badge/docs-v4-green.svg)](https://docs.bugsnag.com/platforms/browsers/js)
[![Build status](https://travis-ci.org/bugsnag/bugsnag-js.svg?branch=master)](https://travis-ci.org/bugsnag/bugsnag-js)
[![BrowserStack Status](https://www.browserstack.com/automate/badge.svg?badge_key=VkNhNGlWRTV6c1Z1VXByYmxFTCtwbUd4M1p5cUI3KzFWRTJvaWk3WFZBTT0tLTBNZjFuM2ZJbW0vUDBPZ1pMQ3ZCd2c9PQ==--003c472323b43561f74fdbca9f732de0f609c74c)](https://www.browserstack.com/automate/public-build/VkNhNGlWRTV6c1Z1VXByYmxFTCtwbUd4M1p5cUI3KzFWRTJvaWk3WFZBTT0tLTBNZjFuM2ZJbW0vUDBPZ1pMQ3ZCd2c9PQ==--003c472323b43561f74fdbca9f732de0f609c74c)
Expand Down
40 changes: 20 additions & 20 deletions browser/transports/xml-http-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,34 @@ const { isoDate } = require('../../base/lib/es-utils')

module.exports = {
sendReport: (logger, config, report, cb = () => {}) => {
const url = config.endpoints.notify
const req = new window.XMLHttpRequest()
req.onreadystatechange = function () {
if (req.readyState === window.XMLHttpRequest.DONE) cb(null, req.responseText)
}
req.open('POST', url)
req.setRequestHeader('Content-Type', 'application/json')
req.setRequestHeader('Bugsnag-Api-Key', report.apiKey || config.apiKey)
req.setRequestHeader('Bugsnag-Payload-Version', '4.0')
req.setRequestHeader('Bugsnag-Sent-At', isoDate())
try {
const url = config.endpoints.notify
const req = new window.XMLHttpRequest()
req.onreadystatechange = function () {
if (req.readyState === window.XMLHttpRequest.DONE) cb(null, req.responseText)
}
req.open('POST', url)
req.setRequestHeader('Content-Type', 'application/json')
req.setRequestHeader('Bugsnag-Api-Key', report.apiKey || config.apiKey)
req.setRequestHeader('Bugsnag-Payload-Version', '4.0')
req.setRequestHeader('Bugsnag-Sent-At', isoDate())
req.send(makePayload(report))
} catch (e) {
logger.error(e)
}
},
sendSession: (logger, config, session, cb = () => {}) => {
const url = config.endpoints.sessions
const req = new window.XMLHttpRequest()
req.onreadystatechange = function () {
if (req.readyState === window.XMLHttpRequest.DONE) cb(null, req.responseText)
}
req.open('POST', url)
req.setRequestHeader('Content-Type', 'application/json')
req.setRequestHeader('Bugsnag-Api-Key', config.apiKey)
req.setRequestHeader('Bugsnag-Payload-Version', '1.0')
req.setRequestHeader('Bugsnag-Sent-At', isoDate())
try {
const url = config.endpoints.sessions
const req = new window.XMLHttpRequest()
req.onreadystatechange = function () {
if (req.readyState === window.XMLHttpRequest.DONE) cb(null, req.responseText)
}
req.open('POST', url)
req.setRequestHeader('Content-Type', 'application/json')
req.setRequestHeader('Bugsnag-Api-Key', config.apiKey)
req.setRequestHeader('Bugsnag-Payload-Version', '1.0')
req.setRequestHeader('Bugsnag-Sent-At', isoDate())
req.send(jsonStringify(session))
} catch (e) {
logger.error(e)
Expand Down
42 changes: 21 additions & 21 deletions dist/bugsnag.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/bugsnag.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/bugsnag.min.js.map

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions features/csp.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
@csp
Feature: Compatibility with a Content Security Policy

Scenario Outline: notifer does not crash for CSP violations
When I navigate to the URL "/csp/<type>/a.html"
And the test should run in this browser
Then I let the test page run for up to 10 seconds
Examples:
| type |
| script |
32 changes: 32 additions & 0 deletions features/fixtures/csp/script/a.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Security-Policy" content="default-src 'none'; script-src 'unsafe-inline' 'self';">
<script src="/node_modules/bugsnag-js/dist/bugsnag.min.js"></script>
<script type="text/javascript">
var ENDPOINT = decodeURIComponent(window.location.search.match(/ENDPOINT=(.+)/)[1])
var bugsnagClient = bugsnag({
apiKey: 'ABC',
endpoints: {
notify: ENDPOINT,
sessions: ENDPOINT
},
autoNotify: false
})
</script>
</head>
<body>
<pre id="bugsnag-test-should-run">YES</pre>
<pre id="bugsnag-test-state">PENDING</pre>
<script>
throw new Error('this should not get through')
</script>
<script>
setTimeout(function () {
var el = document.getElementById('bugsnag-test-state')
// if the CSP violation caused bugsnag not to init, bugsnagClient won't be defined
el.textContent = el.innerText = (bugsnagClient ? 'DONE' : 'ERROR')
}, 5000)
</script>
</body>
</html>
8 changes: 8 additions & 0 deletions features/fixtures/csp/script/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "bugsnag-js-fixtures-csp-script",
"private": true,
"scripts": {
"build": "echo 'Done'"
},
"dependencies": {}
}
9 changes: 8 additions & 1 deletion features/steps/browser_steps.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
include Test::Unit::Assertions

When("I navigate to the URL {string}") do |path|
$driver.navigate.to get_test_url path
end
Expand All @@ -15,8 +17,13 @@
wait = Selenium::WebDriver::Wait.new(timeout: n)
wait.until {
$driver.find_element(id: 'bugsnag-test-state') &&
$driver.find_element(id: 'bugsnag-test-state').text == 'DONE'
(
$driver.find_element(id: 'bugsnag-test-state').text == 'DONE' ||
$driver.find_element(id: 'bugsnag-test-state').text == 'ERROR'
)
}
txt = $driver.find_element(id: 'bugsnag-test-state').text
assert_equal('DONE', txt, "Expected #bugsnag-test-state text to be 'DONE'. It was '#{txt}'.")
end

When("the exception matches the {string} values for the current browser") do |fixture|
Expand Down

0 comments on commit 11921b2

Please sign in to comment.