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

link-to component no longer accepts special chars #14094

Closed
t4t5 opened this issue Aug 19, 2016 · 10 comments
Closed

link-to component no longer accepts special chars #14094

t4t5 opened this issue Aug 19, 2016 · 10 comments
Labels

Comments

@t4t5
Copy link

t4t5 commented Aug 19, 2016

Previously, it was possible to include the "@"-symbol in your URLs.

My router looks like this:

  this.route('user', { path: '/:userHandle' });

... and I could redirect to the path this way:

{{#link-to "user" "@tristan"}}
  Go to profile!
{{/link-to}}

This worked in 2.5 (it transitioned to /@tristan), but in the recent versions it instead changes the URL to /%40tristan.

What's the reason for this change, and is there a way to turn it off?

@Serabe
Copy link
Member

Serabe commented Aug 19, 2016

I think this is related to the fix of #4794. Not sure if we should consider this a bug or not.

@rwjblue
Copy link
Member

rwjblue commented Aug 19, 2016

I think @bantic and/or @nathanhammond would have the best understanding of this. Would love for them to review/reply.

@rwjblue
Copy link
Member

rwjblue commented Aug 19, 2016

The changes in question landed in Ember 2.7.0 (and have been reverted in current release branch, to be released as 2.7.2). They are still included (with bug fixes for the regressions identified) in the current beta (which will become 2.8.0).

The changes regarding encoding/decoding of links (including dynamic segments) were made in tildeio/route-recognizer#91. There is specific discussion there about @ (in the context of a URL reserved character).

@bantic
Copy link
Member

bantic commented Aug 19, 2016

One of the changes in tildeio/route-recognizer#91 was to help make it so that route generation and recognition are symmetric (that is, generating a route with a dynamic segment would always create a string that would then be recognized by the route-recognizer). Before that change it was possible to generate routes that would not be recognized, for example, if you had the route /:userHandle and you did {{#link-to "user" "abc/def"}} the url that is generated would be "/abc/def", which route-recognizer would no longer recognize (it doesn't match /:userHandle).

After that PR the part of a dynamic segment is always encoded via encodeURIComponent so that, e.g. "abc/def" becomes "abc%2Fdef". This happens in the generate method for a dynamic segment here.

I believe if you opt out of the improved encode/decode by setting RouteRecognizer.ENCODE_AND_DECODE_PATH_SEGMENTS = false; in your app, you'll see the behavior you're looking for (the "@" will appear in the URL as-is).

@bantic
Copy link
Member

bantic commented Aug 19, 2016

For context, encodeURIComponent will percent-encode any character in the uri reserved set (these: ; / ? : @ & = + $ ,) as well as non-ascii characters (like emoji). (The JavaScript spec for encodeURIComponent is here.)
And the URI spec for what makes up a path is here.
RFC 3986 implies that the following characters should be allowed (i.e., not percent-encoded) in path segments:
! $ & ' ( ) * + , ; = : @.
I will take a stab at updating route-recognizer to explicitly allow those characters (un-encoded) when generating dynamic segments.

@t4t5
Copy link
Author

t4t5 commented Aug 19, 2016

Thanks for the clarification @bantic! Seem overall like it's a good fix, but that some apps could get some nasty side-effects from it. 👍

Just one question: how can people set RouteRecognizer.ENCODE_AND_DECODE_PATH_SEGMENTS to false in their Ember CLI apps without forking the Ember library's route?

I naively tried setting:

var ENV = {
    RouteRecognizer: {
      ENCODE_AND_DECODE_PATH_SEGMENTS: false,
    },
}

... in the /config/environment.js-file, but that does not seem to work.

bantic added a commit to bantic/route-recognizer that referenced this issue Aug 19, 2016
During route generation, for dynamic segments, do not percent-encode
characters that, according to RFC 3986, are allowed in path segments.

RFC 3986 defines "sub-delims" (these chars: `! $ & ' ( ) * + , ; =`) and
specifies that they along with `:` and `@` are allowed in path segments
in URIs. See: https://tools.ietf.org/html/rfc3986#section-3.3

This commit changes RouteRecognizers generation code for dynamic
segments to explicitly avoid encoding those characters.

Fixes emberjs/ember.js#14094
@bantic
Copy link
Member

bantic commented Aug 19, 2016

@t4t5 Hmm, looking now I'm not sure RouteRecognizer is explicitly exposed for you to set the flag on. I'll check on that.

I have a fix for RR (here: bantic/route-recognizer@23455b6) that I will PR in a little bit after I have a chance to clean it up.

bantic added a commit to bantic/route-recognizer that referenced this issue Aug 19, 2016
During route generation for dynamic segments, do not percent-encode
characters that, according to RFC 3986, are allowed in path segments.

RFC 3986 defines "sub-delims" (these chars: `! $ & ' ( ) * + , ; =`) and
specifies that they along with `:` and `@` do not need to be encoded in path segments
in URIs. See: https://tools.ietf.org/html/rfc3986#section-3.3

This commit changes RouteRecognizer'ss generation code for dynamic
segments to explicitly avoid encoding those characters.

Fixes emberjs/ember.js#14094
@bantic
Copy link
Member

bantic commented Aug 19, 2016

@rwjblue Here's a PR that should fix the problem in RR: tildeio/route-recognizer#109

@nathanhammond
Copy link
Member

@bantic RouteRecognizer isn't exposed to be able to set that flag from Ember land, but that's something I would prefer to keep. As of 0.3.0 I'd prefer to remove the "legacy" encoding behavior (and that flag) and stick with the behavior from RFC 3896 as "correct".

bantic added a commit to bantic/route-recognizer that referenced this issue Aug 20, 2016
During route generation for dynamic segments, do not percent-encode
characters that, according to RFC 3986, are allowed in path segments.

RFC 3986 defines "sub-delims" (these chars: `! $ & ' ( ) * + , ; =`) and
specifies that they along with `:` and `@` do not need to be encoded in path segments
in URIs. See: https://tools.ietf.org/html/rfc3986#section-3.3

This commit changes RouteRecognizer'ss generation code for dynamic
segments to explicitly avoid encoding those characters.

Fixes emberjs/ember.js#14094
@nathanhammond
Copy link
Member

Addressed upstream, can be closed.

@locks locks closed this as completed Aug 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants