History: Path is decoded in createLocation

[ArthurKnaus]

This results in an invalid URL being pushed to the HTML5 history element when there are encoded percent signs in the pathname.

E.g.:

history.push({
  pathname: `randomlocation/${ encodeURIComponent("AB%C") }/furthermore`
});

results in "randomlocation/AB%C/futhermore" being pushed to the HTML5 history.

[pshrmn]

Alright, so just to be clear, the reasoning behind decoding in createLocation is that it allows us to have location object's whose pathname is decoded. That means that React Router users can use decoded strings in their route paths.

// with decoding
<Route path='/역사' />
// without decoding
<Route path='/%EC%97%AD%EC%82%AC' />

Another effect of this is that params will be automatically decoded.

<Route path='/greeting/:word' />
// user visits /greeting/안녕하세요
match === {
  ...,
  params: { word: '안녕하세요' }
  // without decoding this would be %EC%95%88%EB%85%95%ED%95%98%EC%84%B8%EC%9A%94
}

History then relies on the browser to automatically encode the pathname when we push to the browser. This appears to work for all characters except for the percent sign.

There are a few thought I have on possible ways to deal with this:

Stop all decoding

I don't think that this is desirable, but I'll at least throw it out there. We could leave the decoding to the program that is using history's locations. location.pathname would just be what the user provides. Of course, this would lead to to inconsistencies between the locations created from window.location and locations created from values passed by the user.

// URI = '/역사'
// window.location.pathname = '/%EC%97%AD%EC%82%AC'
{ pathname: '/%EC%97%AD%EC%82%AC', ...}

history.push('/역사')
{ pathname: '/역사', ...}

Add a raw property

Before we decode the pathname, we can store the original value as location.raw (or maybe rawPathname, name isn't important here). Then, when we create a URI, we would concatenate using location.raw instead of location.pathname.

Prevent decoding of %25

Instead of just calling location.pathname = decodeURI(location.pathname), we could modify that to skip decoding the character string %25, which is % encoded.

const decodeButSkipPercent = pathname => (
  pathname.split('%25')
    .map(decodeURI)
    .join('%25')
)

That would work, but we would also end up with location's whose pathname is not necessarily fully decoded. The exception should only apply to the encoded percent sign, but it is still an exception.

---

I'm not sure which of these is the best choice (or maybe there is a better alternative that I did not think of?) :woman_shrugging:

[dominykas]

Of course, this would lead to to inconsistencies between the locations created from window.location and locations created from values passed by the user.

I'm sorry, I'm not familiar with the code, but is it not possible to decodeURI only and only when creating a location from window.location? I'd assume the apps/devs would never read the location directly from there, because they access it via this lib? This is the place where history _does_ know the context and can behave accordingly?

[kellyrmilligan]

what is the recommended workaround for this? in components, the route params are only partially decoded, see referenced issue. we have file names that are coming from users, and sometimes they have ? in the name.

[janhoeck]

I have the same problem.
I try to push this to the history:
history.push('/data/Amazing%20Example')

and history converts this to the url: http://localhost:3000/data/Amazing Example

History removes my empty space! But why :)

[Kapaacius]

I guess I have a similar issue as @janhoeck
I want to encode part of my link because it contains stringified object
to={ /123/${encodeURIComponent(JSON.stringify({ encode: "me" }))} }
But when I now click on the link, it ends up being
#/123/{"encode"%3A"me"}
this on the url...it kinda works but what dies for me is when someone pastes such URL to slack or some other location. Clicking on that does another set of encoding magic and then the url does not work anymore.

How can I work around it? ( 2x encodeURIComponent is not really a nice fix...but would kinda help right now :D )

[OliverJAsh]

@pshrmn

History then relies on the browser to automatically encode the pathname when we push to the browser. This appears to work for all characters except for the percent sign.

Do you know why browsers don't encode the % character, but encode everything else? Is it a bug or intentional for some reason? Is it applicable in all browsers?

[pshrmn]

@OliverJAsh Browsers use percent-encoding, so if they encoded the %, that could break pre-encoded paths. From https://tools.ietf.org/html/rfc3986#section-2.4 (emphasis mine).

Because the percent ("%") character serves as the indicator for
percent-encoded octets, it must be percent-encoded as "%25" for that
octet to be used as data within a URI. Implementations must not
percent-encode or decode the same string more than once, as decoding
an already decoded string might lead to misinterpreting a percent
data octet as the beginning of a percent-encoding, or vice versa in
the case of percent-encoding an already percent-encoded string.

[OliverJAsh]

Ah. I've noticed browsers also do this when you enter a URL in the address bar, e.g. https://google.com/?search=sale 50% off redirects to https://google.com/?search=sale%2050%%20off instead of the correct https://google.com/?search=sale%2050%25%20off.

IIUC: as a user, when entering the URL in the address bar, we know the query params are not encoded, but the computer does not!

[OliverJAsh]

Perhaps one argument in favour of stopping all decoding in this module is for consistency with HTML5's native history. Currently we have this inconsistency:

history.push(`/foo/${encodeURIComponent('foo % bar')}`)
// => URL is "/foo/foo%20%%20bar"
// => `history.location.pathname` is `/foo/foo % bar`

window.history.pushState(null, '', `/foo/${encodeURIComponent('foo % bar')}`)
// => URL is "/foo/foo%20%25%20bar"
// => `window.location.pathname` is `/foo/foo%20%25%20bar`

