React-router: [Bug] Unicode routes

[voodooattack]

Using a Unicode route doesn't work.

Version

4.0.0-alpha.6

Test Case

<Match pattern="/اضف" component={AddPlace} />

Steps to reproduce

Simply use any Unicode route. Neither client nor server-side rendering will match.

Expected Behavior

React-router should match the route correctly.

Actual Behavior

The route is missed.

[pshrmn]

This appears to be a path-to-regexp issue that has been discussed but not yet addressed.

I would have to look into it more, but I think that the pathname that matchPattern tests against is encoded. It is possible that you could write a <MatchUnicode> that automatically encodes the pattern for you:

const MatchUnicode = ({ pattern, ...rest }) => (
  <Match pattern={encodeURI(pattern)} {...rest} />  
)

<MatchUnicode pattern="/اضف" component={AddPlace} />

[voodooattack]

@pshrmn That works! Thanks!

[voodooattack]

Update: Apparently this only works with server-side rendering, and not client side. If I navigate to the page inside the client it doesn't render the matches.

[pshrmn]

I just did a little bit of testing (albeit in the unreleased v4 branch, but that should not affect the matching here) and the unicode pattern /اضف was being matched using a regular <Match> for me. Could it just be an issue where on the server you need to decode the req.url prior to passing it to your <ServerRouter>?

[voodooattack]

@pshrmn I tried to decode the URL on the server, it doesn't make a difference because the server renders correctly, it's only when the client mounts that things go askew.

[voodooattack]

Apparently changing this line from:

    const match = matcher.regex.exec(location.pathname)

To:

    const match = matcher.regex.exec(decodeURI(location.pathname))

Fixes the problem, I'm not sure why though.

[timdorr]

Should be fixable if you provide your own history to the router that decodes the URI before passing it back to listeners. That's not super-easy in 4.0 alpha 6, but it's a lot easier in the latest version

[mjackson]

Seems like this is out of scope for us. We just pass whatever <Route path> (was <Match pattern>) you give us directly through to path-to-regexp.

Perhaps you'd like to follow up with them, @voodooattack? Or is there something you think we need to do on our side?

[timdorr]

Is this still relevant in the beta version?

[pshrmn]

If a user is navigating within their app and they push a location with a decoded unicode pathname, it will remain decoded. The issue seems to be that when the history instance is being created, it grabs window.location.pathname, which is encoded.

@mjackson should history be decoding the pathname it grabs from window.location in getDOMLocation? Or possibly add a decode option so that it can, but not by default?

[mjackson]

I'm wondering why no one has ever mentioned decoding URLs in the history repo before now. It seems like such an obvious oversight, but for some reason hasn't been done yet.

Yes, history (both browser and hash history) should be decoding URLs it gets from the browser, and encoding them again before a push/replace.

[pshrmn]

I think that this is the same issue that @bkniffler was having in #4124

[mjackson]

Ya, probably. It threw me off when he used a <MemoryRouter>, but this comment clarified it.

[pshrmn]

@mjackson Would you like a PR for this? I think that it as simple as calling encodeURI in createPath and decodeURI in parsePath.

[clembou]

Just chiming in, I think I hit a related issue when upgrading from v4.0.0-alpha6 to v4.0.0-beta6.

In my case direct links to urls with a space ( encoded as %20) no longer work, they used to work in the alpha version.

I tracked it down to a decodeUriComponent that was present in alpha but is missing from the beta.

[bkniffler]

Any news on this? There are still problems due to what @pshrmn pointed out and I guess it wouldn't be to hard to fix:

"If a user is navigating within their app and they push a location with a decoded unicode pathname, it will remain decoded. The issue seems to be that when the history instance is being created, it grabs window.location.pathname, which is encoded.

@mjackson should history be decoding the pathname it grabs from window.location in getDOMLocation? Or possibly add a decode option so that it can, but not by default?"

[bkniffler]

Okay, fixed it for now by creating a custom BrowserRouter component

export class BrowserRouter extends Component {
  static propTypes = {
    basename: PropTypes.string,
    forceRefresh: PropTypes.bool,
    getUserConfirmation: PropTypes.func,
    keyLength: PropTypes.number,
    children: PropTypes.node
  }

  constructor() {
    super();
    const history = createHistory(this.props);
    history.location.pathname = decodeURIComponent(history.location.pathname);
    this.history = history;
  }

  render() {
    return <Router history={this.history} children={this.props.children} />;
  }
}

[pshrmn]

I opened ReactTraining/history#442, which I believe should fix this issue.

[mjackson]

This was fixed when we upgraded to history 4.6.0 in 7279d22276814d3d6ad2c799a9400e704e33b4b6. Added a test to prove it in 95f5cf9.