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

Properly encode/decode the percentage character (% -> %25) across browsers and upon initial navigation #2725

Closed
posva opened this issue Apr 18, 2019 · 10 comments · Fixed by #3323
Labels
bug discussion fixed on 4.x This issue has been already fixed on the v4 but exists in v3
Projects

Comments

@posva
Copy link
Member

posva commented Apr 18, 2019

Currently, the percentage character creates multiple issues and inconsistencies. This is partly due to:

In Vue router, we have multiple inconsistencies between asterisk path, param paths and regular path regarding escaping these characters and whether the navigation is initial or triggered by a push/router-link

Given this set of routes in history mode

[
  { path: '/foo/:p', component: Foo },
  {
    path: '*',
    component: Other,
  },
]

History mode / Chrome, IE 11,10

to route match href produced in router-link $router info after clicking $router info on direct navigation remarks
/%7D * /%7D /%7D - /} /} - /} User should write /} as the link instead
/} * /%7D /} - /} N/A (browser should encode it te be /%7D) -
/%25 * /%25 /%25 - /% ERROR Navigation should be possible. Should user also write /% to be consistent?
/% * ERROR N/A (not working) N/A (not valid) Or should they escape the percent themselves because it's the only special character
/foo/%7D /foo/:p /%7D /%7D - p = } /} - p = } User should write /foo/}
/foo/} /foo/:p /%7D /} - p = } /} - p = }
/foo/%25 /foo/:p /%25 /%25 - p = % ERROR Navigation should be possible

Hash mode / TODO

Query / TODO


Complete html file to test. Requires a vue-router.js file in the same folder

index.html

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <meta http-equiv="X-UA-Compatible" content="ie=edge" />
    <title>Document</title>
  </head>

  <body>
    <script src="https://unpkg.com/vue"></script>
    <!-- <script src="https://unpkg.com/vue-router"></script> -->
    <script src="/vue-router.js"></script>

    <div id="app">
      <pre>{{ $route.path }}</pre>
      <pre>{{ $route.params }}</pre>
      <router-link to="/">/home</router-link>
      <router-link to="/%7D">/%7D</router-link>
      <router-link to="/}">/}</router-link>
      <router-link to="/%25">/%25</router-link>
      <!-- <router-link to="/%">/%</router-link> -->
      <router-link to="/foo/%7D">/foo/%7D</router-link>
      <router-link to="/foo/}">/foo/}</router-link>
      <!-- <router-link to="/foo/%">/foo/%</router-link> -->
      <router-link to="/foo/%25">/foo/%25</router-link>
      <router-view></router-view>
    </div>

    <script>
      var Home = { template: '<div>Home</div>' }
      var Foo = { template: '<div>Foo: {{ $route.params.p }}</div>' }
      var Other = {
        template: '<div>Other: {{ $route.fullPath }}</div>',
      }
      var router = new VueRouter({
        mode: 'history',
        routes: [
          { path: '/', component: Home },
          { path: '/foo/:p', component: Foo },
          {
            path: '*',
            component: Other,
          },
        ],
      })

      new Vue({
        router: router,
        el: '#app',
      })
    </script>
  </body>
</html>
@zslucky
Copy link

zslucky commented Apr 20, 2019

In most cases, we can avoid using special characters in development. But if we got decode error, I think we should catch it manually and try to find the closest asterisk match.

@posva posva added this to Long term road (high prio, low complex) in Longterm May 8, 2019
@ThomasR
Copy link

ThomasR commented Jun 11, 2019

I recommend adding the following testcases, which seem to be extra nasty:

"/ "
"/%20"
"/%2520"
"/\n"
"/%0a"
"/%250a"

Also, this should be tested with server-side-rendering.

It looks like this whole thing only works if the href-attribute always gets percent-escaped (encodeURI).

@raimund-schluessler
Copy link

Not sure, if this is the same issue, but initial navigation for routes with percent-encoded characters does not work. The problem was initially reported in nextcloud/tasks#479.

