React-router: improper routing when the param contains a slash

[ztmr]

Let item/:id be a route and item.id a key used to lookup the item in a database. If the item.id contains special characters barring the slash, the URL is being generated correctly. If the item.id contains one or more slashes /, none of them is escaped and causes routing issues -- especially when there is another route of item/:id/....

I have experimented with the following:

--- Path.js     2014-09-24 12:17:32.944015842 +0200
+++ Path.js.new 2014-09-24 12:16:51.104016891 +0200
@@ -52,7 +52,7 @@
     }

     var compiled = compilePattern(pattern);
-    var match = URL.decode(path).match(compiled.matcher);
+    var match = path.split('/').map(URL.decode).join('/').match(compiled.matcher);

     if (!match)
       return null;
@@ -97,7 +97,7 @@
       );

       // Preserve forward slashes.
-      return String(params[paramName]).split('/').map(URL.encode).join('/');
+      return URL.encode (params[paramName]);
     });
   },

...but there is still regex that matches params, so I stopped hacking the project and started to ask why the hell somebody took so much of the special care to preserve the slashes in the parameters.

In general, we don't use slashes in object identifiers but we're currently doing migration of some ancient data where the slash sometimes appear in the object id and it is not feasible to remove it across all the object references. Also, doing extra param encoding/decoding when passing it to the link and withdrawing it from the target component properties would work, but it is a serious nuisance.

Is there any special reason for keeping the original behavior? Can we work it out somehow?

[ryanflorence]

Slashes are delimiters in urls. How would you expect it to handle a param with the url delimiter in it to work?

[ztmr]

Think of something like this: <Link to="/foo/bar/:id" params={{ id: 'hello/world' }} />. The resulting URL should be /foo/bar/hello%2Fworld, not /foo/bar/hello/world.
That means that the URI-encoding should be done on each of the parameter value before joining the full URL path. The other way is to URI-decode parameters after splitting the URL path.
That was what I wanted to say using the diff example...

We currently have to work it around through: <Link to="/foo/bar/:id" params={{id: encodeURIComponent('hello/world')}} /> and then use decodeURIComponent (this.props.params.id) from the component being referred.

...and that sucks more and more with the number of parameters you have, n'est-ce pas?

[ryanflorence]

I'm pretty neutral on this. Having slashes in IDs is a pretty strange edge
case, with an acceptable solution, but I'd be open to a pull request that
doesn't have any trade-offs to support your case.

On Saturday, October 4, 2014, Tomas Morstein [email protected]
wrote:

Think of something like this: 'hello/world' }} />. The resulting URL should be /foo/bar/hello%2Fworld,
not /foo/bar/hello/world.
That means that the URI-encoding should be done on each of the parameter
value _before_ joining the full URL path. The other way is to URI-decode
parameters _after_ splitting the URL path.
That was what I wanted to say using the diff example...

We currently have to work it around through: params={{id: encodeURIComponent('hello/world')}} /> and then use decodeURIComponent
(this.props.params.id) from the component being referred.

...and that sucks more and more with the number of parameters you have,
n'est-ce pas?

—
Reply to this email directly or view it on GitHub
https://github.com/rackt/react-router/issues/313#issuecomment-57917778.

[ztmr]

The discussion is not about the weirdness of having slashes in IDs, but about the correctness of the routing encoding/decoding mechanism. We're not so happy to have slashes in IDs too, but we do and have to deal with it.

I simply think the current behaviour is wrong, no matter if I personally need it or not. If somebody states that the parameter has a value of xyz, the router should deliver the value to the target component unchanged -- without complaining whether it is xyz or x/y/z or judging if the parameter value is "nice" or "ugly."

When I saw the code, I couldn't understand why someone took so much of special care to not URI-encode slashes in params and rather URI-encode the whole URL after it was constructed. It does not make sense to me, so that's why I have opened this issue instead of pull-request. I'm just wondering what the author of that code had on his mind when writing that code. There might be a good reason for doing so and I only cannot see it...

What trade-offs do you think? I can't see any, I see just a potential bugfix, because the current behaviour definitely sounds like a bug to me.

I can also imagine some security implications, but that's quite another story.

[ryanflorence]

Sorry, I wasn't reading clearly earlier :

This is totally a bug, we should handle escaped slashes in params.

[ryanflorence]

working on this, please follow #367

[mingfang]

I'm still seeing this problem with the latest version.
Was there a fix?

[bripkens]

We are running into this issue as well and we will also need to encode and decode URI params manually. Unfortunately this is not a scalable solution as @ztmr has mentioned.

[michaelperrin]

I ended up encoding my route parameter in my link:

<Link to={"/my/path/" + mySlugString.replace(/\//g, '%2F')}>link</Link>

[mertkahyaoglu]

Have the same problem 😞

[taion]

pathname is just a string; obviously we can't blindly encode all the / characters in the string, so you'll need to call encodeURIComponent yourself on any params that need to be escaped

[ifokeev]

@taion why we can't match routes with regex? I need slashes too and have no idea how to resolve this issue.

[taion]

Because it's just not a feature yet. And won't be unless we have a way to move to path-to-regexp entirely for path regexps.

[bl-nero]

We've been just struck by this. We have a splat that contains a breadcrumb of product categories; the thing is, one of the category IDs has a slash that is encoded as %2F, and the react-router "intelligently" decodes it, corrupting our IDs. Is there any way to turn off this behavior?

[Brisros]

I had an issue like that, the solution was

item/:id*

[ChadRidings]

Actually some id's do have slashes and it's more common than you'd think (whether it's best practice or not). Take the "Entertainment Identifier Registry" for example. Every EIDR has a slash in it. This is troublesome when setting up param's in the route... as it treats everything before the slash in the EIDR as a directory.

[catonymous]

Why this one is closed? There is a lot of other kinds of params that have to contain slashes. Express router decodes them correctly, but react-router doesn't yet. Shame on you, guys.

P.S. You can fix this by replacing %2F with / for values[index] at https://github.com/ReactTraining/react-router/blob/f24fb54323245db093b8658d6338c019148642da/packages/react-router/modules/matchPath.js#L51 because decodeURI keeps %2F encoded at https://github.com/ReactTraining/history/blob/4aeb6693758bbaf221274b5894218e8e0b057db4/modules/LocationUtils.js#L37.

P.P.S. For now I import following monkey-patch.js into my index file:

import { Route, matchPath } from 'react-router';

if(matchPath('%2F', ':test').params.test !== '/') {
    const matchEncodedSlashes = Route.prototype.computeMatch;
    Route.prototype.computeMatch = function() {
      const match = matchEncodedSlashes.bind(this)(...arguments);
      if(match && match.params) {
        const params = match.params;
        for(const key in params) {
          params[key] = params[key].replace(/%2F/ig, '/');
        }
      }
      return match;
    };
}

[mrchief]

This issue was closed in favor of #367 which was closed due to this reasoning.

The reasoning being:

The reason I want to preserve slashes in params is for when you need to make some file browsing URL scheme. So, e.g. if you have a route that matches /files/:filepath and your filepath is the/path your URL will still look like /files/the/path, which is nicer than /files/the%2Cpath.

So if I'm following this correctly, this issue simply got neglected. I cannot imagine majority of web apps constructing file based urls. Maybe in a native app but can't see a reason to neglect a core issue in web app (where majority of usage lies) to support an edge case.