Skip to content

Commit

Permalink
fix(server, vue-app): address encoding issues with query params (#8748)
Browse files Browse the repository at this point in the history
Co-authored-by: Pooya Parsa <pyapar@gmail.com>
  • Loading branch information
danielroe and pi0 committed Feb 4, 2021
1 parent 0d49749 commit 07dd2cc
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 24 deletions.
3 changes: 2 additions & 1 deletion packages/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"pify": "^5.0.0",
"serve-placeholder": "^1.2.3",
"serve-static": "^1.14.1",
"server-destroy": "^1.0.1"
"server-destroy": "^1.0.1",
"ufo": "^0.6.1"
},
"publishConfig": {
"access": "public"
Expand Down
3 changes: 2 additions & 1 deletion packages/server/src/middleware/nuxt.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import generateETag from 'etag'
import fresh from 'fresh'
import consola from 'consola'
import { normalizeURL } from 'ufo'

import { getContext, TARGETS } from '@nuxt/utils'

Expand All @@ -9,7 +10,7 @@ export default ({ options, nuxt, renderRoute, resources }) => async function nux
const context = getContext(req, res)

try {
const url = decodeURI(req.url)
const url = normalizeURL(req.url)
res.statusCode = 200
const result = await renderRoute(url, context)

Expand Down
22 changes: 14 additions & 8 deletions packages/server/test/middleware/nuxt.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -329,18 +329,24 @@ describe('server: nuxtMiddleware', () => {
expect(consola.error).toBeCalledWith(err)
})

test('should return 400 if request is uri error', async () => {
test('should return handle uri errors by normalizing', async () => {
const context = createContext()
const result = { html: 'rendered html' }
context.renderRoute.mockReturnValue(result)
const nuxtMiddleware = createNuxtMiddleware(context)
const { req, res, next } = createServerContext()

const err = Error('URI malformed')
err.name = 'URIError'

await nuxtMiddleware({ ...req, url: 'http://localhost/test/server/%c1%81' }, res, next)

expect(next).toBeCalledWith(err)
const paths = ['%c1%81', '%c1', '%']

for (const path of paths) {
await nuxtMiddleware(
{ ...req, url: 'http://localhost/test/server/' + path },
res,
next
)

expect(next).toBeCalledTimes(0)
expect(res.statusCode).toBe(200)
next.mockReset()
}
})
})
14 changes: 1 addition & 13 deletions packages/vue-app/template/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,6 @@ export const routerOptions = {
fallback: <%= router.fallback %>
}

function decodeObj(obj) {
for (const key in obj) {
if (typeof obj[key] === 'string') {
obj[key] = decode(obj[key])
}
}
}

export function createRouter (ssrContext, config) {
const base = (config.app && config.app.basePath) || routerOptions.base
const router = new Router({ ...routerOptions, base })
Expand All @@ -124,11 +116,7 @@ export function createRouter (ssrContext, config) {
if (typeof to === 'string') {
to = normalizeURL(to)
}
const r = resolve(to, current, append)
if (r && r.resolved && r.resolved.query) {
decodeObj(r.resolved.query)
}
return r
return resolve(to, current, append)
}

return router
Expand Down
38 changes: 37 additions & 1 deletion test/dev/encoding.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import fetch from 'node-fetch'
import { getPort, loadFixture, Nuxt, rp } from '../utils'

let port
Expand All @@ -22,7 +23,7 @@ describe('encoding', () => {
})

test('/ö/dynamic?q=food,coffee (encodeURIComponent)', async () => {
const { body: response } = await rp(url('/ö/dynamic?q=food%252Ccoffee'))
const { body: response } = await rp(url('/ö/dynamic?q=food,coffee'))

expect(response).toContain('food,coffee')
})
Expand All @@ -33,6 +34,41 @@ describe('encoding', () => {
expect(response).toContain('About')
})

test('query params', async () => {
const queryStrings = {
'?email=some%20email.com': { email: 'some email.com' },
'?str=%26&str2=%2526': { str: '&', str2: '%26' },
'?t=coffee%2Cfood%2C': { t: 'coffee,food,' },
'?redirect=%2Fhomologation%2Flist': { redirect: '/homologation/list' },
'?email=some@email.com&token=DvtiwbIzry319e6KWimopA%3D%3D': {
email: 'some@email.com',
token: 'DvtiwbIzry319e6KWimopA=='
}
}
for (const [param, result] of Object.entries(queryStrings)) {
const { body: response } = await rp(url('/ö/dynamic/test') + param)
expect(response).toContain(
JSON.stringify(result)
.replace(/&/g, '&amp;')
.replace(/"/g, '&quot;')
)
}
})

test('invalidly encoded route params are handled', async () => {
const paths = ['%c1%81', '%c1', '%']
for (const path of paths) {
// We use node-fetch because got uses decodeURI on url and throws its own error
const response = await fetch(url('/ö/dynamic/') + path)
expect(response.ok).toBeTruthy()
expect(await response.text()).toContain(
JSON.stringify({ id: path })
.replace(/&/g, '&amp;')
.replace(/"/g, '&quot;')
)
}
})

// Close server and ask nuxt to stop listening to file changes
afterAll(async () => {
await nuxt.close()
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/encoding/layouts/default.vue
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export default {
'/@about',
'/тест',
encodeURI('/тест'),
'/dynamic/%c',
'/dynamic/%',
'/dynamic/سلام چطوری?q=cofee,food,دسر',
encodeURI('/dynamic/سلام چطوری?q=cofee,food,دسر'),
// Using encodeURIComponent on each segment
Expand Down

0 comments on commit 07dd2cc

Please sign in to comment.