fix(req): correct `c.req.param` decodes invalid percent strings
Fixes #3550
In the current behavior, the c.req.param() will throw the error when decoding invalid percent encoding or invalid UTF-8 sequence.
// app
app.get('/static/:path', (c) => c.text(c.req.param('path'))
// throw `URIError: URI malformed` error:
await app.request('/static/%A4%A2') // %A4%A2 is invalid UTF-8 sequence
// throw `URIError: URI malformed` error:
await app.request('/static/%a') // %a is invalid percent encoding
These behaviors are expected in the current version implementation; they are tested:
https://github.com/honojs/hono/compare/fix/request-dont-throw-error-with-percent-strings?expand=1#diff-8ac13809c9886e994d1db33943de82df4d6c5d88b73fd0270c0189804ff565c2L772-L814
However, according to #3550, throwing the error will stop the application procedure, and it can accept malicious requests. I think it should not throw the error. This should be fixed, though we should rewrite the test code.
The correct behavior
How to handle the invalid path parameter strings. One way is throwing 400 Bad Request, but as I commented, it should follow the decode method of the getPath() to resolve the URL path:
https://github.com/honojs/hono/blob/0a99bd3e74cc444d4b968dab775c99674b89835f/src/utils/url.ts#L112
So, the result will be like the following:
res = await app.request('/static/%A4%A2')
console.log(await res.text()) // %A4%A2
res = await app.request('/static/%a')
console.log(await res.text()) // %a
In this PR, I used the same logic of tryDecodeURI for tryDecodeURIComponent.
The author should do the following, if applicable
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 94.71%. Comparing base (
3a59d86) to head (546a188). Report is 9 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3573 +/- ##
==========================================
- Coverage 94.71% 94.71% -0.01%
==========================================
Files 157 158 +1
Lines 9539 9537 -2
Branches 2774 2794 +20
==========================================
- Hits 9035 9033 -2
Misses 504 504
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi @usualoma !
Can you review this?
@usualoma Thanks! Agree all. Can you review this again?
@usualoma
Thanks! Merging now.