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

Supporting (escaped) ampersands in query #343

Closed
agundermann opened this issue Oct 4, 2014 · 6 comments · Fixed by #366
Closed

Supporting (escaped) ampersands in query #343

agundermann opened this issue Oct 4, 2014 · 6 comments · Fixed by #366

Comments

@agundermann
Copy link
Contributor

If I do a transition with a query such as { foo: "bar&baz"}, it will be correctly encoded in the URL as ?foo=bar%26bar, but the query inside the page handler ends up being {foo: "bar", baz: ""}.

@ryanflorence
Copy link
Member

A failing test case would be awesome here. Think you can make one and send a pull request? (Just with the test case, don't need the fix.)

Probably do an injectParams in: https://github.com/rackt/react-router/blob/master/modules/utils/__tests__/Path-test.js

@agundermann
Copy link
Contributor Author

There you go #355

@ryanflorence
Copy link
Member

The test passes ... are you sure of the issue in your handler?

@agundermann
Copy link
Contributor Author

That's surprising. It fails for me with AssertionError: {"id":"a","b":""} deepEqual {"id":"a&b"} .

Looking at the implementation, I don't see how this test could pass. The URL is decoded via decodeURIComponent before qs.parse is called, so it should be impossible for qs.parse to distinguish between separators and those ampersands wanted inside values. Removing the decodeURL call lets the test pass for me.

Am I missing something?

I don't think it matters, but I'm using Windows 8.1 and Chrome 37.0.2062 .

@ryanflorence
Copy link
Member

oh, I'm sorry, I mistook github's "this can be merged" for "everything is great on travis ci!" :\

Thank you for the test case!

@agundermann
Copy link
Contributor Author

Thank you for resolving this. Glad I could contribute 😄

@lock lock bot locked as resolved and limited conversation to collaborators Jan 25, 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 a pull request may close this issue.

2 participants