From 2bde76d3b5725283d2a18ee8fd5300f0925436a6 Mon Sep 17 00:00:00 2001 From: Yusuke Wada Date: Wed, 30 Oct 2024 10:59:05 +0900 Subject: [PATCH] fix(req): correct `c.req.param` decodes invalid percent strings (#3573) * fix(req): correct `c.req.param` decodes invalid percent strings * make `tryDecodeURIComponent` as a `const` function * use `/\%/.test()` for a better performance --- src/hono.test.ts | 32 ++++---------------------------- src/request.ts | 9 +++++---- src/utils/url.ts | 29 ++++++++++++++++------------- 3 files changed, 25 insertions(+), 45 deletions(-) diff --git a/src/hono.test.ts b/src/hono.test.ts index b6fdd73c..2e8a5ff8 100644 --- a/src/hono.test.ts +++ b/src/hono.test.ts @@ -771,46 +771,22 @@ describe('Routing', () => { it('should decode alphabets with invalid UTF-8 sequence', async () => { app.get('/static/:path', (c) => { - try { - return c.text(`by c.req.param: ${c.req.param('path')}`) // this should throw an error - } catch (e) { - return c.text(`by c.req.url: ${c.req.url.replace(/.*\//, '')}`) - } + return c.text(`by c.req.param: ${c.req.param('path')}`) }) const res = await app.request('http://localhost/%73tatic/%A4%A2') // %73 is 's', %A4%A2 is invalid UTF-8 sequence expect(res.status).toBe(200) - expect(await res.text()).toBe('by c.req.url: %A4%A2') + expect(await res.text()).toBe('by c.req.param: %A4%A2') }) it('should decode alphabets with invalid percent encoding', async () => { app.get('/static/:path', (c) => { - try { - return c.text(`by c.req.param: ${c.req.param('path')}`) // this should throw an error - } catch (e) { - return c.text(`by c.req.url: ${c.req.url.replace(/.*\//, '')}`) - } + return c.text(`by c.req.param: ${c.req.param('path')}`) }) const res = await app.request('http://localhost/%73tatic/%a') // %73 is 's', %a is invalid percent encoding expect(res.status).toBe(200) - expect(await res.text()).toBe('by c.req.url: %a') - }) - - it('should be able to catch URIError', async () => { - app.onError((err, c) => { - if (err instanceof URIError) { - return c.text(err.message, 400) - } - throw err - }) - app.get('/static/:path', (c) => { - return c.text(`by c.req.param: ${c.req.param('path')}`) // this should throw an error - }) - - const res = await app.request('http://localhost/%73tatic/%a') // %73 is 's', %a is invalid percent encoding - expect(res.status).toBe(400) - expect(await res.text()).toBe('URI malformed') + expect(await res.text()).toBe('by c.req.param: %a') }) it('should not double decode', async () => { diff --git a/src/request.ts b/src/request.ts index 63ab2a37..a3f68f9d 100644 --- a/src/request.ts +++ b/src/request.ts @@ -13,7 +13,7 @@ import { parseBody } from './utils/body' import type { BodyData, ParseBodyOptions } from './utils/body' import type { CustomHeader, RequestHeader } from './utils/headers' import type { Simplify, UnionToIntersection } from './utils/types' -import { decodeURIComponent_, getQueryParam, getQueryParams } from './utils/url' +import { decodeURIComponent_, getQueryParam, getQueryParams, tryDecode } from './utils/url' type Body = { json: any @@ -24,6 +24,8 @@ type Body = { } type BodyCache = Partial +const tryDecodeURIComponent = (str: string) => tryDecode(str, decodeURIComponent_) + export class HonoRequest

{ /** * `.raw` can get the raw Request object. @@ -95,8 +97,7 @@ export class HonoRequest

{ private getDecodedParam(key: string): string | undefined { const paramKey = this.#matchResult[0][this.routeIndex][1][key] const param = this.getParamValue(paramKey) - - return param ? (/\%/.test(param) ? decodeURIComponent_(param) : param) : undefined + return param ? (/\%/.test(param) ? tryDecodeURIComponent(param) : param) : undefined } private getAllDecodedParams(): Record { @@ -106,7 +107,7 @@ export class HonoRequest

{ for (const key of keys) { const value = this.getParamValue(this.#matchResult[0][this.routeIndex][1][key]) if (value && typeof value === 'string') { - decoded[key] = /\%/.test(value) ? decodeURIComponent_(value) : value + decoded[key] = /\%/.test(value) ? tryDecodeURIComponent(value) : value } } diff --git a/src/utils/url.ts b/src/utils/url.ts index 47db1c69..92252fa3 100644 --- a/src/utils/url.ts +++ b/src/utils/url.ts @@ -74,6 +74,21 @@ export const getPattern = (label: string): Pattern | null => { return null } +type Decoder = (str: string) => string +export const tryDecode = (str: string, decoder: Decoder): string => { + try { + return decoder(str) + } catch { + return str.replace(/(?:%[0-9A-Fa-f]{2})+/g, (match) => { + try { + return decoder(match) + } catch { + return match + } + }) + } +} + /** * Try to apply decodeURI() to given string. * If it fails, skip invalid percent encoding or invalid UTF-8 sequences, and apply decodeURI() to the rest as much as possible. @@ -83,19 +98,7 @@ export const getPattern = (label: string): Pattern | null => { * tryDecodeURI('Hello%20World') // 'Hello World' * tryDecodeURI('Hello%20World/%A4%A2') // 'Hello World/%A4%A2' */ -const tryDecodeURI = (str: string): string => { - try { - return decodeURI(str) - } catch { - return str.replace(/(?:%[0-9A-Fa-f]{2})+/g, (match) => { - try { - return decodeURI(match) - } catch { - return match - } - }) - } -} +const tryDecodeURI = (str: string) => tryDecode(str, decodeURI) export const getPath = (request: Request): string => { const url = request.url