Have a look at this example:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <meta http-equiv="X-UA-Compatible" content="ie=edge" />
    <title>Document</title>
  </head>

  <body>
    <script src="https://unpkg.com/vue"></script>
    <script src="https://unpkg.com/vue-router"></script>

    <div id="app">
      <router-link
        v-for="item in items"
        :to="{name: 'items', params: {itemId: item.id }}"
        :key="item.id"
      >
        Go to {{ item.name }}
      </router-link>
      <router-view></router-view>
    </div>

    <script>
      const items = [
        {
          id: 'item1',
          name: 'Item1',
        },
        {
          id: 'item2_with_ö',
          name: 'Item2',
        },
        {
          id: 'item3_with_escaped_%C3%B6',
          name: 'Item3',
        },
      ]

      const ItemsComponent = Vue.component('Items', {
        name: 'Items',
        props: {
          itemId: {
            type: String
          }
        },
        computed: {
          item: function() {
            return items.find(item => item.id === this.itemId)
          },
        },
        template: `
          <div v-if="item">
            <span>{{ item.id }}</span>
            <span>{{ item.name }}</span>
          </div>
          <div v-else>
            <span>Item not found</span>
            <span>ItemId: {{ itemId }}</span>
          </div>
        `
      })

      const routes = [
        { name: 'items', 
          path: '/items/:itemId',
          component: ItemsComponent,
          props: true,
        },
      ]

      const router = new VueRouter({
        routes
      })

      const app = new Vue({
        router,
        data() {
          return {
            items: items,
          }
        }
      }).$mount('#app')
    </script>
  </body>
</html>

Clicking the links works fine, the correct item is loaded and the route-view shows the correct item. But if you reload the page after clicking on Go to Item 3, the item is not found, because on initial navigation the url is not encoded the same way as on router.push.

I would appreciate some feedback if this is the same issue described here or not. Thanks a lot!

@alanaasmaa
Copy link

alanaasmaa commented Aug 19, 2019

    onDecode: (req) => {
      let url = req.url
      try {
        url = decodeURI(req.url)
      }
      catch (error) {
        // Rethrow error
        if (error.message !== 'URI malformed') {
          throw error
        }
        url = decodeURI(unescape(req.url))
        // Fix url also on Nuxt side
        req.url = url
      }
      return url
    }

I had issues with ISO urls and fixed it like that.

@raimund-schluessler
Copy link

@alanaasmaa May I ask where to put / how to use onDecode?

@alanaasmaa
Copy link

alanaasmaa commented Aug 19, 2019

@alanaasmaa May I ask where to put / how to use onDecode?

@raimund-schluessler You have to use a version that is not yet released.
So in package.json > "@nuxtjs/redirect-module": "nuxt-community/redirect-module#master"

