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

decodeURI in code breaks my encoded URLs #745

Open
vass-david opened this issue Oct 7, 2019 · 8 comments
Open

decodeURI in code breaks my encoded URLs #745

vass-david opened this issue Oct 7, 2019 · 8 comments

Comments

@vass-david
Copy link

Link to code that causes issues: https://github.com/ReactTraining/history/blob/7b8ab30f98855c31e74eb72f2592a8fff9d7d3ac/modules/LocationUtils.js#L36

I found that there were some issues raised a long while ago, but it seems that recently this has became an issue again.

My use case:
I'm receiving object names from API which are created by user. I can't control what's coming and I don't want to limit users from using special characters in their names. But even if I call encodeURIComponent on the name of an object, url will be decoded anyway and a user is navigated to wrong/faulty URL.

It's hard to tell what's the reason of decodeURI part of code, since commit name is "Undo changes from 78f016 and 0fe9cb" but github search wasn't able to find commits 78f016 or 0fe9cb

Thanks for help

@kaskelotti
Copy link

I'm working also with problems related to decodeURI. The commit for undoing changes stated in the message

This is a temporary change so we can get the master branch back into a
state to cut 4.x releases.

Now the question is, what is expected to happen and when? Is it best to fallback to using version 4.9.0 where this change was introduced?

@vass-david link to 78f016 commit

@michalkvasnicak
Copy link
Contributor

We have this problem too, links where we need to have 100% in them are completely broken.

@Obi-Dann
Copy link

I think it should be decodeURIComponent instead of decodeURI. Replacing it locally, fixes the issue for me. decodeURI is supposed to be used with full URIs https://stackoverflow.com/a/747700

@kaskelotti
Copy link

kaskelotti commented Oct 23, 2019

This problem is pretty thoroughly discusses in #505 with possible solutions and workarounds, and what possible issues they might cause and why they might not be applicable in some specific cases. Please see that thread. We probably don't need another topic for that :) The discussion related to the problem has been ongoing for at least two years.

I think the real question is that why the fix to this issue was apparently taken out once it got introduced? Was there still something unexpected? And is it probable that there will be a fix-it-once-and-for all-solution or does every app just need their own work around to overcome this?

@Obi-Dann
Copy link

Thanks for the reference to #505, it looks like the solution was to stop decoding URLs. There was a PR merged #656 removing decoding, but then it was reverted to prepare a release from master branch 7b8ab30 and later added to dev branch f55f686.
Perhaps it's something to be release in v5?
I created a fork from the dev branch and everything works like a charm now

@kanekv
Copy link

kanekv commented Oct 24, 2019

Also there was a bug earlier with params not being decoded. Is it a norm right now for them not to be decoded?

@felixfbecker
Copy link

Despite #505 having been closed by it, this was not fixed with #656. Link still decodes links, which breaks them if they e.g. contain an encoded %. 4.5.1 is still the last working version.

@djmitche
Copy link

djmitche commented Nov 6, 2020

Here's a simple reproduction showing at least two bugs: https://codesandbox.io/s/heuristic-sun-revzx?file=/src/App.js

/**
 * Issue 1: urldecoding takes place on the path in Link's to prop, so "singly"
 * and "doubly" generate the same URL in the address bar
 *
 * Issue 2: history is parsed differently when loading the page; to see, click
 * "triply" and observe match.params (`%252f`), then reload (now `%2f`).
 */

export default function App() {
  return (
    <Router>
      <div>
        <ul>
          <li>
            <Link to="/url/singly-encoded-%2f">singly</Link>
          </li>
          <li>
            <Link to="/url/doubly-encoded-%252f">doubly</Link>
          </li>
          <li>
            <Link to="/url/triply-encoded-%25252f">triply</Link>
          </li>
        </ul>
        <Switch>
          <Route path="/url/:someUrl" component={Url} />
        </Switch>
      </div>
    </Router>
  );
}

function Url({ match }) {
  return <pre>{JSON.stringify(match, null, 2)}</pre>;
}

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

7 participants