remix-run / history

Manage session history with JavaScript
MIT License
8.28k stars 961 forks source link

LocationUtils should not run pathname through decodeURI #786

Open quisido opened 4 years ago

quisido commented 4 years ago

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 commented 4 years ago

Agreed, this line is breaking things for me too.

This is the cause of https://github.com/ReactTraining/react-router/pull/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 commented 3 years ago

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 commented 3 years ago

@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.