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

fix(encoding): decode params #3350

Merged
merged 2 commits into from Nov 5, 2020
Merged

fix(encoding): decode params #3350

merged 2 commits into from Nov 5, 2020

Conversation

posva
Copy link
Member

@posva posva commented Oct 20, 2020

What is breaking

Users no longer can use unencoded static paths:

  • A route defined with { path: '/cassé' }
  • <router-link to="/cassé" />
  • <router-link :to="{ path '/cassé' }" />
  • router.push('/cassé')
  • router.push({ path: '/cassé' })

They have to manually encode them with encodeURI() or use a named route:

new Router({
  routes: [
    { path: '/cassé' },
    // becomes
    { path: encodeURI('/cassé') },
    // be careful to not encode characters used in a regex ([ and ] are encoded by encodeURI)
	{ path: '/色/:hex([0-9a-f]+)' },
    // becomes
    { path: encodeURI('/色') + '/:hex([0-9a-f]+)' },
	// not necessary if all characters are latin
    { path: '/articles/:articleTitle' }
 ]
})
router.push('/cassé')
// becomes
router.push(encodeURI('/cassé'))
<router-link to="/cassé" />
becomes
<router-link :to="encodeURI('/cassé')" />
or with a name
<router-link :to="{ name: 'cassé' }" />

This is only necessary with string locations. When using params and query, the behavior is still the same and using named routes with params is recommended for that reason:

// given a route /servers/:server
// going to /servers/one%2Ftwo
// should give
this.$route.params.server // one/two

In fact, this PR makes this possible, it wasn't working before.

Similarly, doing router.push({ name: 'servers', params: { server: 'one/two' }}) will change the url to /servers/one%2Ftwo while giving the correct value in $route.params.server.

Why this can't be done automatically by the router

We could automatically call encodeURI() on each static segment of a path but this would also break code for users who are already correctly encoding path (you should always encode URLs, even before this change).

How to help the migration

Since we were never supposed to pass an unencoded path to a route record or even call router.push() with an unencoded string or path property, I propose to warn in dev mode if any unencoded character is passed to help people be aware of how they should encode their routes.


Internal explanation

Previously, this used to work because we were decoding paths twice, but this created other problems like errors with URLs containing the character % encoded. By fixing this bug we must remove the decode call made on the path when matching against existing routes, but this also makes it impossible to automatically encode path anymore because the user wouldn't be able to provide encoded characters, e.g. the / character (%2F) would be transformed to its encoded version, %252F (% is encoded as %25) and there wouldn't be a way to match an encoded slash character which is necessary in segments of the URL since the slash character is used to differentiate segments in a URL. Without this change, we can't correctly decode %2F

Overall, this makes things consistent and it's also the same behavior that exists in v4

Close #3337
Close #3103
Close #3125

This change forces users to encode their `path` in routes but also fixes
existing problems with route location that were provided as string and
not encoded. Specially with the slash character, allowing it to be
encoded and decoded properly.
@posva
Copy link
Member Author

posva commented Nov 4, 2020

Looking forward to merge this at the end of this week

@posva posva merged commit 63c749c into dev Nov 5, 2020
@posva posva deleted the fix/3337/slash-encoding branch November 5, 2020 15:14
@pi0 pi0 mentioned this pull request Nov 9, 2020
7 tasks
ttshivers added a commit to ttshivers/synclounge-1 that referenced this pull request Nov 10, 2020
The parameter encoding is now fixed as of:
vuejs/vue-router#3350
ttshivers added a commit to ttshivers/synclounge-1 that referenced this pull request Nov 10, 2020
The parameter encoding is now fixed as of:
vuejs/vue-router#3350
ttshivers added a commit to ttshivers/synclounge-1 that referenced this pull request Dec 19, 2020
The parameter encoding is now fixed as of:
vuejs/vue-router#3350
ttshivers added a commit to ttshivers/synclounge-1 that referenced this pull request Jan 17, 2021
The parameter encoding is now fixed as of: vuejs/vue-router#3350
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant