fix(router): wildcard paths when using js reserved words (like constr…

…uctor and __proto__) (#2357) * Replace `{}` by `Object.create(null)` in a few places * Add test for js reserved word * Run denoify * Add reserved words tests to common routes tests, and fix the failures on other routers too * Fix format * Run denoify again
honojs · Mar 18, 2024 · f60ab8b · f60ab8b
1 parent e675d50
commit f60ab8b
Show file tree

Hide file tree

Showing 12 changed files with 87 additions and 49 deletions.
diff --git a/deno_dist/router/linear-router/router.ts b/deno_dist/router/linear-router/router.ts
@@ -4,7 +4,7 @@ import { checkOptionalParameter } from '../../utils/url.ts'
 
 type RegExpMatchArrayWithIndices = RegExpMatchArray & { indices: [number, number][] }
 
-const emptyParams = {}
+const emptyParams = Object.create(null)
 
 const splitPathRe = /\/(:\w+(?:{(?:(?:{[\d,]+})|[^}])+})?)|\/[^\/\?]+|(\?)/g
 const splitByStarRe = /\*/
@@ -66,7 +66,7 @@ export class LinearRouter<T> implements Router<T> {
         }
         handlers.push([handler, emptyParams])
       } else if (hasLabel && !hasStar) {
-        const params: Record<string, string> = {}
+        const params: Record<string, string> = Object.create(null)
         const parts = routePath.match(splitPathRe) as string[]
 
         const lastIndex = parts.length - 1

diff --git a/deno_dist/router/pattern-router/router.ts b/deno_dist/router/pattern-router/router.ts
@@ -44,7 +44,7 @@ export class PatternRouter<T> implements Router<T> {
       if (routeMethod === METHOD_NAME_ALL || routeMethod === method) {
         const match = pattern.exec(path)
         if (match) {
-          handlers.push([handler, match.groups || {}])
+          handlers.push([handler, match.groups || Object.create(null)])
         }
       }
     }

diff --git a/deno_dist/router/reg-exp-router/node.ts b/deno_dist/router/reg-exp-router/node.ts
@@ -45,7 +45,7 @@ function compareKey(a: string, b: string): number {
 export class Node {
   index?: number
   varIndex?: number
-  children: Record<string, Node> = {}
+  children: Record<string, Node> = Object.create(null)
 
   insert(
     tokens: readonly string[],

diff --git a/deno_dist/router/reg-exp-router/router.ts b/deno_dist/router/reg-exp-router/router.ts
@@ -16,17 +16,17 @@ type HandlerWithMetadata<T> = [T, number] // [handler, paramCount]
 
 const emptyParam: string[] = []
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
-const nullMatcher: Matcher<any> = [/^$/, [], {}]
+const nullMatcher: Matcher<any> = [/^$/, [], Object.create(null)]
 
-let wildcardRegExpCache: Record<string, RegExp> = {}
+let wildcardRegExpCache: Record<string, RegExp> = Object.create(null)
 function buildWildcardRegExp(path: string): RegExp {
   return (wildcardRegExpCache[path] ??= new RegExp(
     path === '*' ? '' : `^${path.replace(/\/\*/, '(?:|/.*)')}$`
   ))
 }
 
 function clearWildcardRegExpCache() {
-  wildcardRegExpCache = {}
+  wildcardRegExpCache = Object.create(null)
 }
 
 function buildMatcherFromPreprocessedRoutes<T>(
@@ -46,11 +46,11 @@ function buildMatcherFromPreprocessedRoutes<T>(
       isStaticA ? 1 : isStaticB ? -1 : pathA.length - pathB.length
     )
 
-  const staticMap: StaticMap<T> = {}
+  const staticMap: StaticMap<T> = Object.create(null)
   for (let i = 0, j = -1, len = routesWithStaticPathFlag.length; i < len; i++) {
     const [pathErrorCheckOnly, path, handlers] = routesWithStaticPathFlag[i]
     if (pathErrorCheckOnly) {
-      staticMap[path] = [handlers.map(([h]) => [h, {}]), emptyParam]
+      staticMap[path] = [handlers.map(([h]) => [h, Object.create(null)]), emptyParam]
     } else {
       j++
     }
@@ -67,7 +67,7 @@ function buildMatcherFromPreprocessedRoutes<T>(
     }
 
     handlerData[j] = handlers.map(([h, paramCount]) => {
-      const paramIndexMap: ParamIndexMap = {}
+      const paramIndexMap: ParamIndexMap = Object.create(null)
       paramCount -= 1
       for (; paramCount >= 0; paramCount--) {
         const [key, value] = paramAssoc[paramCount]
@@ -123,8 +123,8 @@ export class RegExpRouter<T> implements Router<T> {
   routes?: Record<string, Record<string, HandlerWithMetadata<T>[]>>
 
   constructor() {
-    this.middleware = { [METHOD_NAME_ALL]: {} }
-    this.routes = { [METHOD_NAME_ALL]: {} }
+    this.middleware = { [METHOD_NAME_ALL]: Object.create(null) }
+    this.routes = { [METHOD_NAME_ALL]: Object.create(null) }
   }
 
   add(method: string, path: string, handler: T) {
@@ -136,7 +136,7 @@ export class RegExpRouter<T> implements Router<T> {
 
     if (!middleware[method]) {
       ;[middleware, routes].forEach((handlerMap) => {
-        handlerMap[method] = {}
+        handlerMap[method] = Object.create(null)
         Object.keys(handlerMap[METHOD_NAME_ALL]).forEach((p) => {
           handlerMap[method][p] = [...handlerMap[METHOD_NAME_ALL][p]]
         })
@@ -226,7 +226,7 @@ export class RegExpRouter<T> implements Router<T> {
   }
 
   private buildAllMatchers(): Record<string, Matcher<T> | null> {
-    const matchers: Record<string, Matcher<T> | null> = {}
+    const matchers: Record<string, Matcher<T> | null> = Object.create(null)
 
     ;[...Object.keys(this.routes!), ...Object.keys(this.middleware!)].forEach((method) => {
       matchers[method] ||= this.buildMatcher(method)

diff --git a/deno_dist/router/trie-router/node.ts b/deno_dist/router/trie-router/node.ts
@@ -21,14 +21,14 @@ export class Node<T> {
   patterns: Pattern[]
   order: number = 0
   name: string
-  params: Record<string, string> = {}
+  params: Record<string, string> = Object.create(null)
 
   constructor(method?: string, handler?: T, children?: Record<string, Node<T>>) {
-    this.children = children || {}
+    this.children = children || Object.create(null)
     this.methods = []
     this.name = ''
     if (method && handler) {
-      const m: Record<string, HandlerSet<T>> = {}
+      const m: Record<string, HandlerSet<T>> = Object.create(null)
       m[method] = { handler, possibleKeys: [], score: 0, name: this.name }
       this.methods = [m]
     }
@@ -75,7 +75,7 @@ export class Node<T> {
       curNode.methods = []
     }
 
-    const m: Record<string, HandlerSet<T>> = {}
+    const m: Record<string, HandlerSet<T>> = Object.create(null)
 
     const handlerSet: HandlerSet<T> = {
       handler,
@@ -101,9 +101,9 @@ export class Node<T> {
     for (let i = 0, len = node.methods.length; i < len; i++) {
       const m = node.methods[i]
       const handlerSet = (m[method] || m[METHOD_NAME_ALL]) as HandlerParamsSet<T>
-      const processedSet: Record<string, boolean> = {}
+      const processedSet: Record<string, boolean> = Object.create(null)
       if (handlerSet !== undefined) {
-        handlerSet.params = {}
+        handlerSet.params = Object.create(null)
         handlerSet.possibleKeys.forEach((key) => {
           const processed = processedSet[handlerSet.name]
           handlerSet.params[key] =
@@ -118,7 +118,7 @@ export class Node<T> {
 
   search(method: string, path: string): [[T, Params][]] {
     const handlerSets: HandlerParamsSet<T>[] = []
-    this.params = {}
+    this.params = Object.create(null)
 
     // eslint-disable-next-line @typescript-eslint/no-this-alias
     const curNode: Node<T> = this
@@ -139,9 +139,11 @@ export class Node<T> {
           if (isLast === true) {
             // '/hello/*' => match '/hello'
             if (nextNode.children['*']) {
-              handlerSets.push(...this.gHSets(nextNode.children['*'], method, node.params, {}))
+              handlerSets.push(
+                ...this.gHSets(nextNode.children['*'], method, node.params, Object.create(null))
+              )
             }
-            handlerSets.push(...this.gHSets(nextNode, method, node.params, {}))
+            handlerSets.push(...this.gHSets(nextNode, method, node.params, Object.create(null)))
           } else {
             tempNodes.push(nextNode)
           }
@@ -157,7 +159,7 @@ export class Node<T> {
           if (pattern === '*') {
             const astNode = node.children['*']
             if (astNode) {
-              handlerSets.push(...this.gHSets(astNode, method, node.params, {}))
+              handlerSets.push(...this.gHSets(astNode, method, node.params, Object.create(null)))
               tempNodes.push(astNode)
             }
             continue

diff --git a/src/router/common.case.test.ts b/src/router/common.case.test.ts
@@ -46,7 +46,7 @@ export const runTest = ({
                 params: Object.keys(r[1]).reduce((acc, key) => {
                   acc[key] = stash[(r[1] as ParamIndexMap)[key]]
                   return acc
-                }, {} as Params),
+                }, Object.create(null) as Params),
               }
             : { handler: r[0], params: r[1] as Params }
         )
@@ -78,6 +78,31 @@ export const runTest = ({
       })
     })
 
+    describe('Reserved words', () => {
+      it('Reserved words and named parameter', async () => {
+        router.add('GET', '/entry/:constructor', 'get entry')
+        const res = match('GET', '/entry/123')
+        expect(res.length).toBe(1)
+        expect(res[0].handler).toEqual('get entry')
+        expect(res[0].params['constructor']).toBe('123')
+      })
+
+      it('Reserved words and wildcard', async () => {
+        router.add('GET', '/wild/*/card', 'get wildcard')
+        const res = match('GET', '/wild/constructor/card')
+        expect(res.length).toBe(1)
+        expect(res[0].handler).toEqual('get wildcard')
+      })
+
+      it('Reserved words and optional named parameter', async () => {
+        router.add('GET', '/api/animals/:constructor?', 'animals')
+        const res = match('GET', '/api/animals')
+        expect(res.length).toBe(1)
+        expect(res[0].handler).toEqual('animals')
+        expect(res[0].params['constructor']).toBeUndefined()
+      })
+    })
+
     describe('Complex', () => {
       it('Named Param', async () => {
         router.add('GET', '/entry/:id', 'get entry')

diff --git a/src/router/linear-router/router.ts b/src/router/linear-router/router.ts
@@ -4,7 +4,7 @@ import { checkOptionalParameter } from '../../utils/url'
 
 type RegExpMatchArrayWithIndices = RegExpMatchArray & { indices: [number, number][] }
 
-const emptyParams = {}
+const emptyParams = Object.create(null)
 
 const splitPathRe = /\/(:\w+(?:{(?:(?:{[\d,]+})|[^}])+})?)|\/[^\/\?]+|(\?)/g
 const splitByStarRe = /\*/
@@ -66,7 +66,7 @@ export class LinearRouter<T> implements Router<T> {
         }
         handlers.push([handler, emptyParams])
       } else if (hasLabel && !hasStar) {
-        const params: Record<string, string> = {}
+        const params: Record<string, string> = Object.create(null)
         const parts = routePath.match(splitPathRe) as string[]
 
         const lastIndex = parts.length - 1

diff --git a/src/router/pattern-router/router.ts b/src/router/pattern-router/router.ts
@@ -44,7 +44,7 @@ export class PatternRouter<T> implements Router<T> {
       if (routeMethod === METHOD_NAME_ALL || routeMethod === method) {
         const match = pattern.exec(path)
         if (match) {
-          handlers.push([handler, match.groups || {}])
+          handlers.push([handler, match.groups || Object.create(null)])
         }
       }
     }

diff --git a/src/router/reg-exp-router/node.ts b/src/router/reg-exp-router/node.ts
@@ -45,7 +45,7 @@ function compareKey(a: string, b: string): number {
 export class Node {
   index?: number
   varIndex?: number
-  children: Record<string, Node> = {}
+  children: Record<string, Node> = Object.create(null)
 
   insert(
     tokens: readonly string[],

diff --git a/src/router/reg-exp-router/router.ts b/src/router/reg-exp-router/router.ts
@@ -16,17 +16,17 @@ type HandlerWithMetadata<T> = [T, number] // [handler, paramCount]
 
 const emptyParam: string[] = []
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
-const nullMatcher: Matcher<any> = [/^$/, [], {}]
+const nullMatcher: Matcher<any> = [/^$/, [], Object.create(null)]
 
-let wildcardRegExpCache: Record<string, RegExp> = {}
+let wildcardRegExpCache: Record<string, RegExp> = Object.create(null)
 function buildWildcardRegExp(path: string): RegExp {
   return (wildcardRegExpCache[path] ??= new RegExp(
     path === '*' ? '' : `^${path.replace(/\/\*/, '(?:|/.*)')}$`
   ))
 }
 
 function clearWildcardRegExpCache() {
-  wildcardRegExpCache = {}
+  wildcardRegExpCache = Object.create(null)
 }
 
 function buildMatcherFromPreprocessedRoutes<T>(
@@ -46,11 +46,11 @@ function buildMatcherFromPreprocessedRoutes<T>(
       isStaticA ? 1 : isStaticB ? -1 : pathA.length - pathB.length
     )
 
-  const staticMap: StaticMap<T> = {}
+  const staticMap: StaticMap<T> = Object.create(null)
   for (let i = 0, j = -1, len = routesWithStaticPathFlag.length; i < len; i++) {
     const [pathErrorCheckOnly, path, handlers] = routesWithStaticPathFlag[i]
     if (pathErrorCheckOnly) {
-      staticMap[path] = [handlers.map(([h]) => [h, {}]), emptyParam]
+      staticMap[path] = [handlers.map(([h]) => [h, Object.create(null)]), emptyParam]
     } else {
       j++
     }
@@ -67,7 +67,7 @@ function buildMatcherFromPreprocessedRoutes<T>(
     }
 
     handlerData[j] = handlers.map(([h, paramCount]) => {
-      const paramIndexMap: ParamIndexMap = {}
+      const paramIndexMap: ParamIndexMap = Object.create(null)
       paramCount -= 1
       for (; paramCount >= 0; paramCount--) {
         const [key, value] = paramAssoc[paramCount]
@@ -123,8 +123,8 @@ export class RegExpRouter<T> implements Router<T> {
   routes?: Record<string, Record<string, HandlerWithMetadata<T>[]>>
 
   constructor() {
-    this.middleware = { [METHOD_NAME_ALL]: {} }
-    this.routes = { [METHOD_NAME_ALL]: {} }
+    this.middleware = { [METHOD_NAME_ALL]: Object.create(null) }
+    this.routes = { [METHOD_NAME_ALL]: Object.create(null) }
   }
 
   add(method: string, path: string, handler: T) {
@@ -136,7 +136,7 @@ export class RegExpRouter<T> implements Router<T> {
 
     if (!middleware[method]) {
       ;[middleware, routes].forEach((handlerMap) => {
-        handlerMap[method] = {}
+        handlerMap[method] = Object.create(null)
         Object.keys(handlerMap[METHOD_NAME_ALL]).forEach((p) => {
           handlerMap[method][p] = [...handlerMap[METHOD_NAME_ALL][p]]
         })
@@ -226,7 +226,7 @@ export class RegExpRouter<T> implements Router<T> {
   }
 
   private buildAllMatchers(): Record<string, Matcher<T> | null> {
-    const matchers: Record<string, Matcher<T> | null> = {}
+    const matchers: Record<string, Matcher<T> | null> = Object.create(null)
 
     ;[...Object.keys(this.routes!), ...Object.keys(this.middleware!)].forEach((method) => {
       matchers[method] ||= this.buildMatcher(method)

diff --git a/src/router/trie-router/node.test.ts b/src/router/trie-router/node.test.ts
@@ -28,6 +28,15 @@ describe('Get with *', () => {
   })
 })
 
+describe('Get with * inclusing JS reserved words', () => {
+  const node = new Node()
+  node.insert('get', '*', 'get all')
+  it('get /', () => {
+    expect(node.search('get', '/hello/constructor')[0].length).toBe(1)
+    expect(node.search('get', '/hello/__proto__')[0].length).toBe(1)
+  })
+})
+
 describe('Basic Usage', () => {
   const node = new Node()
   node.insert('get', '/hello', 'get hello')