Skip to content

Commit

Permalink
Fix parameters decoding
Browse files Browse the repository at this point in the history
  • Loading branch information
ivan-tymoshenko committed May 16, 2022
1 parent 455e3ba commit d834596
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 83 deletions.
51 changes: 32 additions & 19 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const deepEqual = require('fast-deep-equal')
const { flattenNode, compressFlattenedNode, prettyPrintFlattenedNode, prettyPrintRoutesArray } = require('./lib/pretty-print')
const { StaticNode, NODE_TYPES } = require('./custom_node')
const Constrainer = require('./lib/constrainer')
const sanitizeUrl = require('./lib/url-sanitizer')
const { safeDecodeURI, safeDecodeURIComponent } = require('./lib/url-sanitizer')

const httpMethods = http.METHODS
const FULL_PATH_REGEXP = /^https?:\/\/.*?\//
Expand Down Expand Up @@ -85,6 +85,7 @@ function Router (opts) {
this.routes = []
this.trees = {}

this.decodeURIComponent = opts.decodeUriParameters || safeDecodeURIComponent
this.constrainer = new Constrainer(opts.constraints)
}

Expand Down Expand Up @@ -167,6 +168,12 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
continue
}

if (path.charCodeAt(i) === 37) {
// We need to encode % char to prevent double decoding
path = path.slice(0, i + 1) + '25' + path.slice(i + 1)
continue
}

const isParametricNode = path.charCodeAt(i) === 58
const isWildcardNode = path.charCodeAt(i) === 42

Expand Down Expand Up @@ -329,10 +336,8 @@ Router.prototype.find = function find (method, path, derivedConstraints) {
path = path.replace(FULL_PATH_REGEXP, '/')
}

let sanitizedUrl
try {
sanitizedUrl = sanitizeUrl(path, this.decodeUriParameters)
path = sanitizedUrl.path
path = safeDecodeURI(path)
} catch (error) {
return this._onBadUrl(path)
}
Expand All @@ -341,6 +346,8 @@ Router.prototype.find = function find (method, path, derivedConstraints) {
path = trimLastSlash(path)
}

const originPath = path

if (this.caseSensitive === false) {
path = path.toLowerCase()
}
Expand Down Expand Up @@ -429,49 +436,55 @@ Router.prototype.find = function find (method, path, derivedConstraints) {
}

