fix: remove duplicated decodeURIComponent

[mestevezdeister]

As it is mentioned by @claudiocastro I proposed the change made at #3268 adding some e2e tests, in order to get the fix merged. This change is a fix for the #2725 issue.

Before this change the URL such as:

• /query/%25
• /query;q=%25

Produced an initial error:

• URIError: URI malformed at decodeURIComponent ()

And recursive errors:

• TypeError: Cannot read property 'path' of undefined

Fix change fix the issue and other test cases still pass.

Best regards

Close #2725
Close #3268
Close #3138

[posva]

Thanks for this! It still has a few breaking things that need to be fixed

[mestevezdeister]

I've added the restrictions you have proposed and solved the differences. The point was that when initially executes the application the route path was decoded (obtains the URL from the browser); whilst when changed the route from inside the application it was encoded (via clicking router-link).

This fact is what initially produces the decoding error on initially load (issue #2725), because it tried to decode an already decoded URL.

I have assumed that the URL used as 'to' param in router-link must be decoded and it worked. As it can be checked in the test, when changing the route via clicking a router-link which has defined a decoded URL the application URL is properly encoded, so I assume it is right.

Thank you for your comments.

[posva]

I have assumed that the URL used as 'to' param in router-link must be decoded and it worked. As it can be checked in the test, when changing the route via clicking a router-link which has defined a decoded URL the application URL is properly encoded, so I assume it is right.

That's the issue though, to as a string or path should be encoded 🤔 I will have to check this on older browsers first to make sure it still works

[mestevezdeister]

Ok @posva, thank you very much for you attention. Can I help in any way? or it is up to you checking the changes in old browsers?

[posva]

These are the only changes in terms of tests that need to be done. The other assertions setup in e2e tests are correct

[mestevezdeister]

After doing some research, coding and testing I found out the following:

The URL got from window.location was decoded, as it can be seen in html5.js and hash.js. But, I cannot see where the URL got from the "to" attribute of router-link is decoded, neither on push, replace, ... methods. As posva said before, the mentioned attribute should be encoded, so I presume that the internal URL was different (encoded or not) depending on the way to access the route. If the application has initially load, the internal URL was decoded, but when change the route without reloading the page, the URL was encoded.

This point caused the duplication decodeURIComponent, which is the core of the issue.

In order to fix this I have made the following changes:

1. Remove the decoding of the initial URL in html5.js and hash.js, to ensure the internal URL is the same independently the way the route is accessed.
2. Apply de decode before executing route matching, instead of decoding each parameter independently, to ensure the application found the route.

With these changes all tests works. If I can help you in any way I would be glad to do it.

[posva]

Thanks a lot, I think this is starting to look good. Sorry, it's taking some time, I must make sure it doesn't break existing code. So far, it's working well on all my tests. One thing I need to add is the application not to crash when the URL is malformed (happens on old browsers like IE9 on /oops%not-valid). This is done by doing a try catch and using the original value if it fails.

[mestevezdeister]

I'm glad he have finally managed to fix the issue successfully! One more thing, what is the protocol or schedule to publish a new release? I am looking forward to finally installing it on my project and close the issues.

Many thanks

[posva]

I still need to fix a few introduced problems and will likely release it next week

[jaapio]

I just installed dev in our project to see the results of this patch. But it doesn't seem to solve the issue for us. We do have a URL like: /records/$%25$%25$%25 I do see the window.location contains this exact value.

The error:

[Vue warn]: Error in beforeCreate hook: "URIError: malformed URI sequence"

(found in <Root>)

[Vue warn]: Error in render: "TypeError: route is undefined"

(found in <Root>)

This happens when a user refreshes the page.

[posva]

@jaapio you need to build vue-router to update the dist folder in order to use the patch

[jaapio]

Will check that

[ttshivers]

This change breaks existing behavior.

I have a route like /join/:server where server is a url encoded value like https:%2F%2Fsubdomain.domain.com.

Before v3.4.6, it decoded the server param to: https://subdomain.domain.com.
Now, in v.3.4.6, it does not: https:%2F%2Fsubdomain.domain.com.

[Dmitry-k42]

Still duplicating decodeURI in case of regex path. I found this code in function matchRoute:

  try {
    m = decodeURI(path).match(regex)
  } catch (err) {
    if (process.env.NODE_ENV !== 'production') {
      warn(false, `Error decoding "${path}". Leaving it intact.`)
    }
  }

  if (!m) {
    return false
  } else if (!params) {
    return true
  }

When second decoding fails, function return false and route can not be matched. I think code must be edited like this:

  try {
    path = decodeURI(path)
  } catch (err) {
    if (process.env.NODE_ENV !== 'production') {
      warn(false, `Error decoding "${path}". Leaving it intact.`)
    }
  }

  m = path.match(regex)

  if (!m) {
    return false
  } else if (!params) {
    return true
  }

[posva]

Thanks @Dmitry-k42