diff --git a/runtime-tests/bun/index.test.tsx b/runtime-tests/bun/index.test.tsx index 17ee3f487a..b40b2ff10d 100644 --- a/runtime-tests/bun/index.test.tsx +++ b/runtime-tests/bun/index.test.tsx @@ -124,7 +124,9 @@ describe('Serve Static Middleware', () => { expect(res.status).toBe(404) expect(res.headers.get('X-Custom')).toBe('Bun') expect(onNotFound).toHaveBeenCalledWith( - './runtime-tests/bun/favicon-notfound.ico', + process.platform === 'win32' + ? 'runtime-tests\\bun\\favicon-notfound.ico' + : 'runtime-tests/bun/favicon-notfound.ico', expect.anything() ) }) diff --git a/src/adapter/bun/serve-static.ts b/src/adapter/bun/serve-static.ts index c5be32e6cb..05e9569386 100644 --- a/src/adapter/bun/serve-static.ts +++ b/src/adapter/bun/serve-static.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ import { stat } from 'node:fs/promises' +import { join } from 'node:path' import { serveStatic as baseServeStatic } from '../../middleware/serve-static' import type { ServeStaticOptions } from '../../middleware/serve-static' import type { Env, MiddlewareHandler } from '../../types' @@ -9,14 +10,10 @@ export const serveStatic = ( ): MiddlewareHandler => { return async function serveStatic(c, next) { const getContent = async (path: string) => { - path = path.startsWith('/') ? path : `./${path}` // @ts-ignore const file = Bun.file(path) return (await file.exists()) ? file : null } - const pathResolve = (path: string) => { - return path.startsWith('/') ? path : `./${path}` - } const isDir = async (path: string) => { let isDir try { @@ -28,7 +25,7 @@ export const serveStatic = ( return baseServeStatic({ ...options, getContent, - pathResolve, + join, isDir, })(c, next) } diff --git a/src/adapter/cloudflare-workers/serve-static.test.ts b/src/adapter/cloudflare-workers/serve-static.test.ts index 2d4a52fcce..e0191ff6c5 100644 --- a/src/adapter/cloudflare-workers/serve-static.test.ts +++ b/src/adapter/cloudflare-workers/serve-static.test.ts @@ -79,7 +79,8 @@ describe('ServeStatic Middleware', () => { expect(res.headers.get('Content-Type')).toBe('text/plain; charset=utf-8') }) - it('Should return index.html', async () => { + // Serve static on Cloudflare Workers cannot determine whether the target path is a directory or not + it.skip('Should return index.html', async () => { const res = await app.request('http://localhost/static/top') expect(res.status).toBe(200) expect(await res.text()).toBe('

Top

') diff --git a/src/adapter/deno/serve-static.ts b/src/adapter/deno/serve-static.ts index 867f6f9ea8..866ee7035a 100644 --- a/src/adapter/deno/serve-static.ts +++ b/src/adapter/deno/serve-static.ts @@ -1,3 +1,4 @@ +import { join } from 'node:path' import type { ServeStaticOptions } from '../../middleware/serve-static' import { serveStatic as baseServeStatic } from '../../middleware/serve-static' import type { Env, MiddlewareHandler } from '../../types' @@ -23,9 +24,6 @@ export const serveStatic = ( return null } } - const pathResolve = (path: string) => { - return path.startsWith('/') ? path : `./${path}` - } const isDir = (path: string) => { let isDir try { @@ -34,11 +32,10 @@ export const serveStatic = ( } catch {} return isDir } - return baseServeStatic({ ...options, getContent, - pathResolve, + join, isDir, })(c, next) } diff --git a/src/middleware/serve-static/index.test.ts b/src/middleware/serve-static/index.test.ts index 9681404673..d822eca35c 100644 --- a/src/middleware/serve-static/index.test.ts +++ b/src/middleware/serve-static/index.test.ts @@ -12,11 +12,10 @@ describe('Serve Static Middleware', () => { const serveStatic = baseServeStatic({ getContent, - pathResolve: (path) => { - return `./${path}` - }, isDir: (path) => { - return path === 'static/hello.world' + if (path === 'static/sub' || path === 'static/hello.world') { + return true + } }, onFound: (path, c) => { if (path.endsWith('hello.html')) { @@ -36,36 +35,29 @@ describe('Serve Static Middleware', () => { expect(res.status).toBe(200) expect(res.headers.get('Content-Encoding')).toBeNull() expect(res.headers.get('Content-Type')).toMatch(/^text\/html/) - expect(await res.text()).toBe('Hello in ./static/hello.html') - expect(res.headers.get('X-Custom')).toBe('Found the file at ./static/hello.html') + expect(await res.text()).toBe('Hello in static/hello.html') + expect(res.headers.get('X-Custom')).toBe('Found the file at static/hello.html') }) it('Should return 200 response - /static/sub', async () => { const res = await app.request('/static/sub') expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toMatch(/^text\/html/) - expect(await res.text()).toBe('Hello in ./static/sub/index.html') - }) - - it('Should return 200 response - /static/helloworld', async () => { - const res = await app.request('/static/helloworld') - expect(res.status).toBe(200) - expect(res.headers.get('Content-Type')).toMatch(/^text\/html/) - expect(await res.text()).toBe('Hello in ./static/helloworld/index.html') + expect(await res.text()).toBe('Hello in static/sub/index.html') }) it('Should return 200 response - /static/hello.world', async () => { const res = await app.request('/static/hello.world') expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toMatch(/^text\/html/) - expect(await res.text()).toBe('Hello in ./static/hello.world/index.html') + expect(await res.text()).toBe('Hello in static/hello.world/index.html') }) it('Should decode URI strings - /static/%E7%82%8E.txt', async () => { const res = await app.request('/static/%E7%82%8E.txt') expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toMatch(/^text\/plain/) - expect(await res.text()).toBe('Hello in ./static/炎.txt') + expect(await res.text()).toBe('Hello in static/炎.txt') }) it('Should return 404 response - /static/not-found.txt', async () => { @@ -228,15 +220,10 @@ describe('Serve Static Middleware', () => { }) describe('Changing root path', () => { - const pathResolve = (path: string) => { - return path.startsWith('/') ? path : `./${path}` - } - it('Should return the content with absolute root path', async () => { const app = new Hono() const serveStatic = baseServeStatic({ getContent, - pathResolve, root: '/home/hono/child', }) app.get('/static/*', serveStatic) @@ -249,7 +236,6 @@ describe('Serve Static Middleware', () => { const app = new Hono() const serveStatic = baseServeStatic({ getContent, - pathResolve, root: '/home/hono/../parent', }) app.get('/static/*', serveStatic) @@ -262,26 +248,24 @@ describe('Serve Static Middleware', () => { const app = new Hono() const serveStatic = baseServeStatic({ getContent, - pathResolve, root: '../home/hono', }) app.get('/static/*', serveStatic) const res = await app.request('/static/html/hello.html') - expect(await res.text()).toBe('Hello in ./../home/hono/static/html/hello.html') + expect(await res.text()).toBe('Hello in ../home/hono/static/html/hello.html') }) it('Should not allow directory traversal with . as relative path', async () => { const app = new Hono() const serveStatic = baseServeStatic({ getContent, - pathResolve, root: '.', }) app.get('*', serveStatic) const res = await app.request('///etc/passwd') - expect(res.status).toBe(404) + expect(await res.text()).toBe('Hello in etc/passwd') }) }) }) diff --git a/src/middleware/serve-static/index.ts b/src/middleware/serve-static/index.ts index 9e44a40aac..ec516cf184 100644 --- a/src/middleware/serve-static/index.ts +++ b/src/middleware/serve-static/index.ts @@ -6,8 +6,8 @@ import type { Context, Data } from '../../context' import type { Env, MiddlewareHandler } from '../../types' import { COMPRESSIBLE_CONTENT_TYPE_REGEX } from '../../utils/compress' -import { getFilePath, getFilePathWithoutDefaultDocument } from '../../utils/filepath' import { getMimeType } from '../../utils/mime' +import { defaultJoin } from './path' export type ServeStaticOptions = { root?: string @@ -27,18 +27,6 @@ const ENCODINGS = { const ENCODINGS_ORDERED_KEYS = Object.keys(ENCODINGS) as (keyof typeof ENCODINGS)[] const DEFAULT_DOCUMENT = 'index.html' -const defaultPathResolve = (path: string) => path - -const isAbsolutePath = (path: string) => { - const isUnixAbsolutePath = path.startsWith('/') - const hasDriveLetter = /^[a-zA-Z]:\\/.test(path) - const isUncPath = /^\\\\[^\\]+\\[^\\]+/.test(path) - return isUnixAbsolutePath || hasDriveLetter || isUncPath -} - -const windowsPathToUnixPath = (path: string) => { - return path.replace(/^[a-zA-Z]:/, '').replace(/\\/g, '/') -} /** * This middleware is not directly used by the user. Create a wrapper specifying `getContent()` by the environment such as Deno or Bun. @@ -46,81 +34,56 @@ const windowsPathToUnixPath = (path: string) => { export const serveStatic = ( options: ServeStaticOptions & { getContent: (path: string, c: Context) => Promise + /** + * + * `join` option according to the runtime. Example `import { join } from 'node:path`. If not specified, it will fall back to the default join function.` + */ + join?: (...paths: string[]) => string + /** + * @deprecated Currently, `pathResolve` is no longer used. + */ pathResolve?: (path: string) => string isDir?: (path: string) => boolean | undefined | Promise } ): MiddlewareHandler => { - let isAbsoluteRoot = false - let root: string - - if (options.root) { - if (isAbsolutePath(options.root)) { - isAbsoluteRoot = true - root = windowsPathToUnixPath(options.root) - root = new URL(`file://${root}`).pathname - } else { - root = options.root - } - } + const root = options.root ?? './' + const optionPath = options.path + const join = options.join ?? defaultJoin return async (c, next) => { // Do nothing if Response is already set if (c.finalized) { - await next() - return + return next() } - let filename = options.path ?? decodeURI(c.req.path) - filename = options.rewriteRequestPath ? options.rewriteRequestPath(filename) : filename - - // If it was Directory, force `/` on the end. - if (!filename.endsWith('/') && options.isDir) { - const path = getFilePathWithoutDefaultDocument({ - filename, - root, - }) - if (path && (await options.isDir(path))) { - filename += '/' + let filename: string + + if (options.path) { + filename = options.path + } else { + try { + filename = decodeURIComponent(c.req.path) + if (/(?:^|[\/\\])\.\.(?:$|[\/\\])/.test(filename)) { + throw new Error() + } + } catch { + await options.onNotFound?.(c.req.path, c) + return next() } } - let path = getFilePath({ - filename, + let path = join( root, - defaultDocument: DEFAULT_DOCUMENT, - }) - - if (!path) { - return await next() - } + !optionPath && options.rewriteRequestPath ? options.rewriteRequestPath(filename) : filename + ) - if (isAbsoluteRoot) { - path = '/' + path + if (options.isDir && (await options.isDir(path))) { + path = join(path, DEFAULT_DOCUMENT) } const getContent = options.getContent - const pathResolve = options.pathResolve ?? defaultPathResolve - path = pathResolve(path) let content = await getContent(path, c) - if (!content) { - let pathWithoutDefaultDocument = getFilePathWithoutDefaultDocument({ - filename, - root, - }) - if (!pathWithoutDefaultDocument) { - return await next() - } - pathWithoutDefaultDocument = pathResolve(pathWithoutDefaultDocument) - - if (pathWithoutDefaultDocument !== path) { - content = await getContent(pathWithoutDefaultDocument, c) - if (content) { - path = pathWithoutDefaultDocument - } - } - } - if (content instanceof Response) { return c.newResponse(content.body, content) } diff --git a/src/middleware/serve-static/path.test.ts b/src/middleware/serve-static/path.test.ts new file mode 100644 index 0000000000..92d3f4a54a --- /dev/null +++ b/src/middleware/serve-static/path.test.ts @@ -0,0 +1,50 @@ +import { join as posixJoin } from 'node:path/posix' +import { defaultJoin } from './path' + +describe('defaultJoin', () => { + describe('Comparison with node:path/posix.join', () => { + it('Should behave like path.posix.join for all path operations', () => { + const testCases = [ + // Basic path joining + ['/home/yusuke/work/app/public', 'static/main.html'], + ['public', 'sub/', 'file.html'], + ['', 'file.html'], + ['public', ''], + ['public/', 'file.html'], + ['public', 'static', 'main.html'], + ['assets', 'images', 'logo.png'], + + // Parent directory references + ['public', '../parent/file.html'], + ['public', '../../grandparent/file.html'], + ['/abs/path', '../relative.html'], + ['a/b', '../c'], + ['a/b/c', '../../d'], + + // Current directory references + ['./public', 'static/main.html'], + ['public', './file.html'], + ['./public', '/absolute/path.html'], + ['.', 'file.html'], + ['public/.', 'file.html'], + + // Edge cases + [], + ['.'], + [''], + ['/'], + ['a', 'b', 'c'], + + // Backslash handling (security) + ['static', 'test\\file.txt'], + ['public', 'path\\with\\backslash'], + ] + + testCases.forEach(([...args]) => { + const expected = posixJoin(...args) + const actual = defaultJoin(...args) + expect(actual).toBe(expected) + }) + }) + }) +}) diff --git a/src/middleware/serve-static/path.ts b/src/middleware/serve-static/path.ts new file mode 100644 index 0000000000..0253cf5df4 --- /dev/null +++ b/src/middleware/serve-static/path.ts @@ -0,0 +1,25 @@ +/** + * `defaultJoin` does not support Windows paths and always uses `/` separators. + * If you need Windows path support, please use `join` exported from `node:path` etc. instead. + */ +export const defaultJoin = (...paths: string[]): string => { + // Join non-empty paths with '/' + let result = paths.filter((p) => p !== '').join('/') + + // Normalize multiple slashes to single slash + result = result.replace(/(?<=\/)\/+/g, '') + + // Handle path resolution (. and ..) + const segments = result.split('/') + const resolved = [] + + for (const segment of segments) { + if (segment === '..' && resolved.length > 0 && resolved.at(-1) !== '..') { + resolved.pop() + } else if (segment !== '.') { + resolved.push(segment) + } + } + + return resolved.join('/') || '.' +}