Then in nuxt config

  redirect: {
    rules: [
      { from: /^\/page(?:\/?)(?:\?(.*))?$/, to: '/?$1' },
    ],
    onDecode: (req) => {
      let url = req.url
      try {
        url = decodeURI(req.url)
      }
      catch (error) {
        // Rethrow error
        if (error.message !== 'URI malformed') {
          throw error
        }
        url = decodeURI(unescape(req.url))
        // Fix url also on Nuxt side
        req.url = url
      }
      return url
    },
    onDecodeError: (error, req, res, next) => next(error)

I also had to nuke node_modules

@David-337
Copy link

David-337 commented Aug 19, 2019

A simple example with steps to reproduce:

  1. Load the index.html app below
  2. Click on the button, vue router will happily navigate to the route which contains the '%' sign.
  3. Refresh the page
  4. The app won't load, console produces the following error:
[Vue warn]: Error in beforeCreate hook: "URIError: malformed URI sequence"

(found in <Root>) vue.js:634:17
URIError: "malformed URI sequence"
    matchRoute https://unpkg.com/vue-router/dist/vue-router.js:1591
    match https://unpkg.com/vue-router/dist/vue-router.js:1463
    match https://unpkg.com/vue-router/dist/vue-router.js:2656
    transitionTo https://unpkg.com/vue-router/dist/vue-router.js:1995
    init https://unpkg.com/vue-router/dist/vue-router.js:2701
    beforeCreate https://unpkg.com/vue-router/dist/vue-router.js:1192
    VueJS 4
    <anonymous> file:///.../index.html#/tag/this-is-a-test-tag-%:37
vue.js:1897:15
[Vue warn]: Error in render: "TypeError: route is undefined"

(found in <Root>) vue.js:634:17
TypeError: "route is undefined"
    render https://unpkg.com/vue-router/dist/vue-router.js:93
    VueJS 15
    <anonymous> file:///.../index.html#/tag/this-is-a-test-tag-%:37

index.html

<html>
  <meta charset="UTF-8">

  <head>
    <script src="https://cdn.jsdelivr.net/npm/vue/dist/vue.js"></script>
    <script src="https://unpkg.com/vue-router/dist/vue-router.js"></script>
  </head><body>
    <div id="app">

    </div>
    <script>
      let homeComponent = Vue.component('homePage', {
        template: `
          <router-link :to="{ name: 'tag', params: { tag: testTag } }" tag="button">
            Go to "{{testTag}}"
          </router-link>
        `,
        data () {
          return {
            testTag: 'this-is-a-test-tag-%'
          }
        },
      })

      let tagComponent = Vue.component('tagPage', {
        template: `
          <div>
            <p>
              this is the page for tag <b>"{{$route.params.tag}}"</b></br></br>

              refresh now to induce the bug.
            </p>
            <router-link :to="{ name: 'home' }" tag="button">Go back</router-link>
          </div>
        `
      })

      let app = new Vue({
        el: '#app',
        template: `
          <router-view></router-view>
        `,
        router: new VueRouter({
          routes: [
            {
              path: '/',
              name: 'home',
              component: homeComponent
            },
            {
              path: '/tag/:tag',
              name: 'tag',
              component: tagComponent
            }
          ]
        })
      })
    </script>
  </body>
</html>

@posva posva changed the title Properly encode/decode the percentage character (% -> %25) across browser and upon initial navigation Properly encode/decode the percentage character (% -> %25) across browsers and upon initial navigation Nov 27, 2019
isra17 added a commit to Flared/vue-router that referenced this issue Nov 27, 2019
Fix an issue where page browsing to a route and then reloading leads to
inconsistent parameters and fullPath values.
isra17 added a commit to Flared/vue-router that referenced this issue Nov 27, 2019
Fix an issue where page browsing to a route and then reloading leads to
inconsistent parameters and fullPath values.
@posva posva moved this from Long term road (high prio, low complex) to Design proposals + discussion (high prio, high complex) in Longterm Jan 15, 2020
@aradjdi
Copy link

aradjdi commented May 29, 2020

I open a PR for this issue, can you review please ? #3213

@posva posva added the fixed on 4.x This issue has been already fixed on the v4 but exists in v3 label Jun 24, 2020
@mestevezdeister
Copy link
Contributor

I've seen the issue has been fixed on 4.x. It is expected to be fixed on the current release 3.3.X? If not, is there a release date for v4.x (I could not manage to find it)?

I have some projects which I have to deliver soon, and I would apreciate the information in order to wait for the fix, or trying to remove the percentatge from the path params; which will cause me many troubles :).

Thank you very much

@posva
Copy link
Member Author

posva commented Jul 2, 2020

v4 is for Vue 3, it already exist under the next tag. Nobody is working on a fix for this right now, so contributions are welcome but there is no date

@posva posva moved this from Design proposals + discussion (high prio, high complex) to Done in Longterm Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug discussion fixed on 4.x This issue has been already fixed on the v4 but exists in v3
Projects
8 participants