fix: normalize routes and decode resolved query #8430
Conversation
pi0
changed the title
router.resolve
|
Note: we see this issue when a browser visits the encoded URL (via typing, for instance), without any router push. |
Codecov Report
@@ Coverage Diff @@
## 2.x #8430 +/- ##
==========================================
- Coverage 68.27% 68.23% -0.04%
==========================================
Files 91 91
Lines 3902 3904 +2
Branches 1066 1066
==========================================
Hits 2664 2664
- Misses 1004 1005 +1
- Partials 234 235 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
pi0
changed the title
router.resolve
pi0
force-pushed
the
fix/query-encoding
branch
from
657b68f
to
93e58ba
Compare
packages/utils/src/route.js
Outdated
| @@ -280,3 +279,11 @@ export const promisifyRoute = function promisifyRoute (fn, ...args) { | |||
| } | |||
| return promise | |||
| } | |||
|
|
|||
| export function safeEncodeComponent (str) { | |||
| return /%[0-9a-fA-F]{2}/.test(str) ? str : encodeURI(str) | |||
There was a problem hiding this comment.
What if url includes %.. as path or parameter ?
Like: /rate/%30?max=%90
There was a problem hiding this comment.
/%[0-9a-fA-F]{2}/.test('/rate/%30?max=%90') > true
In this case we avoid encoding
There was a problem hiding this comment.
So if url also includes special characters, we’ll skip encoding, is that expected?
Like: /rate/%30?max=%90&name= тест
There was a problem hiding this comment.
Last update: /rate/%30?max=%90&name=%20%D1%82%D0%B5%D1%81%D1%82
There was a problem hiding this comment.
Is there any merit in using something like this as our util - not reinventing url parsing?
function safeEncode (str) {
const { pathname, search } = new URL(`http://localhost${str}`)
return `${pathname}${search}`
}
safeEncode('/rate/%30?max=%90&name= тест')
// "/rate/%30?max=%90&name=%20%D1%82%D0%B5%D1%81%D1%82"There was a problem hiding this comment.
Indeed we can but it costs bundle size for rest of the browsers and actually it is not enough to make url utils (for joining or ensure there is no leading slash for example) -- not specific to this PR's case
There was a problem hiding this comment.
In fact IE11 works fine for me - just have to wrap in brackets.
There was a problem hiding this comment.
Fair enough :) Switching to URL
There was a problem hiding this comment.
There is just one downside that it is implicit it is full url or URI :))
There was a problem hiding this comment.
Thanks for bringing it up. Working on util before merge ;)
- leading slash - preserve trailing ?
pi0
changed the title
Types of changes
Description
Since
vue-router@2.14.9makes it mandatory to useencodeURIwhen usingRouterLink,NuxtLinkandrouter.push, we introduced a fix by automatically encoding URIs. But seems it makes regression when a query param (and probably path param) is encoded and not being properly decoded.Changes:
normalizeURLfrom@nuxt/uforoute.resolved.querywithdecodeURIComponentnormalizeURLsince http headers should be encodedFoot note: Case of #8429 only happens when manually using
encodeURIComponenton query parameters instead ofencodeURIand at least chrome auto fixes this. (still this is a regression that we should fixed)Checklist: