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

Don't crash on invalid uri query parameters. #3453

Merged
merged 3 commits into from
May 17, 2016

Conversation

bae-unidev
Copy link
Contributor

Replace decodeURIComponent with _tryDecodeURIComponent to ignore crash "URIError: malformed URI sequence".

@taion
Copy link
Contributor

taion commented May 12, 2016

The router offers an onError callback to handle errors, though. Why not just use that? null doesn't seem like a suitable fallback here.

@bae-unidev
Copy link
Contributor Author

bae-unidev commented May 12, 2016

I don't know that not call onError handler when decodeURIComponent throwing.
Is History of the problem?

@taion
Copy link
Contributor

taion commented May 12, 2016

onError on the router catches all errors

@bae-unidev
Copy link
Contributor Author

bae-unidev commented May 12, 2016

<Router history={ hashHistory } onError={ () => console.error("err!") }>
   <Route path="/" component={ Index } />
</Router>

path: /#/%
My browser(Chrome 50.0.2661.94) throw Uncaught URIError: URI malformed. and not working router.
did you mean it?

PS: Sorry, I am not good at English. I am able to speak rudely. Thank you for your understanding.

@taion
Copy link
Contributor

taion commented May 16, 2016

I see – we should catch the error there from matchRoutes, then. The correct behavior here is still to hit the error handling logic.

@bae-unidev
Copy link
Contributor Author

What if add callback(error, matched) parameter to matchPattern function in PatternUtils.js?

@taion
Copy link
Contributor

taion commented May 16, 2016

It doesn't need that – it's synchronous.

@bae-unidev
Copy link
Contributor Author

So, you mean I should handle error in matchPattern function, not by using callback?
If so, when user pass malformed URI, router will not working. Is it right?

@taion
Copy link
Contributor

taion commented May 16, 2016

I am saying to just add a try/catch here: https://github.com/reactjs/react-router/blob/master/modules/matchRoutes.js#L91 and invoke callback with the error if there was one.

@bae-unidev
Copy link
Contributor Author

I modified it that you saying. Is it fine?

@taion
Copy link
Contributor

taion commented May 17, 2016

Thanks! LGTM.

@timdorr timdorr merged commit 3691914 into remix-run:master May 17, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants