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

LocationUtils should not run pathname through decodeURI #786

Open
quisido opened this issue Apr 19, 2020 · 3 comments
Open

LocationUtils should not run pathname through decodeURI #786

quisido opened this issue Apr 19, 2020 · 3 comments

Comments

@quisido
Copy link

quisido commented Apr 19, 2020

https://github.com/ReactTraining/history/blob/master/modules/LocationUtils.js#L36

I have the following pathname:
:a/:b where a and b can contain symbols (so long as they are encoded)

Example:

encodeURIComponent('go/od') + '/' + encodeURIComponent('b%ad')

This example gives me the expected, valid path: go%2Fod/b%25ad.

However, when this path is used in history, createLocation('go%2Fod/b%25ad', ...irrelevant), it gets incorrectly mutated by line 36 and becomes go%2Fod/b%ad, because decodeURI replaces %25 with % but does not replace %2F with /.

The path is not go%2Fod/b%ad, and this causes the path to later be incorrectly parsed where %ad is a URI encoded character instead of a string literal.

What is the reason for decodeURI here and how is it accurate?

While I am encountering this error at a higher level (React Router DOM), the error seems to be in this package.

<Link to={`/user/${encodeURIComponent('go/od')}/filter/${encodeURIComponent('b%ad')}`>view user go/od with filter b%ad</Link>
// becomes correct syntax:
<Link to="/user/go%2Fod/filter/b%25ad">view user go/od with filter b%ad</Link>
// /user/go%2Fod/filter/b%25ad <-- expected output href
// /user/go%2Fod/filter/b%ad <-- actual, unexpected output href
vpstuart pushed a commit to EBMSPTYLTD/history that referenced this issue May 19, 2020
decodeURI decodes paths in a very strange way. This should hopefully solve remix-run#786.

This will need to be coupled with a PR in react-router remix-run/react-router#6578.
@vpstuart
Copy link

Agreed, this line is breaking things for me too.

This is the cause of remix-run/react-router#6578 and might be blocking it?

I've done a PR, but I'm a novice at Github and this project so I guess.. we'll see what happens.

@project707
Copy link

project707 commented Mar 5, 2021

FYI this broke use of the library for us as well. We had customers reporting some bug that we thought was something we were doing wrong, and it went unfixed for months until I finally figured it out and made a fork that removed that line and the error handling.

@vpstuart
Copy link

vpstuart commented Mar 8, 2021

@project707 we ended up using https://webpack.js.org/plugins/normal-module-replacement-plugin/ to replace the impacted file, but still be able to use the normal build of react-router and other files of history.

Just comment out this part of history.js.

  //try {
  //  location.pathname = decodeURI(location.pathname);
  //} catch (e) {
  //  if (e instanceof URIError) {
  //    throw new URIError('Pathname "' + location.pathname + '" could not be decoded. ' + 'This is likely caused by an invalid percent-encoding.');
  //  } else {
  //    throw e;
  //  }
  //}

You do have to do additional work at the other end to decode your params, but I've just routed through a single component.

function RoutableShim(props: { render: (props: RouteComponentProps) => React.ReactNode }) {
    const history = useHistory();
    const location = useLocation();
    const match = useRouteMatch();

    const correctedParams = {};

    Object.keys(match.params).forEach(key => {
        const value = match.params[key];

        let decoded = typeof value == 'undefined' ? undefined : decodeURIComponent(match.params[key]);

        correctedParams[key] = decoded;
    });

    const copyOfMatch = Object.assign({}, match);
    copyOfMatch.params = correctedParams;

    const innerProps: RouteComponentProps = {
        history: history,
        location: location,
        match: copyOfMatch
    };

    return <React.Fragment>{props.render(innerProps)}</React.Fragment>;
}

export function renderRouteComponent(Component: React.ComponentType<RouteComponentProps>) {
    return <RoutableShim render={props => <Component {...props} />} />;
}

export function renderRouteFunction(callback: (props: RouteComponentProps) => React.ReactNode) {
    return <RoutableShim render={callback} />;
}

Hopefully this helps other people who find this issue. I believe its no longer an issue in react-router 6.

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

No branches or pull requests

3 participants