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: normalize routes and decode resolved query #8430

Merged
merged 29 commits into from Dec 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/config/package.json
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion packages/config/src/options.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 8 additions & 4 deletions packages/server/src/server.js
Expand Up @@ -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))) {
pi0 marked this conversation as resolved.
Show resolved Hide resolved
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
})
Expand Down
1 change: 1 addition & 0 deletions packages/utils/package.json
Expand Up @@ -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",
Expand Down
8 changes: 3 additions & 5 deletions 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) {
Expand Down Expand Up @@ -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 += '?'
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/vue-app/package.json
Expand Up @@ -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",
Expand Down
20 changes: 16 additions & 4 deletions 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'

Expand Down Expand Up @@ -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)
}
const r = resolve(to, current, append)
if (r && r.resolved && r.resolved.query) {
decodeObj(r.resolved.query)
}
return resolve(to, current, append)
return r
}

return router
Expand Down
5 changes: 3 additions & 2 deletions 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 {
Expand Down Expand Up @@ -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()
}
Expand Down
4 changes: 3 additions & 1 deletion 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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -692,3 +693,4 @@ export function setScrollRestoration (newVal) {
window.history.scrollRestoration = newVal;
} catch(e) {}
}

3 changes: 2 additions & 1 deletion packages/vue-renderer/src/renderer.js
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/webpack/src/config/base.js
Expand Up @@ -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 })
]
}
Expand Down
2 changes: 1 addition & 1 deletion test/dev/async-config.size-limit.test.js
Expand Up @@ -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
Expand Down
170 changes: 79 additions & 91 deletions test/dev/dynamic-routes.test.js
Expand Up @@ -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')
})
})
12 changes: 12 additions & 0 deletions test/dev/encoding.test.js
Expand Up @@ -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()
Expand Down