-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
url-encoded values in params object #3263
Comments
What browser are you using? I suspect this may be a browser bug. |
Behavior observed on |
Unfortunately I cannot get my hands on an |
This works fine for me on a Mac with Chrome 29 and FF 23. However, it does not work on Safari 6. Fiddle here: http://jsfiddle.net/M4YJC/1 or http://jsfiddle.net/M4YJC/1/show/#/search/便 This seems like a browser bug, but we may be able to do something about it in Ember. |
Thanks for testing. Would dropping a |
@heartsentwined and @wagenet if it has already be decoded, then adding another decode could decode characters not meant to be decoded. A fix needs to normalize behavior between browsers, I seen this workaround by splitting the hash off of location.href and decoding it instead of relying on location.hash to be consistently decoded. |
@heartsentwined Would you be interested in attempting a PR for this? |
I have no idea on how to fix this while avoiding the double-decoding issue if this is browser-dependent |
@heartsentwined location.href is never decoded, so using it instead of location.hash should avoid double decoding. |
@alexspeller, this seems related to #3396. |
I believe this is the exact same issue - can anyone confirm the issue is fixed if you use master and enable the query params feature? (i.e. |
@wagenet confirm |
@alexspeller Under what circumstances would the app want encoded params? |
Was trying to take a stab at it, but further testing shows that this same issue is observed on main route paths themselves too, which #3396 doesn't address. Example: App.Router.map ->
@route 'foo', { path: '便' } # again just a non-url-safe character Entering by
I'm starting to think whether this should actually belong to the @wagenet thoughts? @alexspeller I think the proper fix and expectation is to decode all parts. Any app that could expect a non-url-safe character is most likely targeting audiences that can read those characters in the first place, instead of just wanting to pass the encoded characters around behind-the-scenes. Like the search term, if ember exposes a universally encoded term, then userland code would like have to (1) decode them to display current search term, (2) decode them to pass to backend API, etc. In short, userland code would be doing the decoding work everywhere, where ember could have handled it centrally in one place. Use case: any ember app that targets any audience beyond purely-English-speaking fellows. |
I've been thinking about the issue (I've created #3514). and I was wondering if we could have something like Symfony2 has or how it could be done already (I'm not that familiar with ember yet, sorry). Handling this centrally and sanely could help with issues I had in the past with "/" in params. They should be encoded when the link is generated imho otherwise a bookmark breaks while clicking a link works. |
@heartsentwined I agree, decoding and encoding should be automatic. |
Sidenote: what should be the expected behavior for the space character? It could come in |
To me it feels it should be %20 by default (afaik only that's a real standard) but easy to change to + (as that's what I want to use ;-)) |
I think it should just use |
@alexspeller Why not have an abstraction for that? #3489 talks about a serialize and deserialize function that has been removed. why?! That's exactly what I would do. Even if they only wrap encodeURIComponent and decodeURIComponent it leaves it up for anyone to exchange that implementation. |
@dschmidt |
Overriding I have created a test page that outputs the different properties of the
(edited: added encoded-version url entry) |
The issue is that, contrary to what @kselden has said, even the encoded/decoded status |
Oh and the exact |
(edited: added encoded-version url entry) |
You have to encode it on the way in for it to be consistent. |
@kselden what do you mean? The test code was just """
<pre>
window.location.href = '#{window.location.href}'
window.location.pathname = '#{window.location.pathname}'
window.location.search = '#{window.location.search}'
window.location.hash = '#{window.location.hash}'
</pre>
""" |
So I was trying to inspect the properties that the |
Oh wait, I get it, so the entry URL should have been |
|
@heartsentwined Looks like this is more complicated than we had hoped. I don't really want to have a ton of Ember code to manage browser inconsistencies and I definitely don't want to have to do browser sniffing (though behavior sniffing is ok). Any ideas? |
@heartsentwined the user typing in a query params seems a dubious concern, but even if they do, decoding it will not be an issue. Regardless it is still consistent encoding the during serialization and decoding during deserialization. |
To clarify what @kselden says, it should work to always extract with |
@kselden I agree that nobody would type in query strings, but I am imagining a use case where the user types in the url paths, which have been internationalized for the target audience. Example: while an English-based app might make a @wagenet I can make a microlib just for this particular issue, exposing a |
Please reread peters comment, there isn't a double decode and it doesn't matter if the browser doesn't percent encode Unicode chars |
@kselden yes it isn't a double-decode, I was just pointing out that we should allow reasonably imaginable use cases of entry by the internationalized characters themselves. |
|
the thing you are pointing out with location.href is not the same issue as the location.hash difference |
Wait a minute... then why don't we just slap a My apologies for my own unfamiliarity on this url-encoding issue. slaps myself in face |
@wagenet I think we shouldn't
I'll submit a PR once we agree on this one. |
You have to encode params as they may have URL meaningful characters. |
I definitely think encoding should be default. I'm not opposed to an |
I am still skeptical about the need to be URL-safe at this level. As far as I can understand the source, the stored URL is used for:
(1), at the browser level, presumably it has its own standard of encoding/decoding all states, such that it can know that pushing a state with url (2), there are still browser inconsistencies regarding whether it would automatically encode/decode the url, and at which url parts - that, we can't do anything about it. But what we can do, as I have shown in the above snippet, is to "encourage" the browser to display the decoded form by passing in a decoded form at first. - As for why the decoded form? Like I have said earlier, I venture to say that most devs would prefer a pretty, decoded url, instead of an encoded one, as long as they can get away with it. (3) this occurs, most likely, when the user presses
Maxthon failed either way, so it doesn't matter whether we encode or decode the unsafe characters. IE, however, succeeded in loading (or deserializing) a url when given in encoded form, but failed when not encoded. This, I believe, is the use case that @kselden has in mind. (I don't have any apple-based products, so I have no test results from those platforms.) From this I also conclude that the standard browser behavior should be allowing URLs with decoded/unsafe characters, decode them on-the-fly when needed, and displaying either the encoded or decoded form in the address bar. - although I cannot find any spec relating to this issue. Ideally I would like to hear more from other devs and get a consensus on this trade off. Essentially it's "human-friendly-display" vs "IE support". Obviously I lean on the former, because from the look of it, IE (again!) is implementing non-standard behaviors, but I'd respect any community consensus. P.S. There is also the use case of passing in some urls to ajax requests, but in that case, the responsibility of ensuring that the url is encoded should rest with the ajax call itself, instead of relying on the "input url" being encoded in the first place. For this reason I think that this is out of scope for this discussion. |
When faced with old IE vs other browsers, we've always supported IE by default with the ability to change the default. |
Also urls are external - so if a chrome user pastes a link in an email, it should work when sent to an IE user - it's not just a case of being able to have friendly urls for chrome users and different behaviour for ie because this breaks shareability which is fundamentally important IMHO. |
@wagenet @alexspeller Agreed on the compatibility and shareability arguments. PR over at #3545 |
Closing in favor of any further discussion on the PR. |
As reported in getoutreach/epf#71. @ghempton suggested that this might be an ember-level issue.
In summary, if your entry URL is
/search/便
(just a random non-safe character), the params object will give youObject {search_term: "%E4%BE%BF"}
. Should it beObject {search_term: "便"}
instead?The text was updated successfully, but these errors were encountered: