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

Dynamic route segments aren't URL-encoded #4794

Closed
dfreeman opened this issue Apr 30, 2014 · 10 comments
Closed

Dynamic route segments aren't URL-encoded #4794

dfreeman opened this issue Apr 30, 2014 · 10 comments
Assignees

Comments

@dfreeman
Copy link
Contributor

I've read through #3263 and the associated PR (#3545), and while it looks like everybody involved agreed that URL encoding should 'just work', it doesn't look like that's where things ultimately landed.

Those issues were closed in favor of rolling in a fix that was part of @alexspeller's query params work, but unfortunately it appears URL encoding is only handled for query parameters, not dynamic route segments.

As a quick example of where things can go wrong, take a look at this JS Bin example. It has two models, one with ID 'A B' (Raw) and one with ID 'A%20B' (Encoded). If you click the link for the 'Encoded' model and then refresh the page, you'll see it loads the 'Raw' one instead.

HashLocation and HistoryLocation both appear to have this behavior. I would expect them always to encode dynamic path segments to avoid this kind of ambiguity.

@alexspeller
Copy link
Contributor

I think https://github.com/emberjs/ember.js/pull/3396/files needs to be reapplied without the feature flag as it looks like it got chucked

@alexspeller
Copy link
Contributor

Also possibly needs updating to test and support HashLocation too if that's still got a problem?

@alexspeller
Copy link
Contributor

@machty I think we talked about this at some point

@wagenet
Copy link
Member

wagenet commented Aug 6, 2014

@machty ping.

1 similar comment
@wagenet
Copy link
Member

wagenet commented Nov 1, 2014

@machty ping.

@xkb
Copy link

xkb commented Dec 29, 2014

Any news on this?

@MichalBryxi
Copy link

Got also hit by this one. I think Ember should handle this as I'm now supposed to encode and decode each potentially "dangerous" dynamic segment I'm using by hand. I don't see a reason why the user should care about this - should work out-of-box.

@rwjblue
Copy link
Member

rwjblue commented Aug 24, 2015

I updated the provided JSBin to current canary setup:

http://rwjblue.jsbin.com/dusuti/edit?html,js

The reported issue of clicking on "Encoded" and being transitioned into "Raw" after a refresh remains.

@dfreeman
Copy link
Contributor Author

dfreeman commented Aug 4, 2016

I haven't dug in to be certain yet, but it looks like this may have been fixed in 2.7?

@Serabe
Copy link
Member

Serabe commented Aug 19, 2016

It seems so. Closing this as fixed, though I think there is a related issue :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants