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

Fix a crash when XMLHttpRequest URL isn't a string #1598

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

imjoehaines
Copy link
Contributor

@imjoehaines imjoehaines commented Nov 24, 2021

Goal

An XMLHttpRequest can be passed any object that returns a URL from its toString method. For example, a HTMLAnchorElement returns its href and so can be passed to XMLHttpRequest.open (obviously other values can also be passed, but they wouldn't lead to a valid URL so the request wouldn't work)

This PR allows us to handle this case as previously we assumed the URL was a string. As Bugsnag endpoints must be strings we don't try to convert an object into a string, because we would never send a request with any other value in the first place

@github-actions
Copy link

github-actions bot commented Nov 24, 2021

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 41.54 kB 12.79 kB
After 41.58 kB 12.80 kB
± ⚠️ +42 bytes ⚠️ +9 bytes

code coverage diff

Ok File Lines Branches Functions Statements
/home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-network-breadcrumbs/network-breadcrumbs.js 98.78%
(+0.03%)
90.38%
(+0.8%)
100%
(+0%)
98.84%
(+0.03%)

Total:

Lines Branches Functions Statements
82.05%(+0%) 71.91%(+0.05%) 83.26%(+0%) 81.11%(+0.01%)

Generated by 🚫 dangerJS against 1e8b8e0

e.g. a HTMLAnchorElement returns its 'href' from 'toString', which
allows it to be used in place of a URL string
@imjoehaines imjoehaines marked this pull request as ready for review November 25, 2021 10:41
@imjoehaines imjoehaines merged commit 26f58b6 into next Nov 25, 2021
@imjoehaines imjoehaines deleted the fix-network-breadcrumb-crash branch November 25, 2021 11:12
@imjoehaines imjoehaines mentioned this pull request Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants