vuejs / router

🚦 The official router for Vue.js
https://router.vuejs.org/
MIT License
3.75k stars 1.15k forks source link

hash decode not match the later encode #2187

Closed zcqno1 closed 2 months ago

zcqno1 commented 3 months ago

Reproduction

https://jsfiddle.net/qhn7gka8/

Steps to reproduce the bug

  1. route like /test#/?redirect=%2F%3Fid%3D1%23%2Fabc (just `/test#/?redirect=${encodeURIComponent('/?id=1#/abc')}`)
  2. do replace the upper route, the final route.fullpath turn to /test#/?redirect=/?id=1#/abc, that's unexpected
  3. push works well.

Expected behavior

the hash of route remain its original encode

Actual behavior

hash's original encode lost

Additional information

posible reason.

The hash part of route is decode in the parseURL function.

https://github.com/vuejs/router/blob/4e82939d3094569fa732937834544ec76aa7a014/packages/router/src/location.ts#L89

decode uses decodeURIComponent, which would decode the /?.

After doing a replace action, it calls parseURL first, and then calls resolve(to) later.

https://github.com/vuejs/router/blob/4e82939d3094569fa732937834544ec76aa7a014/packages/router/src/router.ts#L657

In function resolve, hash is encode. However, encodeHash use encodeURI indexof encodeURIComponent, so the hash would never be encoded to the original hash if there's characters /? in it.

https://github.com/vuejs/router/blob/4e82939d3094569fa732937834544ec76aa7a014/packages/router/src/router.ts#L531

https://github.com/vuejs/router/blob/4e82939d3094569fa732937834544ec76aa7a014/packages/router/src/encoding.ts#L72

/**
 * Encode characters that need to be encoded on the path, search and hash
 * sections of the URL.
 *
 * @internal
 * @param text - string to encode
 * @returns encoded string
 */
function commonEncode(text: string | number): string {
  return encodeURI('' + text)
    .replace(ENC_PIPE_RE, '|')
    .replace(ENC_BRACKET_OPEN_RE, '[')
    .replace(ENC_BRACKET_CLOSE_RE, ']')
}

/**
 * Encode characters that need to be encoded on the hash section of the URL.
 *
 * @param text - string to encode
 * @returns encoded string
 */
export function encodeHash(text: string): string {
  return commonEncode(text)
    .replace(ENC_CURLY_OPEN_RE, '{')
    .replace(ENC_CURLY_CLOSE_RE, '}')
    .replace(ENC_CARET_RE, '^')
}

Maybe decode(hash) in function parseURL should use decodeURI instead of decodeURIComponent.

see https://github.com/vuejs/router/issues/2060 and its pr https://github.com/vuejs/router/pull/2061

posva commented 3 months ago

The inconsistency in route.fullPath is a bit inconvenient but as long as the route.hash (and the other properties) is consistent, it's okay. In future major versions it will be possible to migrate to use the native URL constructor and get a more flexible behavior but it wasn't possible to do so when the router was initially written. Changing this now would be a breaking change as the router intentionally normalizes the hash (and other properties) to always be decoded. The fullPath, on the other hand, can vary depending on the browser and the target location, which is what you are seeing here.

So technically, this is fixable with https://github.com/vuejs/router/pull/2189 but I wouldn't rely on route.fullPath being consistent as it can vary depending of environments and other factors

posva commented 2 months ago

I had to revert this because it breaks other behaviors... This will be a wontfix until we use URL