Both the generated URL and pathname are different in these two cases.

[mjackson]

I think my vote is to make our push as consistent with pushState as we can. That just makes sense.

@pshrmn Isn't there some other way we can provide decoded parameters to RR users? Instead of decoding the pathnames in history, can we do it later in the router (i.e. just before we need to match with path-to-regexp)?

[pshrmn]

RR could decode in matchPath (or maybe higher up the chain/memoized so it isn't called as often). Off the top of my head I can't recall if there was a reason that was avoided before, but I want to say that the idea was that doing it in history would make it available to all history users, not just RR users.

If decoding is removed, there will be an inconsistency in location objects unless history expects to be given encoded pathnames. Whether or not there are real consequences to this, I am not sure (pushing the same location and whether that pushes/replaces is the one that comes to mind).

[pshrmn]

I haven't thought through the implications, but if you wanted to enforce that pathnames are encoded for consistency, that could be done using an anchor. That would leave encoded ones alone, but ensure anything that should be encoded is.

function encode(pathname) {
  const a = document.createElement('a');
  a.setAttribute('href', pathname);
  return a.pathname;
}

[OliverJAsh]

@pshrmn

This appears to work for all characters except for the percent sign.

It also appears this doesn't work for & characters. To test this:

<a href="https://google.com/foo & bar"/>go</a>

When clicked, the browser navigates to https://google.com/foo%20&%20bar (& is not URI encoded), instead of the https://google.com/foo%20%26%20bar (& is URI encoded).

I don't know whether this needs to be considered as part of this or not—just pointing it out.

[pshrmn]

@OliverJAsh that is the search segment of a URI, which the history package doesn't touch.

[OliverJAsh]

@pshrmn I've updated my example to show the same problem for pathnames.

[OliverJAsh]

@pshrmn In fact it would seem the browser only URI encodes spaces, unlike encodeURIComponent. Example: http://output.jsbin.com/ceqerer/1.

I presume it's using encodeURI under the hood, which unlike encodeURIComponent doesn't encode reserved characters (&, …).

(My example uses query parameters for easy testing, but the same browser mechanism is used for pathnames.)

[pshrmn]

I'm trying to remember exactly what I was talking about in that comment (it _has_ been six months since I wrote it). I believe that what I meant by "this appears to work for all characters except for the percent sign" is that the percent sign is the only character that we cannot safely decode and expect the browser to properly encode.

From MDN, decodeURI (which is what history is using to decode pathnames) does the following:

Replaces each escape sequence in the encoded URI with the character that it represents, but does not decode escape sequences that could not have been introduced by encodeURI. The character “#” is not decoded from escape sequences.

Also from MDN, the following characters (which does not include the percent sign) are not encoded by encodeURI (so the encoded sequences for these characters will not be decoded by decodeURI):

A-Z a-z 0-9 ; , / ? : @ & = + $ - _ . ! ~ * ' ( ) #

history.push('/test&ing');
const decoded = decodeURI('/test&ing'); // /test&ing
history.pushState(null, '', decoded)
// window.location.pathname = '/test&ing'

decodeURI won't change the escape sequence %26 because it could not have been introduced by encodeURI

history.push('/test%26ing');
const decoded = decodeURI('/test%26ing'); // /test%26ing
// window.location.pathname = '/test%26ing'

encodeURI can encode a percent sign (encodeURI('%') === '%25'), so decodeURI will decode %25. However, the browser will not encode the percent sign (because it would not want to encode %26 into %2526), which is the source of this issue.

history.push('/test%25ing'); // %25 = encodeURI('%')
const decoded = decodeURI('/test%25ing'); // /test%ing
// window.location.pathname = '/test%ing'

[OliverJAsh]

@pshrmn That makes sense!

IIUC, this issue only concerns the percent symbol and not other characters that will be decoded by decodeURI because the percent symbol is particularly problematic as the browser will not re-encode it, unlike other characters.

However, it is quite surprising to find that some characters in location.pathname (and location params in React Router) are decoded whilst others are not. As you have explained, this is because decodeURI is only permitted to decode some characters. However, I would expect consistency with window.location.pathname:

const myLocation = history.createLocation({ pathname: `/search/${encodeURIComponent('foo & bar')}` });

historyInstance.push(myLocation);

myLocation.pathname
// => "/search/foo %26 bar"

window.location.pathname
// => "/search/foo%20%26%20bar"

Even if most of the decoded characters are correctly re-encoded by the browser, this is still an issue when working with location.pathname (or location params in React Router) directly.

I think this is also called out in https://github.com/ReactTraining/react-router/issues/5607.

[OliverJAsh]

For anyone who runs into this issue and is looking for workarounds…

On unsplash.com we are working around this by double URI encoding:

history.push({
  pathname: `randomlocation/${ encodeURIComponent(encodeURIComponent("AB%C")) }/furthermore`
});

This results in randomlocation/AB%25C/futhermore being pushed to the HTML5 history, which is as expected.

However, this has unintended side effects.

If a user navigates directly to randomlocation/AB%25C/futhermore, location.pathname will be randomlocation/AB%C/futhermore (decoded).

If we push randomlocation/AB%25C/futhermore to the user's history using the above workaround (as randomlocation/AB%2525C/furthermore), location.pathname will be randomlocation/AB%25C/futhermore (encoded).

(The same applies to location params.)

To workaround this side effect, we run a "safe decode":

const safeDecode = (pathname) => {
  try {
    return decodeURIComponent(pathname);
  } catch (_error) {
    return pathname;
  }
}

I'm sure this workaround has further implications…

I'd be very happy to help getting this issue fixed. Is there enough consensus for us to go with one of the proposed solutions?

[kevinliang]

@OliverJAsh Double encoding doesn't work because if the user hits the back button then the forward button, the error will throw again.

[OliverJAsh]

@kevinliang What exactly would throw again? I tried what you said and I didn't get any exception.

[kevinliang]

it'll throw something like URIError: URI malformed. The reason why it would do that is because when you transition via back and then forward, the "forward" decodeURI will try to decode a "%" instead of %25.

For example, if the URL contains a parameter like https://domain.com/some%name and you tried to decodeURIComponent on some%name, your first time you will be sending over some%25%25name. ReactHistory will decode it to some%25name. Then you hit back and forward. ReactHistory will then decode that to some%name and if you try to call decodeURIComponent on that because you wanted to use a decoded version of the parameter, an error will throw. At least that's what's currently happening on my end =/
I guess if you don't get any exceptions, then it's all good!

[OliverJAsh]

@kevinliang I believe we mitigate that with our safeDecode function, as I demonstrated in https://github.com/ReactTraining/history/issues/505#issuecomment-360897304.

[kevinliang]

ahh my bad I missed that part. What would happen if (i know this is very unlikely) a user hit back and then forward again a 2nd time?

[kevinliang]

@OliverJAsh also with safeDecode, if the param ended up being something like %3AHello%, how would we handle that case?

Perhaps safeDecode can "re-encode" the percent on the catch and then return decodeURIComponent on that?

[AnshulM34]

Hi all
Since already discussed even if I encode url, The url is decoded itself.
Is there any way to encode url which shows in address bar..(I want /mypage/My%20Article) ...but it is decoded and it converts %20 to space....

[AnshulM34]

Apologies I found a workaround by double encoding the uri
{encodeURIComponent(encodeURIComponent(item.url))}
I currently donot know side effects

[AnshulM34]

It does have a side effect though it shows perfect in address bar but if anybody copies the link address by right clicking it is encoded twice...I don't know how to fix this.

[OliverJAsh]

I patched our local copy of history to remove all decoding of location.pathname. It fixes all my issues and I'm yet to see any side effects. Is there anything I could watch out for?

[OliverJAsh]

@pshrmn @mjackson How do you think we should move forward? I would be happy to help implement the solution, whatever we think is best. This is quite a common issue so I'm keen to get it solved.

[oliver-stripe]

Is perhaps a reasonable solution to do the re-encoding manually instead of relying on the browser? The browser can't encode % because it doesn't know if the provided value is already encoded or not, but we know our value is not because we've already decoded it. Seems like making the result of createHref here a URL with an encoded path should work:

https://github.com/ReactTraining/history/blob/master/modules/createBrowserHistory.js#L175-L179

[kadishmal]

What we ended up doing is:

• Imagine a scenario where we have links like #dashboard/:name.

• We add full real links to these a elements after manually encoding strings like:

  #dashboard/${encodeURIComponent(name)}}>{name}

• A sample name is:

  '~`!1@2#% 3$45^6&7*8()-_+={}[]\;"\'<,>.?'

• The link turns out to be:

  #dashboard/'~%60!1%402%23%25%203%2445%5E6%267*8()-_%2B%3D%7B%7D%5B%5D%5C%5C%3B%22%5C'%3C%2C%3E.%3F'

• This means we are not relying on manual history.push(...); action.

• When a user clicks on the link, the router detects this, since this is the native History API action.

• The underlying react-router match returns us an incorrectly, partially decoded name like:

  '~`!1%402%23% 3%2445^6%267*8()-_%2B%3D{}[]\%3B"\'<%2C>.%3F'

• So, we ignore this value and instead read the real URL directly from the URL like:

  // The underlying History module has an unexpected behavior where it incorrectly handles URLs with percentage % sign.
  // At the time of writing this is still an open issue https://github.com/ReactTraining/history/issues/505.
  // Therefore, we fall back to reading the properly encoded URL directly from the browser URL.
  // The forward slash is excluded from the scaling plan names, so the below is safe for our use case.
  const nameEncoded = location.hash.match(/[^\/]+\/([^\/]+)(\/.*)?/)[1];
  const name = decodeURIComponent(planNameEncoded);

This became our workaround.

[Prophet32j]

I'd like to see a permanent resolution on this that would not require the hacks listed above to safe decode values. The difference in behavior between a in-app routing and sharing a URL is a deterrent to my team upgrading to RR4.

@pshrmn your option of Add a raw property looks to be the best option so far IMO. It doesn't interfere with anyone's application and is handled completely by RR. It allows teams using RR3 and below to migrate to RR4 without another breaking change.

We heavily rely on URL encoded values in our routes because we have lots of special characters and spaces in our target resources. I don't want to base64 encode these names because the routes then become much less readable and not as "share-friendly"

[jh3141]

+1 for the vote of raw properties -- but honestly, any solution to this, which is a real pain to deal with for anyone using special characters in identifiers, would be better than nothing.

[OliverJAsh]

I've run into this issue for the second time, this time for a different reason.

We have server side rendering, and the location.pathname created by React Router's StaticRouter is different from the location.pathname created by history and used by React Router's Router.

This creates checksum errors such as:

 (client) eightMedium" href="/search/photos/sydney opera house" data-reactid="215"><span d
 (server) eightMedium" href="/search/photos/sydney%20opera%20house" data-reactid="215"><sp

Edit: this has been fixed in React Router by https://github.com/ReactTraining/react-router/pull/5722 (not released at time of writing).

[OliverJAsh]

IIUC, here's a summary of the problems caused by history's decoding of the pathname:

1. pathname (and thereby also params) is decoded using decodeURI, and browsers do not re-encode percent (%) characters, resulting in invalid URLs: see original post in this issue
2. pathname (and thereby also params) is not fully decoded (characters such as space are decoded, but characters such as & are not) because decoding uses decodeURI, which does not decode all characters, instead of decodeURIComponent, which does decode all characters. This makes it difficult (impossible?) to access the fully decoded params. See https://github.com/ReactTraining/history/issues/505#issuecomment-359550278

I would like to fix 1 by stopping all decoding of pathname. This brings the behaviour inline with the native push API and other frameworks such as [Express], which will always return the pathname provided by the user—it is never decoded.

We can then build on that to fix 2 by decoding params using decodeURIComponent instead of the current decodeURI. This brings the behaviour inline with other frameworks such as Express, which automatically decodes the values in req.params using decodeURIComponent.

I'd like to understand the implications of the suggested fix, to stop all decoding.

Quoting @pshrmn in https://github.com/ReactTraining/history/issues/505#issuecomment-318119540

the reasoning behind decoding in createLocation is that it allows us to have location object's whose pathname is decoded. That means that React Router users can use decoded strings in their route paths.

As an alternative, why can't React Router decode the pathname internally as it matches route paths?

Another effect of this is that params will be automatically decoded.

They are automatically decoded, but only partially so. See bug 2 mentioned at top of this comment. Because of this, I would consider the current behaviour as a bug.

Of course, this would lead to to inconsistencies between the locations created from window.location and locations created from values passed by the user.

@pshrmn Could you elaborate on this point to help me understand what the problem is?

[pshrmn]

// 1. start on root
{ pathname: '/' }
// 2. navigate to /역사
{ pathname: '/역사' }
// 3. navigate to /some-other-page
{ pathnamE: '/some-other-page' }
// 4. click browser's back button
{ pathname:  '/%EC%97%AD%EC%82%AC' }

Locations 2 and 4 are the same, but have different pathname values.

history cannot just encode the pathname because it doesn't want to double encode. This means that it would be the onus of the library using history to provide encoded pathnames to create consistent locations.

[OliverJAsh]

Locations 2 and 4 are the same, but have different pathname values.

Forgive me but I still don't fully follow you.

If we take the native API as an example, when navigating to /역사, window.location.pathname matches that of 4 (after navigating to some other page and then back). So I don't see the problem?

// 2. navigate to /역사
window.history.pushState(null, '', '/역사')
// => window.location.pathname === "/%EC%97%AD%EC%82%AC"

[pshrmn]

history only generates the pathname from window.location.pathname for the initial location and when popping.

[OliverJAsh]

I see.

This means that it would be the onus of the library using history to provide encoded pathnames to create consistent locations.

That sounds reasonable to me, but it sounds like you think it's unreasonable?

[pshrmn]

I like "bytes on the outside, unicode on the inside", so having location.pathname be encoded seems wrong to me. As far as what _should_ happen, I don't really know. I don't think there is a "right" answer. However, pushing the encoding responsibility to React Router and other packages that use history just spreads the pain instead of taking care of it here.

I don't actually use history/React Router (I have my own history/router), so I don't really want to advocate strongly one way or the other for something that doesn't actually affect me. My implementation uses location.rawPathname, which I think works well, but I also don't have a million+ weekly downloads, so I can afford to experiment more.

[OliverJAsh]

I think having location.pathname be encoded seems right to me because it mirrors the behaviour of the native window.location.pathname.

However, pushing the encoding responsibility to React Router and other packages that use history just spreads the pain instead of taking care of it here.

IIUC, if location.pathname was encoded, React Router would have to decode it for matching paths of Route components. Can you think of any examples when else would it need to be decoded?

If it's encoded, at least libraries have the choice of using the encoded version or decoding it to use the decoded version. If it's already decoded, they have no way of accessing the encoded version (unless we add another property like rawPathname).

If we were to add a rawPathname to represent the encoded pathname, keeping pathname decoded, which property would then be used to derive params? I'm curious how/if this would help to solve the bug whereby params are not fully decoded (see 2 in https://github.com/ReactTraining/history/issues/505#issuecomment-375646353).

[kellyrmilligan]

just to add into this, here's what i've been using as a workaround if I feel like this may affect a certain route:

export function cleanParams(params) {
  // cleans the params and returns a new params object
  const newParams = {}
  Object.keys(params).forEach(param => {
    let newParamValue = params[param]
    newParamValue = newParamValue.replace(/%26/gi, '&')
    newParamValue = newParamValue.replace(/%3F/gi, '?')
    newParamValue = newParamValue.replace(/%23/gi, '#')
    newParamValue = newParamValue.replace(/%2B/gi, '+')
    newParams[param] = newParamValue
  })
  return newParams
}

and I pass it this.props.match.params

seems to be working for my current use cases.

[kellyrmilligan]

slightly cleaner version with more characters handled:

export function cleanParams (params) {
  // cleans the params and returns a new params object
  return Object.keys(params).reduce((newParams, param) => {
    let newParamValue = params[param]
    newParamValue = newParamValue.replace(/%26/gi, '&')
    newParamValue = newParamValue.replace(/%3F/gi, '?')
    newParamValue = newParamValue.replace(/%23/gi, '#')
    newParamValue = newParamValue.replace(/%2B/gi, '+')
    newParamValue = newParamValue.replace(/%3B/gi, ';')
    newParamValue = newParamValue.replace(/%2C/gi, ',')
    newParamValue = newParamValue.replace(/%2F/gi, '/')
    newParamValue = newParamValue.replace(/%3A/gi, ':')
    newParamValue = newParamValue.replace(/%40/gi, '@')
    newParamValue = newParamValue.replace(/%3D/gi, '=')
    newParamValue = newParamValue.replace(/%24/gi, '$')
    newParams[param] = newParamValue
    return newParams
  }, {})
}

[mjackson]

I think you're right, @OliverJAsh. We should have left the pathname encoded and then only decoded it when we needed to make the match in React Router. This is the way previous versions of the router worked.

Now, the question is how to get there from here w/out breaking everyone's apps. This would be a breaking change, so we would need to push a new major version.

[wincent]

Came here to file an issue for this but I see the discussion is already quite ripe here. It seems objectively wrong to decode when creating location objects: it produces behavior which is divergent with native window.history.pushState — and with other libraries, apparently — and it is pretty easy to encounter scenarios where this leads to breakage.

For example, in one app, I have URLs with paths like /search/term. If "term" is something innocuous like "foo", then all is well. But if it is something like "\bfoo\b" (that is, a regex pattern that says to match "foo" with word boundaries on either side), then it must be encoded to "/search/%5Cfoo%5C". Once this is push-ed via the history package, it gets decoded back to "/search/\bfoo\b", and from there it eventually hits the browser's window.history.pushState at which point it gets mangled to "/search/b/foo/b" because the browser normalizes the path separators, producing a completely wrong result. If instead window.history.pushState gets the encoded path, no mangling occurs.

The workaround? Double encode to compensate for the unwanted decoding by the history package:

history.push('/search/' + encodeURIComponent(encodeURIComponent(searchTerm)), state);

Not very elegant, but it works. Still, I'd love to see the unwanted decoding removed from the history package so I could get rid of hacks like this.

[kellyrmilligan]

Is it only when using history.push that this happens? Or when using the link component as well? I guess I’m wondering if there is something that can be written on what to do now while the major version solution is being worked on.

[wincent]

@kellyrmilligan: I'd say it can definitely affect the link component if the link component ends up calling history.push under the covers (depends on where you are getting the link component from). In the example scenario I gave above, I am using a custom <Link /> component, so I had to hack it to make things work nicely with the double-encoding workaround (I want the double-encoded URL to be passed to history.push, but I want the browser to see and render a single-encoded URL in case the user inspects it); I made it take two URLs, one for display purposes and one for actually pushing to the history. Not pretty.

[devilhtc]

Why do you think it is a good idea to change the location irreversibly in the first place? Leave the display for the browser please. I would suggest using a pathnameToDecode on it instead of changing the behavior of pathname, which is the default everyone looks for.

[Prophet32j]

We are approaching a year of this issue being open. Are we at a point where a pull request can be started or are we still deciding on a solution?

[luismasg]

Hi!!!
glad to see there is a discussion going on about this.
I'm just here because I need a way to stop the router from crashing.
our QA is making a fuss about it.
can't we just do tap into the window object and clean up the route before it reaches the reactrouter?
a quick, dirty, unhealthy way of sidestepping the issue?
we don't need any decoding. just to ignore it.
www.ourhomepage.com/% is breaking the site.
It might be an issue once we actually start parsing every single url. but for now,
anyone have an idea how we can throw away everything after the '/' ?

[luismasg]

@mjackson I'm in favor of just adding a note in the react router docs. the same way their is a note about scroll restoration (which totally worked for me btw 🙏 )

"handling special characters: if ever you have an issue with special characters just do this dirty hack " - something like that.
Just a matter of deciding on the 'correct' hack 🤷‍♂️

This looks promising but I'm not sure how and when to call it

export function cleanParams (params) { // cleans the params and returns a new params object return Object.keys(params).reduce((newParams, param) => { let newParamValue = params[param] newParamValue = newParamValue.replace(/%26/gi, '&') newParamValue = newParamValue.replace(/%3F/gi, '?') newParamValue = newParamValue.replace(/%23/gi, '#') newParamValue = newParamValue.replace(/%2B/gi, '+') newParamValue = newParamValue.replace(/%3B/gi, ';') newParamValue = newParamValue.replace(/%2C/gi, ',') newParamValue = newParamValue.replace(/%2F/gi, '/') newParamValue = newParamValue.replace(/%3A/gi, ':') newParamValue = newParamValue.replace(/%40/gi, '@') newParamValue = newParamValue.replace(/%3D/gi, '=') newParamValue = newParamValue.replace(/%24/gi, '$') newParams[param] = newParamValue return newParams }, {}) }

right now i have a blank page in production and this console.log
static-www-ho.27595e071816e19.js:114 Uncaught URIError: Pathname "/%" could not be decoded. This is likely caused by an invalid percent-encoding. at t.createLocation (static-www-home.0.4.6.e67c127595e071816e19.js:114) at b (static-www-ho.27595e071816e19.js:114) at t.default (static-www-ho.27595e071816e19.js:114) at Object.<anonymous> (static-www-ho27595e071816e19.js:114) at n (static-www-ho.27595e071816e19.js:1) at Object.<anonymous> (static-www.ho0.4.6.e67c127595e071816e19.js:114) at n (static-www.0.4.6.e67c127595e071816e19.js:1) at static-www-ho.27595e071816e19.js:1 at static-www-ho.27595e071816e19.js:1

[pmwisdom]

👍 this issue needs to be looked into deeper at some point, I manually have to double encode the url parts that might have percents in them.

The place I run into a problem is when someone clicks on an anchor link it actually decodes the encoded url, which brings me back to the original offending url.

[charlesverge]

Commenting on this issue to keep it alive as it would a nice to have fix.

To summarize the the current solution/hacks is to selectively double encodeURIComponent or manually patch components in core.

The problems I see are

• Percent symbols cause javascript errors due to decoding and then reuse as a URI requiring the need of additional try / catch
• encoding is handled differently in StaticRouter used for server side rendering and BrowserRouter/Link components
• There appears to be no documentation missing for this edge case
• The issue is spread across more then one component, adding in some coordination required

[StanislavBerezin]

can somebody pin point to a solution?

[luismasg]

Hi stan. This seems to be an invalid url. I can't solve your question but I can tell you that our devops guys ended up deciding it was a non issue

Get Outlook for Androidhttps://aka.ms/ghei36

---

From: StanislavBerezin [e-mail]
Sent: Saturday, September 15, 2018 8:19:59 AM
To: ReactTraining/history
Cc: Luis Manuel Suarez; Comment
Subject: Re: [ReactTraining/history] Path is decoded in createLocation (#505)

can somebody pin point to a solution?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHubhttps://github.com/ReactTraining/history/issues/505#issuecomment-421569549, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AMWs6UIMldEkmCjsIx12x5Uvr0TElNz4ks5ubP5_gaJpZM4OjeVh.

[Prophet32j]

@luismasg
I fail to see how this is a non issue, unless you’re referring to your specific project. This issue is a bit bigger than a single project. I can tell you that my team can’t move forward with upgrading React Router because of this issue. We MUST use special characters in our routes to uniquely identify entities.

As far as this problem, is there a timeline for a major change that this bug fix can be a part of?

[cephirothdy2j]

Here's how we're getting around the issue in our project:

1. Switch your project to use Yarn if necessary.
2. In package.json, add "history": "4.5.1" to your project's "dependencies".
3. Also add "history": "4.5.1" to your "resolutions".
4. You likely will have to encodeURIComponent and decodeURIComponent as you push entries to history and read params off of the URL.

Per the library's changelog, the encoding / decoding stuff was first added in v4.6.0, and v4.5.1 is the most recent version published without these changes.

By making history a direct dependency of our project and having any libraries that rely on it resolve to our selective dependency, we get around the encoding / decoding mess. Yes, we do miss out on the benefits this functionality provides - as well as any other upgrades and fixes published to history since 4.5.1 - but we were not using any of those other features anyways.

[Andrekra]

For me it seems the decodeURI is the culprit.

I tried to visit an url with reserved characters (? # % /) so I use encodeURIComponent to build the url. /the/path%23%25%3F%2F. But the decodeURI in createLocation, changes it to /the/path/%23%%3F%2F Which cant be decoded with decodeURIComponent or used re-encode with encodeURI in the hope to get the original string back.

// User
let pathname = encodeURIComponent('%?#') // %25%3F%23

// History
pathname = decodeURI(patname) // %%3F%23

// User Trying to get the ```%?#``` back
decodeURIComponent(pathname) // Uncaught URIError: URI malformed
// Another attempt, encode it back to get the original pathname
ecodeURI(pathname) // %25%253F%2523 Huh? Thats not my original string. Double encode the %

From random googling, people said the decodeURI/encodeURI should be used on full url's, while decodeURIComponent/encodeURIComponent on parts (component) of the url. The pathname is a component. More info here for the difference between them https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURI#Description

I've wrote a few tests. If you change decodeURI to decodeURIComponent these will pass:

  describe('with a path that can be decoded', () => {
    it('handles encoded path', () => {
      expect(
        createLocation({pathname: '/the/path%23%25%3F%2F'})
      ).toMatchObject({
        pathname: '/the/path#%?/',
        search: '',
        hash: ''
      });
    });

    it('handles not encoded path', () => {
      expect(
        createLocation('/the/path?the=query#the-hash')
      ).toMatchObject({
        pathname: '/the/path',
        search: '?the=query',
        hash: '#the-hash'
      });
    });
  });

There are 3 (well 1 for 3 histories) tests failing, but I think that would be a false negative:

        Expected value to match object:
          {"pathname": "/view/%23abc"}
        Received:
          {"hash": "", "key": "rw3pt0", "pathname": "/view/#abc", "search": "", "state": undefined}

A problem would arise, when you decode %23 as an hash, and later on would re-interpertate the hash as a valid fragment (if this would be possible). Safest way would be to store the original path somewhere.

Edit:

I also discovered a second issue, related to this.
If you refresh on an URL with a safe character encoded, it's decoded (PUSH). If you use POP, the path stays as is.

const history = createBrowserHistory()
console.log('INIT:', history.location.pathname)
history.listen(location => {
  console.log(history.location.pathname)
})

// Refresh on page /the/path%23%25%3F%2F
=> INIT /the/path/%23%%3F%2F (POP, decoded, notice the double %)
// Go back and forth in in browser history
=>  /the/path/%23%25%3F%2F (PUSH, no decode)

This means I sometime have to decode twice, and sometimes once, if I use the double encode method described.

My current workaround:

const decode = str => {
  try {
    return decodeURIComponent(decodeURIComponent(str))
  } catch (e) {
    try {
      return decodeURIComponent(str)
    } catch {
      return str
    }
  }
}

const encode = str => encodeURIComponent(encodeURIComponent(str))

[Dalzhim]

Invoking decodeURI(location.pathname) on URLs before handling them to the browser's history is a major issue. First of all, it forces the browser to store malformed URLs (i.e. the original url https://mydomain.com/test%25 is modified to be https://mydomain.com/test%). Any browser vendor could disregard such blatant misuse of the API and completely break every user of this library when modifying their API to throw when pushing/replacing invalid URLs. They could also very well silently discard malformed URLs and never allow popping them back.

In this thread, workarounds were suggested as to decode everything except % signs. I would argue that it is a bad idea because you're going to surprise some of your indirect users who are going to have a hard time when their RR matcher doesn't work the same way once the % character is part of the URL (i.e. URLs are partially decoded except %25, which means you need to match against %25 rather than %).

Even though this represents a breaking change for people who came to rely on this implicit decoding, it is also a mandatory bug fix as applications relying directly or indirectly on this library are broken in regards to the % character and there is no reasonable workaround without fixing the root cause.

In conclusion, better figure out a path to make the breaking change at the time of your choosing rather than wait for a browser vendor to force you into it. In the mean time, the main Readme should list this as a known issue.

[Prophet32j]

We are ditching RR for @reach router because of this massive issue.

[OliverJAsh]

Trying to figure out where this issue now stands and how we can move forward.

I think you're right, @OliverJAsh. We should have left the pathname encoded and then only decoded it when we needed to make the match in React Router. This is the way previous versions of the router worked.

Now, the question is how to get there from here w/out breaking everyone's apps. This would be a breaking change, so we would need to push a new major version.

https://github.com/ReactTraining/history/issues/505#issuecomment-383985723

Plan:

1. Raise PR to remove decoding from history
2. Release a new version of history with this change
3. Raise PR to upgrade history in react-router and handle decoding internally
4. Release a new version of react-router with this change

I've started 1 in https://github.com/ReactTraining/history/pull/656.

[kevinkyyro]

As a matter of API design, if you want to allow users to provide decoded paths, you could accept a list instead of a string:

{
  pathname: '/search/a%2Fb%20testing/books'
}
// should be equivalent to something like
{ 
  pathname: ['search', 'a/b testing', 'books'] 
}

The relationship between each form should be approximately:

• listPathname = stringPathname.replace(/^\/|\/$/g, '').split('/').map(decodeURIComponent)
• stringPathname = '/' + listPathname.map(encodeURIComponent).join('/')

[OliverJAsh]

Update on https://github.com/ReactTraining/history/issues/505#issuecomment-453175833. We've been waiting for a few weeks now for feedback from a maintainer of this repo. No signs yet. :-(

In the meantime, I thought I'd share how we're working around this at Unsplash. We use patch-package, which lets us modify the contents of node_modules. Using this, we can remove the decoding. This has fixed all of our problems.

patch-package
--- a/node_modules/history/LocationUtils.js
+++ b/node_modules/history/LocationUtils.js
@@ -44,15 +44,9 @@ var createLocation = exports.createLocation = function createLocation(path, stat
     if (state !== undefined && location.state === undefined) location.state = state;
   }

-  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;
-    }
-  }
+  // Waiting for upstream PR to be merged
+  // https://github.com/ReactTraining/history/issues/505#issuecomment-453175833
+  // https://github.com/ReactTraining/history/pull/656

   if (key) location.key = key;

--- a/node_modules/history/es/LocationUtils.js
+++ b/node_modules/history/es/LocationUtils.js
@@ -31,16 +31,6 @@ export var createLocation = function createLocation(path, state, key, currentLoc
     if (state !== undefined && location.state === undefined) location.state = state;
   }

-  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;
-    }
-  }
-
   if (key) location.key = key;

   if (currentLocation) {
--- a/node_modules/history/umd/history.js
+++ b/node_modules/history/umd/history.js
@@ -157,16 +157,6 @@ return /******/ (function(modules) { // webpackBootstrap
        if (state !== undefined && location.state === undefined) location.state = state;
      }

-     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;
-       }
-     }
-
      if (key) location.key = key;

      if (currentLocation) {

[OliverJAsh]

There's some discussion in https://github.com/ReactTraining/history/pull/656 about what the correct behaviour is (as opposed to decoding pathname). Our options are:

1. Just use whatever pathname is provided (i.e. leave it to the browser/user to encode).
2. Try to normalize pathnames so that they are encoded. If the provided pathname is fully encoded, use as is, otherwise (if the pathname is not encoded at all, or some parts are not) encode the parts that are not yet encoded.

What do people here think? Can anyone foresee 2 causing issues?

With 2, I'm worried about bugs in the normalization process. This would be off loaded to encodeurl. Is it a completely safe operation? How likely is it to go wrong?

/cc @pshrmn

[bripkens]

Try to normalize pathnames so that they are encoded. If the provided pathname is fully encoded, use as is, otherwise (if the pathname is not encoded at all, or some parts are not) encode the parts that are not yet encoded.

IMHO this is impossible to implement. Let's say I have a path template like /article/:name with the name parameter having the value the best 10%. Since we are all fine developers we encode the value and build the full path. This results in:

$ '/article/' + encodeURIComponent('the best 10%')
'/article/the%20best%2010%25'

A generic piece of code cannot identify that this is already properly encoded. This is the whole reason why this issue here came up. encodeURI is already attempting such an implementation. For completeness:

$ encodeURI('/article/the%20best%2010%25')
/article/the%2520best%252010%2525

Just use whatever pathname is provided (i.e. leave it to the browser/user to encode).

The value is called pathname which translates in my head to path and thus to the way it is defined in the specification. IMHO it is fair to assume / require that developers properly escape when fiddling around with dynamic paths.

P.S. We are making heavy use of matrix parameters at Instana. Due to this bug we are operating a fork of history for the last 1.5 years which just removes the double encoding. It is working perfectly fine for us.

[pshrmn]

@bripkens encodeurl is designed to work with already/partially encoded strings.

With this sandbox you can see that it handles fully encoded, non-encoded, and partially encoded strings.

[Dalzhim]

@bripkens is right when he says it is impossible to implement. In order to correctly encode an URL, you must be able to prove the reverse transformation is always possible without any loss of information. Otherwise you might be causing prejudice to the application that expects to see the same URL that was given to the system originally.

Your sandbox demonstrates this impossibility. Three different source URLs are encoded to the same resulting URL. When you decode the final URL, you cannot possibly retrieve the correct source URL all of the time because you cannot guess the intent of the user. This will inevitably lead to subtle bugs that often go unnoticed.

One final example to demonstrate how encodeurl will screw up some URLs is shown in this sandbox I derived from yours. Assuming I create an article with the following title : %20 is a space %-encoded, the urlencode library sees it as being partially encoded when really, it is not encoded at all. The resulting encoded string is the following : %20%20is%20a%20space%20%25-encoded which then decodes to * is a space %-encoded, corrupting the title of my article by decoding the litteral *%20 I originally used.

[pshrmn]

@Dalzhim I'll start out with your final example; please let me know if I am misinterpreting your point.

Let's start out by completely ignoring the encodeurl package.

If we use history.pushState to navigate to this /%20 is a space %-encoded, we end up with the following pathname:

history.pushState(null, "", "/%20 is a space %-encoded")
window.location.pathname === "/%20%20is%20a%20space%20%-encoded"

The browser does not encode percentage signs, so the initial %20 octet is used as provided. We also have an invalid URI because %-e is an invalid octet.

decodeURI(window.location.pathname) // this throws an URIError

To make the URI valid, we can pass it to an encodeURI call. This will encode %-en to %25-en as well as convert %20 to %2520%20. This will result in a pathname that can be decoded to the desired result.

history.pushState(null, "", encodeURI("/%20 is a space %-encoded"))
window.location.pathname === "/%2520%20is%20a%20space%20%25-encoded"
decodeURI(window.location.pathname) // "/%20 is a space %-encoded" CORRECT

Now, getting back to encodeurl, what happens if we wrap the pathnames in encodeurl calls?

history.pushState(null, "", encodeurl("/%20 is a space %-encoded"))
window.location.pathname === "/%20%20is%20a%20space%20%25-encoded"
decodeURI(window.location.pathname) // "/ is a space %-encoded" INCORRECT

history.pushState(null, "", encodeurl(encodeURI("/%20 is a space %-encoded")))
window.location.pathname === "/%2520%20is%20a%20space%20%25-encoded"
decodeURI(window.location.pathname) // "/%20 is a space %-encoded" CORRECT

In the first case, we still have the incorrect pathname, but it is no longer an invalid URI because encodeurl correctly encodes %-en to %25-en. The issue is that encodeurl interpets the initial %20 as a valid octet.

In the second case, we get the same (correct) result as the plain encodeURI result; wrapping an encodeURI call in an encodeurl call does not have any issues with double encoding.

encodeurl offers partial protection, but is not capable of determining the users intent. The conclusion that I draw from this is that the user should always encode percentage signs themselves.

[Dalzhim]

@pshrmn : Your previous message is 100% correct. The user is the only one who has enough information to disambiguate any malformed URL. The first case ended up with a valid, but meaningless URL whereas the second example displays proper construction of the URL by the user, which makes it possible for the library to handle in a generic way. In other words, decodeURI(encodeURI(url)) === url must always be true, otherwise the error is the user's responsibility.

[OliverJAsh]

Someone who knows better than me should raise a PR to encodeurl to mention these caveats…

[kelly-tock]

Can I ask before this issue, maybe rr 3 and below, what were the isssues with how it treated path params and what not? I don’t remember tbh having an issue until 4.