Skip to content

Commit

Permalink
fix(encoding): differentiate keys and values in query
Browse files Browse the repository at this point in the history
  • Loading branch information
posva committed Oct 2, 2020
1 parent 11acb3d commit a967e42
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 34 deletions.
32 changes: 22 additions & 10 deletions __tests__/encoding.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {
encodeHash,
encodeParam,
encodeQueryProperty,
encodeQueryKey,
encodeQueryValue,
// decode,
} from '../src/encoding'

Expand Down Expand Up @@ -58,8 +59,16 @@ describe('Encoding', () => {

describe('query params', () => {
const safePerSpec = "!$'*+,:;@[]_|?/{}^()`"
const toEncode = ' "<>#&='
const encodedToEncode = toEncode
const toEncodeForKey = ' "<>#&='
const toEncodeForValue = ' "<>#&'
const encodedToEncodeForKey = toEncodeForKey
.split('')
.map(c => {
const hex = c.charCodeAt(0).toString(16).toUpperCase()
return '%' + (hex.length > 1 ? hex : '0' + hex)
})
.join('')
const encodedToEncodeForValue = toEncodeForValue
.split('')
.map(c => {
const hex = c.charCodeAt(0).toString(16).toUpperCase()
Expand All @@ -68,25 +77,28 @@ describe('Encoding', () => {
.join('')

it('does not encode safe chars', () => {
expect(encodeQueryProperty(unreservedSet)).toBe(unreservedSet)
expect(encodeQueryValue(unreservedSet)).toBe(unreservedSet)
expect(encodeQueryKey(unreservedSet)).toBe(unreservedSet)
})

it('encodes non-ascii', () => {
expect(encodeQueryProperty('é')).toBe('%C3%A9')
expect(encodeQueryValue('é')).toBe('%C3%A9')
expect(encodeQueryKey('é')).toBe('%C3%A9')
})

it('encodes non-printable ascii', () => {
expect(encodeQueryProperty(nonPrintableASCII)).toBe(
encodedNonPrintableASCII
)
expect(encodeQueryValue(nonPrintableASCII)).toBe(encodedNonPrintableASCII)
expect(encodeQueryKey(nonPrintableASCII)).toBe(encodedNonPrintableASCII)
})

it('does not encode a safe set', () => {
expect(encodeQueryProperty(safePerSpec)).toBe(safePerSpec)
expect(encodeQueryValue(safePerSpec)).toBe(safePerSpec)
expect(encodeQueryKey(safePerSpec)).toBe(safePerSpec)
})

it('encodes a specific charset', () => {
expect(encodeQueryProperty(toEncode)).toBe(encodedToEncode)
expect(encodeQueryKey(toEncodeForKey)).toBe(encodedToEncodeForKey)
expect(encodeQueryValue(toEncodeForValue)).toBe(encodedToEncodeForValue)
})
})

Expand Down
8 changes: 7 additions & 1 deletion __tests__/parseQuery.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ describe('parseQuery', () => {
})
})

it('decodes empty values as null', () => {
it('allows = inside values', () => {
expect(parseQuery('e=c=a')).toEqual({
e: 'c=a',
})
})

it('parses empty values as null', () => {
expect(parseQuery('e&b&c=a')).toEqual({
e: null,
b: null,
Expand Down
8 changes: 8 additions & 0 deletions __tests__/stringifyQuery.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,12 @@ describe('stringifyQuery', () => {
it('encodes values in arrays', () => {
expect(stringifyQuery({ e: ['%', 'a'], b: 'c' })).toEqual('e=%25&e=a&b=c')
})

it('encodes = in key', () => {
expect(stringifyQuery({ '=': 'a' })).toEqual('%3D=a')
})

it('keeps = in value', () => {
expect(stringifyQuery({ a: '=' })).toEqual('a==')
})
})
22 changes: 13 additions & 9 deletions __tests__/urlEncoding.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,10 @@ describe('URL Encoding', () => {
it('calls encodeQueryProperty with query', async () => {
const router = createRouter()
await router.push({ name: 'home', query: { p: 'foo' } })
expect(encoding.encodeQueryProperty).toHaveBeenCalledTimes(2)
expect(encoding.encodeQueryProperty).toHaveBeenNthCalledWith(1, 'p')
expect(encoding.encodeQueryProperty).toHaveBeenNthCalledWith(2, 'foo')
expect(encoding.encodeQueryValue).toHaveBeenCalledTimes(1)
expect(encoding.encodeQueryKey).toHaveBeenCalledTimes(1)
expect(encoding.encodeQueryKey).toHaveBeenNthCalledWith(1, 'p')
expect(encoding.encodeQueryValue).toHaveBeenNthCalledWith(1, 'foo')
})

it('calls decode with query', async () => {
Expand All @@ -113,21 +114,24 @@ describe('URL Encoding', () => {
it('calls encodeQueryProperty with arrays in query', async () => {
const router = createRouter()
await router.push({ name: 'home', query: { p: ['foo', 'bar'] } })
expect(encoding.encodeQueryProperty).toHaveBeenCalledTimes(3)
expect(encoding.encodeQueryProperty).toHaveBeenNthCalledWith(1, 'p')
expect(encoding.encodeQueryProperty).toHaveBeenNthCalledWith(2, 'foo')
expect(encoding.encodeQueryProperty).toHaveBeenNthCalledWith(3, 'bar')
expect(encoding.encodeQueryValue).toHaveBeenCalledTimes(2)
expect(encoding.encodeQueryKey).toHaveBeenCalledTimes(1)
expect(encoding.encodeQueryKey).toHaveBeenNthCalledWith(1, 'p')
expect(encoding.encodeQueryValue).toHaveBeenNthCalledWith(1, 'foo')
expect(encoding.encodeQueryValue).toHaveBeenNthCalledWith(2, 'bar')
})

it('keeps decoded values in query', async () => {
// @ts-ignore: override to make the difference
encoding.decode = () => 'd'
// @ts-ignore
encoding.encodeQueryProperty = () => 'e'
encoding.encodeQueryValue = () => 'ev'
// @ts-ignore
encoding.encodeQueryKey = () => 'ek'
const router = createRouter()
await router.push({ name: 'home', query: { p: '%' } })
expect(router.currentRoute.value).toMatchObject({
fullPath: '/?e=e',
fullPath: '/?ek=ev',
query: { p: '%' },
})
})
Expand Down
14 changes: 11 additions & 3 deletions src/encoding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,31 @@ export function encodeHash(text: string): string {
}

/**
* Encode characters that need to be encoded query keys and values on the query
* Encode characters that need to be encoded query values on the query
* section of the URL.
*
* @param text - string to encode
* @returns encoded string
*/
export function encodeQueryProperty(text: string | number): string {
export function encodeQueryValue(text: string | number): string {
return commonEncode(text)
.replace(HASH_RE, '%23')
.replace(AMPERSAND_RE, '%26')
.replace(EQUAL_RE, '%3D')
.replace(ENC_BACKTICK_RE, '`')
.replace(ENC_CURLY_OPEN_RE, '{')
.replace(ENC_CURLY_CLOSE_RE, '}')
.replace(ENC_CARET_RE, '^')
}

/**
* Like `encodeQueryValue` but also encodes the `=` character.
*
* @param text - string to encode
*/
export function encodeQueryKey(text: string | number): string {
return encodeQueryValue(text).replace(EQUAL_RE, '%3D')
}

/**
* Encode characters that need to be encoded on the path section of the URL.
*
Expand Down
21 changes: 10 additions & 11 deletions src/query.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { decode, encodeQueryProperty } from './encoding'
import { decode, encodeQueryKey, encodeQueryValue } from './encoding'

/**
* Possible values in normalized {@link LocationQuery}
Expand Down Expand Up @@ -50,13 +50,12 @@ export function parseQuery(search: string): LocationQuery {
const hasLeadingIM = search[0] === '?'
const searchParams = (hasLeadingIM ? search.slice(1) : search).split('&')
for (let i = 0; i < searchParams.length; ++i) {
let [key, rawValue] = searchParams[i].split('=') as [
string,
string | undefined
]
key = decode(key)
// avoid decoding null
let value = rawValue == null ? null : decode(rawValue)
const searchParam = searchParams[i]
// allow the = character
let eqPos = searchParam.indexOf('=')
let key = decode(eqPos < 0 ? searchParam : searchParam.slice(0, eqPos))
let value = eqPos < 0 ? null : decode(searchParam.slice(eqPos + 1))

if (key in query) {
// an extra variable for ts types
let currentValue = query[key]
Expand Down Expand Up @@ -85,16 +84,16 @@ export function stringifyQuery(query: LocationQueryRaw): string {
for (let key in query) {
if (search.length) search += '&'
const value = query[key]
key = encodeQueryProperty(key)
key = encodeQueryKey(key)
if (value == null) {
// only null adds the value
if (value !== undefined) search += key
continue
}
// keep null values
let values: LocationQueryValueRaw[] = Array.isArray(value)
? value.map(v => v && encodeQueryProperty(v))
: [value && encodeQueryProperty(value)]
? value.map(v => v && encodeQueryValue(v))
: [value && encodeQueryValue(value)]

for (let i = 0; i < values.length; i++) {
// only append & with i > 0
Expand Down

0 comments on commit a967e42

Please sign in to comment.