fix: url handle #247
fix: url handle #247
Conversation
| try { | ||
| const parsed = new URL(url, 'http://localhost.com/') | ||
| return parsed.pathname + (parsed.pathname[parsed.pathname.length - 1] !== '/' ? '/' : '') + (parsed.search || '') | ||
| } catch (error) { | ||
| // the try-catch here is actually unreachable, but we keep it for safety and prevent DoS attack |
There was a problem hiding this comment.
In here, the catch branch is actually unreachable after we convert all leading slash to a single /. But, it may be changed in future and URL will be crashed on some weird case.
So, I will keep the try {} catch {} here and ignore the coverage.
Why it is unreachable?
When we specify the host for in URL, the path can be either a full url or path. After we stripe the leading slash and it can only be path or schema://something in this case. And URL do not check the invalid encoded component. Thus, it is not able to crash anymore.
There was a problem hiding this comment.
This code was reachable for this command:
curl --path-as-is "http://localhost:3000//^/.."
There was a problem hiding this comment.
http://localhost:3000//^/.. is actually doing new URL('/^/..', 'http://localhost.com/') and it is not throwing.
There was a problem hiding this comment.
on node 16.8 I get (see the double slash - this trigger the issue but your check hide this case)
➜ fastify-static git:(PULL_247) ✗ node
Welcome to Node.js v16.8.0.
Type ".help" for more information.
> new URL('//^/..', 'http://localhost.com/')
Uncaught TypeError [ERR_INVALID_URL]: Invalid URL
at __node_internal_captureLargerStackTrace (node:internal/errors:464:5)
at new NodeError (node:internal/errors:371:5)
at onParseError (node:internal/url:552:9)
at new URL (node:internal/url:628:5) {
input: '//^/..',
code: 'ERR_INVALID_URL'
}
It is a quick fix to #246
Fixes #246 Closes #245
I am think of how to reach the coverage to 100%. Some helps is appreciate.
Checklist
npm run testandnpm run benchmarkand the Code of conduct