if (currentNode.kind === NODE_TYPES.WILDCARD) {
const decoded = sanitizedUrl.sliceParameter(pathIndex)
if (decoded === null) {
return this._onBadUrl(path.slice(pathIndex))
let param = originPath.slice(pathIndex)

const firstPercentIndex = param.indexOf('%')
if (firstPercentIndex !== -1) {
param = this.decodeURIComponent(param, firstPercentIndex)
}

if (decoded.length > maxParamLength) {
if (param.length > maxParamLength) {
return null
}

params.push(decoded)
params.push(param)
pathIndex = pathLen
continue
}

if (currentNode.kind === NODE_TYPES.PARAMETRIC) {
let paramEndIndex = pathIndex
let firstPercentIndex = -1
for (; paramEndIndex < pathLen; paramEndIndex++) {
if (path.charCodeAt(paramEndIndex) === 47) {
const charCode = path.charCodeAt(paramEndIndex)
if (charCode === 47) {
break
} else if (firstPercentIndex === -1 && charCode === 37) {
firstPercentIndex = paramEndIndex - pathIndex
}
}

const decoded = sanitizedUrl.sliceParameter(pathIndex, paramEndIndex)
if (decoded === null) {
return this._onBadUrl(path.slice(pathIndex, paramEndIndex))
let param = originPath.slice(pathIndex, paramEndIndex)
if (firstPercentIndex !== -1) {
param = this.decodeURIComponent(param, firstPercentIndex)
}

if (currentNode.isRegex) {
const matchedParameters = currentNode.regex.exec(decoded)
const matchedParameters = currentNode.regex.exec(param)
if (matchedParameters === null) continue

for (let i = 1; i < matchedParameters.length; i++) {
const param = matchedParameters[i]
if (param.length > maxParamLength) {
const matchedParam = matchedParameters[i]
if (matchedParam.length > maxParamLength) {
return null
}
params.push(param)
params.push(matchedParam)
}
} else {
if (decoded.length > maxParamLength) {
if (param.length > maxParamLength) {
return null
}
params.push(decoded)
params.push(param)
}

pathIndex = paramEndIndex
Expand Down
128 changes: 66 additions & 62 deletions lib/url-sanitizer.js
Original file line number Diff line number Diff line change
@@ -1,83 +1,87 @@
'use strict'

const fastDecode = require('fast-decode-uri-component')

// It must spot all the chars where decodeURIComponent(x) !== decodeURI(x)
// The chars are: # $ & + , / : ; = ? @
const uriComponentsCharMap = new Array(53).fill(0x00)
uriComponentsCharMap[50] = new Array(103).fill(0x00)
uriComponentsCharMap[50][51] = true // # '%23'
uriComponentsCharMap[50][52] = true // $ '%24'
uriComponentsCharMap[50][54] = true // & '%26'
uriComponentsCharMap[50][66] = true // + '%2B'
uriComponentsCharMap[50][98] = true // + '%2b'
uriComponentsCharMap[50][67] = true // , '%2C'
uriComponentsCharMap[50][99] = true // , '%2c'
uriComponentsCharMap[50][70] = true // / '%2F'
uriComponentsCharMap[50][102] = true // / '%2f'

uriComponentsCharMap[51] = new Array(103).fill(0x00)
uriComponentsCharMap[51][65] = true // : '%3A'
uriComponentsCharMap[51][97] = true // : '%3a'
uriComponentsCharMap[51][66] = true // ; '%3B'
uriComponentsCharMap[51][98] = true // ; '%3b'
uriComponentsCharMap[51][68] = true // = '%3D'
uriComponentsCharMap[51][100] = true // = '%3d'
uriComponentsCharMap[51][70] = true // ? '%3F'
uriComponentsCharMap[51][102] = true // ? '%3f'
function decodeComponentChar (highCharCode, lowCharCode) {
if (highCharCode === 50) {
if (lowCharCode === 53) return '%'

uriComponentsCharMap[52] = new Array(49).fill(0x00)
uriComponentsCharMap[52][48] = true // @ '%40'
if (lowCharCode === 51) return '#'
if (lowCharCode === 52) return '$'
if (lowCharCode === 54) return '&'
if (lowCharCode === 66) return '+'
if (lowCharCode === 98) return '+'
if (lowCharCode === 67) return ','
if (lowCharCode === 99) return ','
if (lowCharCode === 70) return '/'
if (lowCharCode === 102) return '/'
return null
}
if (highCharCode === 51) {
if (lowCharCode === 65) return ':'
if (lowCharCode === 97) return ':'
if (lowCharCode === 66) return ';'
if (lowCharCode === 98) return ';'
if (lowCharCode === 68) return '='
if (lowCharCode === 100) return '='
if (lowCharCode === 70) return '?'
if (lowCharCode === 102) return '?'
return null
}
if (highCharCode === 52 && lowCharCode === 48) {
return '@'
}
return null
}

function sanitizeUrl (url, customDecoder) {
let originPath = url
function safeDecodeURI (path) {
let shouldDecode = false
let containsEncodedComponents = false
let highChar
let lowChar
for (var i = 0, len = url.length; i < len; i++) {
var charCode = url.charCodeAt(i)

if (shouldDecode && !containsEncodedComponents) {
if (highChar === 0 && uriComponentsCharMap[charCode]) {
highChar = charCode
lowChar = 0x00
} else if (highChar && lowChar === 0 && uriComponentsCharMap[highChar][charCode]) {
containsEncodedComponents = true
for (let i = 1; i < path.length; i++) {
const charCode = path.charCodeAt(i)

if (charCode === 37) {
const highCharCode = path.charCodeAt(i + 1)
const lowCharCode = path.charCodeAt(i + 2)

if (decodeComponentChar(highCharCode, lowCharCode) === null) {
shouldDecode = true
} else {
highChar = undefined
lowChar = undefined
// %25 - encoded % char. We need to encode one more time to prevent double decoding
if (highCharCode === 50 && lowCharCode === 53) {
shouldDecode = true
path = path.slice(0, i + 1) + '25' + path.slice(i + 1)
i += 2
}
i += 2
}
}

// Some systems do not follow RFC and separate the path and query
// string with a `;` character (code 59), e.g. `/foo;jsessionid=123456`.
// Thus, we need to split on `;` as well as `?` and `#`.
if (charCode === 63 || charCode === 59 || charCode === 35) {
originPath = url.slice(0, i)
} else if (charCode === 63 || charCode === 59 || charCode === 35) {
path = path.slice(0, i)
break
} else if (charCode === 37) {
shouldDecode = true
highChar = 0x00
}
}
const decoded = shouldDecode ? decodeURI(originPath) : originPath
return shouldDecode ? decodeURI(path) : path
}

return {
path: decoded,
originPath,
sliceParameter: containsEncodedComponents
? sliceAndDecode
: slicePath
}
function safeDecodeURIComponent (uriComponent, startIndex = 0) {
let decoded = ''
let lastIndex = startIndex

function sliceAndDecode (from, to) {
return (customDecoder || fastDecode)(this.originPath.slice(from, to))
}
}
for (let i = startIndex; i < uriComponent.length; i++) {
if (uriComponent.charCodeAt(i) === 37) {
const highCharCode = uriComponent.charCodeAt(i + 1)
const lowCharCode = uriComponent.charCodeAt(i + 2)

module.exports = sanitizeUrl
const decodedChar = decodeComponentChar(highCharCode, lowCharCode)
decoded += uriComponent.slice(lastIndex, i) + decodedChar

function slicePath (from, to) {
return this.path.slice(from, to)
lastIndex = i + 3
}
}
return uriComponent.slice(0, startIndex) + decoded + uriComponent.slice(lastIndex)
}

module.exports = { safeDecodeURI, safeDecodeURIComponent }
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
"tsd": "^0.13.1"
},
"dependencies": {
"fast-decode-uri-component": "^1.0.1",
"fast-deep-equal": "^3.1.3",
"safe-regex2": "^2.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion test/custom-decode-uri.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ test('decodeUriParameters called when needed 2', t => {
const results = [
'ci%40o',
'foo%23bar',
'%23%F'
'%23🍌'
]
const findMyWay = FindMyWay({
decodeUriParameters: (stringToDecode) => {
Expand Down
95 changes: 95 additions & 0 deletions test/issue-234.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
'use strict'

const t = require('tap')
const test = t.test
const FindMyWay = require('..')

test('Match static url without encoding option', t => {
t.plan(2)

const findMyWay = FindMyWay()

const handler = () => {}

findMyWay.on('GET', '/🍌', handler)

t.same(findMyWay.find('GET', '/🍌').handler, handler)
t.same(findMyWay.find('GET', '/%F0%9F%8D%8C').handler, handler)
})

test('Match parametric url with encoding option', t => {
t.plan(2)

const findMyWay = FindMyWay()

findMyWay.on('GET', '/🍌/:param', () => {})

t.same(findMyWay.find('GET', '/🍌/@').params, { param: '@' })
t.same(findMyWay.find('GET', '/%F0%9F%8D%8C/@').params, { param: '@' })
})

test('Match encoded parametric url with encoding option', t => {
t.plan(2)

const findMyWay = FindMyWay()

findMyWay.on('GET', '/🍌/:param', () => {})

t.same(findMyWay.find('GET', '/🍌/%23').params, { param: '#' })
t.same(findMyWay.find('GET', '/%F0%9F%8D%8C/%23').params, { param: '#' })
})

test('Decode url components', t => {
t.plan(3)

const findMyWay = FindMyWay()

findMyWay.on('GET', '/:param1/:param2', () => {})

t.same(findMyWay.find('GET', '/foo%23bar/foo%23bar').params, { param1: 'foo#bar', param2: 'foo#bar' })
t.same(findMyWay.find('GET', '/%F0%9F%8D%8C/%F0%9F%8D%8C').params, { param1: '🍌', param2: '🍌' })
t.same(findMyWay.find('GET', '/%F0%9F%8D%8C/foo%23bar').params, { param1: '🍌', param2: 'foo#bar' })
})

test('Decode url components', t => {
t.plan(5)

const findMyWay = FindMyWay()

findMyWay.on('GET', '/foo🍌bar/:param1/:param2', () => {})
findMyWay.on('GET', '/user/:id', () => {})

t.same(findMyWay.find('GET', '/foo%F0%9F%8D%8Cbar/foo%23bar/foo%23bar').params, { param1: 'foo#bar', param2: 'foo#bar' })
t.same(findMyWay.find('GET', '/user/maintainer+tomas').params, { id: 'maintainer+tomas' })
t.same(findMyWay.find('GET', '/user/maintainer%2Btomas').params, { id: 'maintainer+tomas' })
t.same(findMyWay.find('GET', '/user/maintainer%20tomas').params, { id: 'maintainer tomas' })
t.same(findMyWay.find('GET', '/user/maintainer%252Btomas').params, { id: 'maintainer%2Btomas' })
})

test('Decode url components', t => {
t.plan(18)

const findMyWay = FindMyWay()

findMyWay.on('GET', '/:param1', () => {})
t.same(findMyWay.find('GET', '/foo%23bar').params, { param1: 'foo#bar' })
t.same(findMyWay.find('GET', '/foo%24bar').params, { param1: 'foo$bar' })
t.same(findMyWay.find('GET', '/foo%26bar').params, { param1: 'foo&bar' })
t.same(findMyWay.find('GET', '/foo%2bbar').params, { param1: 'foo+bar' })
t.same(findMyWay.find('GET', '/foo%2Bbar').params, { param1: 'foo+bar' })
t.same(findMyWay.find('GET', '/foo%2cbar').params, { param1: 'foo,bar' })
t.same(findMyWay.find('GET', '/foo%2Cbar').params, { param1: 'foo,bar' })
t.same(findMyWay.find('GET', '/foo%2fbar').params, { param1: 'foo/bar' })
t.same(findMyWay.find('GET', '/foo%2Fbar').params, { param1: 'foo/bar' })

t.same(findMyWay.find('GET', '/foo%3abar').params, { param1: 'foo:bar' })
t.same(findMyWay.find('GET', '/foo%3Abar').params, { param1: 'foo:bar' })
t.same(findMyWay.find('GET', '/foo%3bbar').params, { param1: 'foo;bar' })
t.same(findMyWay.find('GET', '/foo%3Bbar').params, { param1: 'foo;bar' })
t.same(findMyWay.find('GET', '/foo%3dbar').params, { param1: 'foo=bar' })
t.same(findMyWay.find('GET', '/foo%3Dbar').params, { param1: 'foo=bar' })
t.same(findMyWay.find('GET', '/foo%3fbar').params, { param1: 'foo?bar' })
t.same(findMyWay.find('GET', '/foo%3Fbar').params, { param1: 'foo?bar' })

t.same(findMyWay.find('GET', '/foo%40bar').params, { param1: 'foo@bar' })
})

0 comments on commit d834596

Please sign in to comment.