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

Do not throw an exception when URI parsing fails #1412

Merged
merged 1 commit into from Aug 15, 2017
Merged

Do not throw an exception when URI parsing fails #1412

merged 1 commit into from Aug 15, 2017

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Aug 14, 2017

Ref: medialize/URI.js#352

URIjs v1.18.11 added hostname validation, and withinString can find invalid urls, so our safest bet is to just ignore invalid URLs instead of crashing.

While I do believe this is a URIjs bug, we're safer just ignoring any errors it throws as we are interacting with user input.

@xPaw xPaw added the Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. label Aug 14, 2017
@xPaw xPaw added this to the 2.5.0 milestone Aug 14, 2017
Copy link
Member

@astorije astorije left a comment

Choose a reason for hiding this comment

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

Yeah, pretty awkward that a library would throw on something it detected itself earlier lol.

Approving this fix (more of a hack, really), under the condition that we remove it once the related bug is fixed.

@xPaw xPaw merged commit 6ce46a6 into master Aug 15, 2017
@xPaw xPaw deleted the xpaw/fix-uri branch August 15, 2017 11:40
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
Do not throw an exception when URI parsing fails
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants