Skip to content

Commit a0be2f4

Browse files
yusukebeusualoma
andauthored
feat(serve-static): support absolute path (#257)
* feat(serve-static): support absolute path * add ci-windows * fix the formats * fix the jest path * support Windows * test support Windows * fix the logic * correct handling paths for Windows * typo * update README * simplify the implementation * format * fixed file pathes in the test * resolve if both `root` and `path` set Co-authored-by: Taku Amano <[email protected]> * resolve root when initialize the app Co-authored-by: Taku Amano <[email protected]> * Currect the behavior if root is set and path is `/favicon.ico` --------- Co-authored-by: Taku Amano <[email protected]>
1 parent b5663eb commit a0be2f4

File tree

5 files changed

+140
-31
lines changed

5 files changed

+140
-31
lines changed

.github/workflows/ci.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,15 @@ jobs:
2929
- run: bun run lint
3030
- run: bun run build
3131
- run: bun run test
32+
33+
ci-windows:
34+
runs-on: windows-latest
35+
36+
steps:
37+
- uses: actions/checkout@v4
38+
- uses: actions/setup-node@v4
39+
with:
40+
node-version: 22.x
41+
- uses: oven-sh/setup-bun@v2
42+
- run: bun install
43+
- run: bun run test

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ import { serveStatic } from '@hono/node-server/serve-static'
168168
app.use('/static/*', serveStatic({ root: './' }))
169169
```
170170

171-
Note that `root` must be _relative_ to the current working directory from which the app was started. Absolute paths are not supported.
171+
If using a relative path, `root` will be relative to the current working directory from which the app was started.
172172

173173
This can cause confusion when running your application locally.
174174

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
}
5555
},
5656
"scripts": {
57-
"test": "node --expose-gc ./node_modules/.bin/jest",
57+
"test": "node --expose-gc node_modules/jest/bin/jest.js",
5858
"build": "tsup --external hono",
5959
"watch": "tsup --watch",
6060
"postbuild": "publint",
@@ -99,4 +99,4 @@
9999
"peerDependencies": {
100100
"hono": "^4"
101101
}
102-
}
102+
}

src/serve-static.ts

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import type { Context, Env, MiddlewareHandler } from 'hono'
2-
import { getFilePath, getFilePathWithoutDefaultDocument } from 'hono/utils/filepath'
32
import { getMimeType } from 'hono/utils/mime'
4-
import { createReadStream, lstatSync } from 'node:fs'
53
import type { ReadStream, Stats } from 'node:fs'
4+
import { createReadStream, lstatSync } from 'node:fs'
5+
import { join, resolve } from 'node:path'
66

77
export type ServeStaticOptions<E extends Env = Env> = {
88
/**
@@ -44,10 +44,6 @@ const createStreamBody = (stream: ReadStream) => {
4444
return body
4545
}
4646

47-
const addCurrentDirPrefix = (path: string) => {
48-
return `./${path}`
49-
}
50-
5147
const getStats = (path: string) => {
5248
let stats: Stats | undefined
5349
try {
@@ -60,6 +56,9 @@ const getStats = (path: string) => {
6056
export const serveStatic = <E extends Env = any>(
6157
options: ServeStaticOptions<E> = { root: '' }
6258
): MiddlewareHandler<E> => {
59+
const root = resolve(options.root || '.')
60+
const optionPath = options.path
61+
6362
return async (c, next) => {
6463
// Do nothing if Response is already set
6564
if (c.finalized) {
@@ -69,35 +68,40 @@ export const serveStatic = <E extends Env = any>(
6968
let filename: string
7069

7170
try {
72-
filename = options.path ?? decodeURIComponent(c.req.path)
71+
const rawPath = optionPath ?? c.req.path
72+
// Prevent encoded path traversal attacks
73+
if (!optionPath) {
74+
const decodedPath = decodeURIComponent(rawPath)
75+
if (decodedPath.includes('..')) {
76+
await options.onNotFound?.(rawPath, c)
77+
return next()
78+
}
79+
}
80+
filename = optionPath ?? decodeURIComponent(c.req.path)
7381
} catch {
7482
await options.onNotFound?.(c.req.path, c)
7583
return next()
7684
}
7785

78-
let path = getFilePathWithoutDefaultDocument({
79-
filename: options.rewriteRequestPath ? options.rewriteRequestPath(filename, c) : filename,
80-
root: options.root,
81-
})
86+
const requestPath = options.rewriteRequestPath
87+
? options.rewriteRequestPath(filename, c)
88+
: filename
8289

83-
if (path) {
84-
path = addCurrentDirPrefix(path)
85-
} else {
86-
return next()
87-
}
90+
let path = optionPath
91+
? options.root
92+
? resolve(join(root, optionPath))
93+
: optionPath
94+
: resolve(join(root, requestPath))
8895

8996
let stats = getStats(path)
9097

9198
if (stats && stats.isDirectory()) {
92-
path = getFilePath({
93-
filename: options.rewriteRequestPath ? options.rewriteRequestPath(filename, c) : filename,
94-
root: options.root,
95-
defaultDocument: options.index ?? 'index.html',
96-
})
99+
const indexFile = options.index ?? 'index.html'
100+
path = resolve(join(path, indexFile))
97101

98-
if (path) {
99-
path = addCurrentDirPrefix(path)
100-
} else {
102+
// Security check: prevent path traversal attacks
103+
if (!optionPath && !path.startsWith(root)) {
104+
await options.onNotFound?.(path, c)
101105
return next()
102106
}
103107

test/serve-static.test.ts

Lines changed: 97 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Hono } from 'hono'
2-
32
import request from 'supertest'
3+
import path from 'node:path'
44
import { serveStatic } from './../src/serve-static'
55
import { createAdaptorServer } from './../src/server'
66

@@ -68,7 +68,9 @@ describe('Serve Static Middleware', () => {
6868
expect(res.status).toBe(200)
6969
expect(res.text).toBe('<h1>Hello Hono</h1>')
7070
expect(res.headers['content-type']).toBe('text/html; charset=utf-8')
71-
expect(res.headers['x-custom']).toBe('Found the file at ./test/assets/static/index.html')
71+
expect(res.headers['x-custom']).toMatch(
72+
/Found the file at .*[\/\\]test[\/\\]assets[\/\\]static[\/\\]index\.html$/
73+
)
7274
})
7375

7476
it('Should return hono.html', async () => {
@@ -167,8 +169,8 @@ describe('Serve Static Middleware', () => {
167169
it('Should handle the `onNotFound` option', async () => {
168170
const res = await request(server).get('/on-not-found/foo.txt')
169171
expect(res.status).toBe(404)
170-
expect(notFoundMessage).toBe(
171-
'./not-found/on-not-found/foo.txt is not found, request to /on-not-found/foo.txt'
172+
expect(notFoundMessage).toMatch(
173+
/.*[\/\\]not-found[\/\\]on-not-found[\/\\]foo\.txt is not found, request to \/on-not-found\/foo\.txt$/
172174
)
173175
})
174176

@@ -226,4 +228,95 @@ describe('Serve Static Middleware', () => {
226228
expect(res.headers['vary']).toBeUndefined()
227229
expect(res.text).toBe('Hello Not Compressed')
228230
})
231+
232+
describe('Absolute path', () => {
233+
const rootPaths = [
234+
path.join(__dirname, 'assets'),
235+
__dirname + path.sep + '..' + path.sep + 'test' + path.sep + 'assets',
236+
]
237+
rootPaths.forEach((root) => {
238+
describe(root, () => {
239+
const app = new Hono()
240+
const server = createAdaptorServer(app)
241+
app.use('/static/*', serveStatic({ root }))
242+
app.use('/favicon.ico', serveStatic({ path: root + path.sep + 'favicon.ico' }))
243+
244+
it('Should return index.html', async () => {
245+
const res = await request(server).get('/static')
246+
expect(res.status).toBe(200)
247+
expect(res.headers['content-type']).toBe('text/html; charset=utf-8')
248+
expect(res.text).toBe('<h1>Hello Hono</h1>')
249+
})
250+
251+
it('Should return correct headers and data for text', async () => {
252+
const res = await request(server).get('/static/plain.txt')
253+
expect(res.status).toBe(200)
254+
expect(res.headers['content-type']).toBe('text/plain; charset=utf-8')
255+
expect(res.text).toBe('This is plain.txt')
256+
})
257+
it('Should return correct headers for icons', async () => {
258+
const res = await request(server).get('/favicon.ico')
259+
expect(res.status).toBe(200)
260+
expect(res.headers['content-type']).toBe('image/x-icon')
261+
})
262+
})
263+
})
264+
})
265+
266+
describe('Root and path combination tests', () => {
267+
const rootPaths = [
268+
path.join(__dirname, 'assets'),
269+
path.join(__dirname, 'assets'),
270+
__dirname + path.sep + '..' + path.sep + 'test' + path.sep + 'assets',
271+
]
272+
const optionPaths = ['favicon.ico', '/favicon.ico']
273+
rootPaths.forEach((root) => {
274+
optionPaths.forEach((optionPath) => {
275+
describe(`${root} + ${optionPath}`, () => {
276+
const app = new Hono()
277+
const server = createAdaptorServer(app)
278+
279+
app.use(
280+
'/favicon.ico',
281+
serveStatic({
282+
root,
283+
path: optionPath,
284+
})
285+
)
286+
287+
it('Should return 200 response if both root and path set', async () => {
288+
const res = await request(server).get('/favicon.ico')
289+
expect(res.status).toBe(200)
290+
expect(res.headers['content-type']).toBe('image/x-icon')
291+
})
292+
})
293+
})
294+
})
295+
})
296+
297+
describe('Security tests', () => {
298+
const app = new Hono()
299+
const server = createAdaptorServer(app)
300+
app.use('/static/*', serveStatic({ root: './test/assets' }))
301+
302+
it('Should prevent path traversal attacks with double dots', async () => {
303+
const res = await request(server).get('/static/../secret.txt')
304+
expect(res.status).toBe(404)
305+
})
306+
307+
it('Should prevent path traversal attacks with multiple levels', async () => {
308+
const res = await request(server).get('/static/../../package.json')
309+
expect(res.status).toBe(404)
310+
})
311+
312+
it('Should prevent path traversal attacks with mixed separators', async () => {
313+
const res = await request(server).get('/static/..\\..\\package.json')
314+
expect(res.status).toBe(404)
315+
})
316+
317+
it('Should prevent path traversal attacks with encoded dots', async () => {
318+
const res = await request(server).get('/static/%2e%2e%2fsecret.txt')
319+
expect(res.status).toBe(404)
320+
})
321+
})
229322
})

0 commit comments

Comments
 (0)