From fc51ca333062519d8376262eff93362e3a5739f9 Mon Sep 17 00:00:00 2001 From: pooya parsa Date: Sun, 6 Dec 2020 18:32:39 +0100 Subject: [PATCH 1/5] fix: normalize routes and decode resolved query (#8430) --- packages/config/package.json | 1 + packages/config/src/options.js | 3 +- packages/server/src/server.js | 12 +- packages/utils/package.json | 1 + packages/utils/src/route.js | 8 +- packages/vue-app/package.json | 1 + packages/vue-app/template/router.js | 20 ++- packages/vue-app/template/server.js | 5 +- packages/vue-app/template/utils.js | 4 +- packages/vue-renderer/src/renderer.js | 3 +- packages/webpack/src/config/base.js | 1 + test/dev/async-config.size-limit.test.js | 2 +- test/dev/dynamic-routes.test.js | 170 +++++++++---------- test/dev/encoding.test.js | 12 ++ test/dev/route-name-splitter.test.js | 35 ++-- test/fixtures/encoding/layouts/default.vue | 43 +++-- test/fixtures/encoding/pages/@about.vue | 5 + test/fixtures/encoding/pages/dynamic/_id.vue | 17 ++ test/fixtures/encoding/pages/redirect.vue | 13 ++ yarn.lock | 5 + 20 files changed, 214 insertions(+), 147 deletions(-) create mode 100644 test/fixtures/encoding/pages/@about.vue create mode 100644 test/fixtures/encoding/pages/dynamic/_id.vue create mode 100644 test/fixtures/encoding/pages/redirect.vue diff --git a/packages/config/package.json b/packages/config/package.json index 82c5117129..f701ddf870 100644 --- a/packages/config/package.json +++ b/packages/config/package.json @@ -10,6 +10,7 @@ "index.d.ts" ], "dependencies": { + "@nuxt/ufo": "^0.0.3", "@nuxt/utils": "2.14.9", "consola": "^2.15.0", "create-require": "^1.1.1", diff --git a/packages/config/src/options.js b/packages/config/src/options.js index 674e672b03..f1701fad26 100644 --- a/packages/config/src/options.js +++ b/packages/config/src/options.js @@ -7,6 +7,7 @@ import uniq from 'lodash/uniq' import consola from 'consola' import destr from 'destr' import { TARGETS, MODES, guardDir, isNonEmptyString, isPureObject, isUrl, getMainModule, urlJoin, getPKG } from '@nuxt/utils' +import { normalizeURL } from '@nuxt/ufo' import { defaultNuxtConfigFile, getDefaultNuxtConfig } from './config' export function getNuxtConfig (_options) { @@ -126,7 +127,7 @@ export function getNuxtConfig (_options) { if (!/\/$/.test(options.router.base)) { options.router.base += '/' } - options.router.base = encodeURI(decodeURI(options.router.base)) + options.router.base = normalizeURL(options.router.base) // Legacy support for export if (options.export) { diff --git a/packages/server/src/server.js b/packages/server/src/server.js index 7d49cb74ac..f93260acaf 100644 --- a/packages/server/src/server.js +++ b/packages/server/src/server.js @@ -162,12 +162,16 @@ export default class Server { })) // DX: redirect if router.base in development - if (this.options.dev && this.nuxt.options.router.base !== '/') { + const routerBase = this.nuxt.options.router.base + if (this.options.dev && routerBase !== '/') { this.useMiddleware({ prefix: false, - handler: (req, res) => { - const to = urlJoin(this.nuxt.options.router.base, req.url) - consola.info(`[Development] Redirecting from \`${decodeURI(req.url)}\` to \`${decodeURI(to)}\` (router.base specified).`) + handler: (req, res, next) => { + if (decodeURI(req.url).startsWith(decodeURI(routerBase))) { + return next() + } + const to = urlJoin(routerBase, req.url) + consola.info(`[Development] Redirecting from \`${decodeURI(req.url)}\` to \`${decodeURI(to)}\` (router.base specified)`) res.writeHead(302, { Location: to }) diff --git a/packages/utils/package.json b/packages/utils/package.json index 583768738c..8f60e06614 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -8,6 +8,7 @@ "dist" ], "dependencies": { + "@nuxt/ufo": "^0.0.3", "consola": "^2.15.0", "fs-extra": "^8.1.0", "hash-sum": "^2.0.0", diff --git a/packages/utils/src/route.js b/packages/utils/src/route.js index 40be60903e..d09eea26db 100644 --- a/packages/utils/src/route.js +++ b/packages/utils/src/route.js @@ -1,7 +1,7 @@ import path from 'path' import get from 'lodash/get' import consola from 'consola' - +import { normalizeURL } from '@nuxt/ufo' import { r } from './resolve' const routeChildren = function (route) { @@ -201,10 +201,8 @@ export const createRoutes = function createRoutes ({ } else if (key === 'index' && i + 1 === keys.length) { route.path += i > 0 ? '' : '/' } else { - const isDynamic = key.startsWith('_') - route.path += '/' + getRoutePathExtension(isDynamic ? key : encodeURIComponent(decodeURIComponent(key))) - - if (isDynamic && key.length > 1) { + route.path += normalizeURL(getRoutePathExtension(key)) + if (key.startsWith('_') && key.length > 1) { route.path += '?' } } diff --git a/packages/vue-app/package.json b/packages/vue-app/package.json index 1f6fbb6bee..12b1d01823 100644 --- a/packages/vue-app/package.json +++ b/packages/vue-app/package.json @@ -13,6 +13,7 @@ "index.d.ts" ], "dependencies": { + "@nuxt/ufo": "^0.0.3", "node-fetch": "^2.6.1", "unfetch": "^4.2.0", "vue": "^2.6.12", diff --git a/packages/vue-app/template/router.js b/packages/vue-app/template/router.js index 32d8331944..260f0e2f3c 100644 --- a/packages/vue-app/template/router.js +++ b/packages/vue-app/template/router.js @@ -1,5 +1,6 @@ import Vue from 'vue' import Router from 'vue-router' +import { normalizeURL } from '@nuxt/ufo' import { interopDefault } from './utils'<%= isTest ? '// eslint-disable-line no-unused-vars' : '' %> import scrollBehavior from './router.scrollBehavior.js' @@ -105,16 +106,27 @@ export const routerOptions = { fallback: <%= router.fallback %> } +function decodeObj(obj) { + for (const key in obj) { + if (typeof obj[key] === 'string') { + obj[key] = decodeURIComponent(obj[key]) + } + } +} + export function createRouter () { const router = new Router(routerOptions) - const resolve = router.resolve.bind(router) - // encodeURI(decodeURI()) ~> support both encoded and non-encoded urls + const resolve = router.resolve.bind(router) router.resolve = (to, current, append) => { if (typeof to === 'string') { - to = encodeURI(decodeURI(to)) + to = normalizeURL(to) } - return resolve(to, current, append) + const r = resolve(to, current, append) + if (r && r.resolved && r.resolved.query) { + decodeObj(r.resolved.query) + } + return r } return router diff --git a/packages/vue-app/template/server.js b/packages/vue-app/template/server.js index 960c25ddf6..0b4fb62b29 100644 --- a/packages/vue-app/template/server.js +++ b/packages/vue-app/template/server.js @@ -1,5 +1,6 @@ import { stringify } from 'querystring' import Vue from 'vue' +import { normalizeURL } from '@nuxt/ufo' <% if (fetch.server) { %>import fetch from 'node-fetch'<% } %> <% if (features.middleware) { %>import middleware from './middleware.js'<% } %> import { @@ -50,12 +51,12 @@ const createNext = ssrContext => (opts) => { opts.path = urlJoin(routerBase, opts.path) } // Avoid loop redirect - if (encodeURI(decodeURI(opts.path)) === ssrContext.url) { + if (decodeURI(opts.path) === decodeURI(ssrContext.url)) { ssrContext.redirected = false return } ssrContext.res.writeHead(opts.status, { - Location: opts.path + Location: normalizeURL(opts.path) }) ssrContext.res.end() } diff --git a/packages/vue-app/template/utils.js b/packages/vue-app/template/utils.js index 130e3bcc11..6435b7297f 100644 --- a/packages/vue-app/template/utils.js +++ b/packages/vue-app/template/utils.js @@ -1,4 +1,5 @@ import Vue from 'vue' +import { normalizeURL } from '@nuxt/ufo' // window.{{globals.loadedCallback}} hook // Useful for jsdom testing or plugins (https://github.com/tmpvar/jsdom#dealing-with-asynchronous-script-loading) @@ -309,7 +310,7 @@ export function getLocation (base, mode) { const fullPath = (path || '/') + window.location.search + window.location.hash - return encodeURI(fullPath) + return normalizeURL(fullPath) } // Imported from path-to-regexp @@ -692,3 +693,4 @@ export function setScrollRestoration (newVal) { window.history.scrollRestoration = newVal; } catch(e) {} } + diff --git a/packages/vue-renderer/src/renderer.js b/packages/vue-renderer/src/renderer.js index 39471f8ce3..564c21e618 100644 --- a/packages/vue-renderer/src/renderer.js +++ b/packages/vue-renderer/src/renderer.js @@ -3,6 +3,7 @@ import fs from 'fs-extra' import consola from 'consola' import template from 'lodash/template' import { TARGETS, isModernRequest, waitFor } from '@nuxt/utils' +import { normalizeURL } from '@nuxt/ufo' import SPARenderer from './renderers/spa' import SSRRenderer from './renderers/ssr' @@ -274,7 +275,7 @@ export default class VueRenderer { consola.debug(`Rendering url ${url}`) // Add url to the renderContext - renderContext.url = encodeURI(decodeURI(url)) + renderContext.url = normalizeURL(url) // Add target to the renderContext renderContext.target = this.options.target diff --git a/packages/webpack/src/config/base.js b/packages/webpack/src/config/base.js index d15047a023..8ffa9ab1ba 100644 --- a/packages/webpack/src/config/base.js +++ b/packages/webpack/src/config/base.js @@ -73,6 +73,7 @@ export default class WebpackBaseConfig { return [ /\.vue\.js/i, // include SFCs in node_modules /consola\/src/, + /@nuxt[/\\]ufo/, // exports modern syntax for browser field ...this.normalizeTranspile({ pathNormalize: true }) ] } diff --git a/test/dev/async-config.size-limit.test.js b/test/dev/async-config.size-limit.test.js index cffdb9b973..52977e1c8b 100644 --- a/test/dev/async-config.size-limit.test.js +++ b/test/dev/async-config.size-limit.test.js @@ -20,7 +20,7 @@ describe('nuxt basic resources size limit', () => { it('should stay within the size limit range in legacy mode', async () => { const legacyResourcesSize = await getResourcesSize(distDir, 'client', { gzip: true, brotli: true }) - const LEGACY_JS_RESOURCES_KB_SIZE = 200 + const LEGACY_JS_RESOURCES_KB_SIZE = 201 expect(legacyResourcesSize.uncompressed).toBeWithinSize(LEGACY_JS_RESOURCES_KB_SIZE) const LEGACY_JS_RESOURCES_GZIP_KB_SIZE = 70 diff --git a/test/dev/dynamic-routes.test.js b/test/dev/dynamic-routes.test.js index 74c96c375f..d86d540718 100644 --- a/test/dev/dynamic-routes.test.js +++ b/test/dev/dynamic-routes.test.js @@ -5,98 +5,86 @@ import { promisify } from 'util' const readFile = promisify(fs.readFile) describe('dynamic routes', () => { - test('Check .nuxt/router.js', () => { - return readFile( - resolve(__dirname, '..', 'fixtures/dynamic-routes/.nuxt/router.js'), - 'utf-8' - ).then((routerFile) => { - routerFile = routerFile - .slice(routerFile.indexOf('routes: [')) - .replace('routes: [', '[') - .replace(/ _[0-9A-Za-z]+,/g, ' "",') - routerFile = routerFile.substr( - routerFile.indexOf('['), - routerFile.lastIndexOf(']') + 1 - ) - const routes = eval('( ' + routerFile + ')') // eslint-disable-line no-eval - // pages/test/index.vue - expect(routes[0].path).toBe('/parent') - expect(routes[0].name).toBeFalsy() // parent route has no name - // pages/parent/*.vue - expect(routes[0].children.length).toBe(3) // parent has 3 children - expect(routes[0].children.map(r => r.path)).toEqual(['', 'child', 'teub']) - expect(routes[0].children.map(r => r.name)).toEqual([ - 'parent', - 'parent-child', - 'parent-teub' - ]) - // pages/posts.vue - expect(routes[1].path).toBe('/posts') - expect(routes[1].name).toBe('posts') - expect(routes[1].children.length).toBe(1) - // pages/posts/_id.vue - expect(routes[1].children[0].path).toBe(':id?') - expect(routes[1].children[0].name).toBe('posts-id') - // pages/parent.vue - expect(routes[2].path).toBe('/test') - expect(routes[2].name).toBe('test') - // pages/test/projects/index.vue - expect(routes[3].path).toBe('/test/projects') - expect(routes[3].name).toBe('test-projects') - // pages/test/users.vue - expect(routes[4].path).toBe('/test/users') - expect(routes[4].name).toBeFalsy() // parent route has no name - // pages/test/users/*.vue - expect(routes[4].children.length).toBe(5) // parent has 5 children - expect(routes[4].children.map(r => r.path)).toEqual([ - '', - 'projects', - 'projects/:category', - ':id', - ':index/teub' - ]) - expect(routes[4].children.map(r => r.name)).toEqual([ - 'test-users', - 'test-users-projects', - 'test-users-projects-category', - 'test-users-id', - 'test-users-index-teub' - ]) - // pages/test/songs/toto.vue - expect(routes[5].path).toBe('/test/songs/toto') - expect(routes[5].name).toBe('test-songs-toto') - // pages/test/projects/_category.vue - expect(routes[6].path).toBe('/test/projects/:category') - expect(routes[6].name).toBe('test-projects-category') - // pages/test/songs/_id.vue - expect(routes[7].path).toBe('/test/songs/:id?') - expect(routes[7].name).toBe('test-songs-id') - // pages/users/_id.vue - expect(routes[8].path).toBe('/users/:id?') - expect(routes[8].name).toBe('users-id') - // pages/test/_.vue - expect(routes[9].path).toBe('/test/*') - expect(routes[9].name).toBe('test-all') + test('Check .nuxt/routes.json', async () => { + const routesFile = await readFile(resolve(__dirname, '..', 'fixtures/dynamic-routes/.nuxt/routes.json'), 'utf-8') + const routes = JSON.parse(routesFile) + // pages/test/index.vue + expect(routes[0].path).toBe('/parent') + expect(routes[0].name).toBeFalsy() // parent route has no name + // pages/parent/*.vue + expect(routes[0].children.length).toBe(3) // parent has 3 children + expect(routes[0].children.map(r => r.path)).toEqual(['', 'child', 'teub']) + expect(routes[0].children.map(r => r.name)).toEqual([ + 'parent', + 'parent-child', + 'parent-teub' + ]) + // pages/posts.vue + expect(routes[1].path).toBe('/posts') + expect(routes[1].name).toBe('posts') + expect(routes[1].children.length).toBe(1) + // pages/posts/_id.vue + expect(routes[1].children[0].path).toBe(':id?') + expect(routes[1].children[0].name).toBe('posts-id') + // pages/parent.vue + expect(routes[2].path).toBe('/test') + expect(routes[2].name).toBe('test') + // pages/test/projects/index.vue + expect(routes[3].path).toBe('/test/projects') + expect(routes[3].name).toBe('test-projects') + // pages/test/users.vue + expect(routes[4].path).toBe('/test/users') + expect(routes[4].name).toBeFalsy() // parent route has no name + // pages/test/users/*.vue + expect(routes[4].children.length).toBe(5) // parent has 5 children + expect(routes[4].children.map(r => r.path)).toEqual([ + '', + 'projects', + 'projects/:category', + ':id', + ':index/teub' + ]) + expect(routes[4].children.map(r => r.name)).toEqual([ + 'test-users', + 'test-users-projects', + 'test-users-projects-category', + 'test-users-id', + 'test-users-index-teub' + ]) + // pages/test/songs/toto.vue + expect(routes[5].path).toBe('/test/songs/toto') + expect(routes[5].name).toBe('test-songs-toto') + // pages/test/projects/_category.vue + expect(routes[6].path).toBe('/test/projects/:category') + expect(routes[6].name).toBe('test-projects-category') + // pages/test/songs/_id.vue + expect(routes[7].path).toBe('/test/songs/:id?') + expect(routes[7].name).toBe('test-songs-id') + // pages/users/_id.vue + expect(routes[8].path).toBe('/users/:id?') + expect(routes[8].name).toBe('users-id') + // pages/test/_.vue + expect(routes[9].path).toBe('/test/*') + expect(routes[9].name).toBe('test-all') - // pages/index.vue - expect(routes[10].path).toBe('/') - expect(routes[10].name).toBe('index') + // pages/index.vue + expect(routes[10].path).toBe('/') + expect(routes[10].name).toBe('index') - // pages/_slug.vue - expect(routes[11].path).toBe('/:slug') - expect(routes[11].name).toBe('slug') - // pages/_key/_id.vue - expect(routes[12].path).toBe('/:key/:id?') - expect(routes[12].name).toBe('key-id') - // pages/_.vue - expect(routes[13].path).toBe('/*/p/*') - expect(routes[13].name).toBe('all-p-all') - // pages/_/_.vue - expect(routes[14].path).toBe('/*/*') - expect(routes[14].name).toBe('all-all') - // pages/_.vue - expect(routes[15].path).toBe('/*') - expect(routes[15].name).toBe('all') - }) + // pages/_slug.vue + expect(routes[11].path).toBe('/:slug') + expect(routes[11].name).toBe('slug') + // pages/_key/_id.vue + expect(routes[12].path).toBe('/:key/:id?') + expect(routes[12].name).toBe('key-id') + // pages/_.vue + expect(routes[13].path).toBe('/*/p/*') + expect(routes[13].name).toBe('all-p-all') + // pages/_/_.vue + expect(routes[14].path).toBe('/*/*') + expect(routes[14].name).toBe('all-all') + // pages/_.vue + expect(routes[15].path).toBe('/*') + expect(routes[15].name).toBe('all') }) }) diff --git a/test/dev/encoding.test.js b/test/dev/encoding.test.js index c3f5182eac..f035ea36f0 100644 --- a/test/dev/encoding.test.js +++ b/test/dev/encoding.test.js @@ -21,6 +21,18 @@ describe('encoding', () => { expect(response).toContain('Unicode base works!') }) + test('/ö/dynamic?q=food,coffee (encodeURIComponent)', async () => { + const { body: response } = await rp(url('/ö/dynamic?q=food%252Ccoffee')) + + expect(response).toContain('food,coffee') + }) + + test('/ö/@about', async () => { + const { body: response } = await rp(url('/ö/@about')) + + expect(response).toContain('About') + }) + // Close server and ask nuxt to stop listening to file changes afterAll(async () => { await nuxt.close() diff --git a/test/dev/route-name-splitter.test.js b/test/dev/route-name-splitter.test.js index 86cb29d1f9..68b74a2fdf 100644 --- a/test/dev/route-name-splitter.test.js +++ b/test/dev/route-name-splitter.test.js @@ -5,29 +5,16 @@ import { promisify } from 'util' const readFile = promisify(fs.readFile) describe('route-name-splitter', () => { - test('Check routes names', () => { - return readFile( - resolve(__dirname, '..', 'fixtures/route-name-splitter/.nuxt/router.js'), - 'utf-8' - ).then((routerFile) => { - routerFile = routerFile - .slice(routerFile.indexOf('routes: [')) - .replace('routes: [', '[') - .replace(/ _[0-9A-Za-z]+,/g, ' "",') - routerFile = routerFile.substr( - routerFile.indexOf('['), - routerFile.lastIndexOf(']') + 1 - ) - const routes = eval('( ' + routerFile + ')') // eslint-disable-line no-eval - - expect(routes[0].name).toBe('parent') - expect(routes[1].name).toBe('posts') - expect(routes[1].children[0].name).toBe('posts/id') - expect(routes[2].name).toBe('parent/child') - expect(routes[3].name).toBe('index') - expect(routes[4].name).toBe('all/p/all') - expect(routes[5].name).toBe('all/all') - expect(routes[6].name).toBe('all') - }) + test('Check routes names', async () => { + const routesFile = await readFile(resolve(__dirname, '..', 'fixtures/route-name-splitter/.nuxt/routes.json'), 'utf-8') + const routes = JSON.parse(routesFile) + expect(routes[0].name).toBe('parent') + expect(routes[1].name).toBe('posts') + expect(routes[1].children[0].name).toBe('posts/id') + expect(routes[2].name).toBe('parent/child') + expect(routes[3].name).toBe('index') + expect(routes[4].name).toBe('all/p/all') + expect(routes[5].name).toBe('all/all') + expect(routes[6].name).toBe('all') }) }) diff --git a/test/fixtures/encoding/layouts/default.vue b/test/fixtures/encoding/layouts/default.vue index 9507657a94..4ea1ac4134 100644 --- a/test/fixtures/encoding/layouts/default.vue +++ b/test/fixtures/encoding/layouts/default.vue @@ -1,24 +1,41 @@ + +