Skip to content

Commit

Permalink
feat(serve-static): support absolute root (honojs#3420)
Browse files Browse the repository at this point in the history
* feat(serve-static): support absolute root

* add bun runtime test

* don't use `Request`

* add code for deno

* remove unnecessay `console.log`

* use `normalizeFilePath` instead of `URL`

* use `URL` for `options.root`

* don't allow directory traversal and fix the behavior when root including `../`
  • Loading branch information
yusukebe authored and TinsFox committed Oct 28, 2024
1 parent fd4b129 commit 96114f8
Show file tree
Hide file tree
Showing 12 changed files with 137 additions and 12 deletions.
16 changes: 12 additions & 4 deletions runtime-tests/bun/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import fs from 'fs/promises'
import path from 'path'
import { stream, streamSSE } from '../..//src/helper/streaming'
import { serveStatic, toSSG } from '../../src/adapter/bun'
import { createBunWebSocket } from '../../src/adapter/bun/websocket'
Expand All @@ -11,7 +13,6 @@ import { Hono } from '../../src/index'
import { jsx } from '../../src/jsx'
import { basicAuth } from '../../src/middleware/basic-auth'
import { jwt } from '../../src/middleware/jwt'
import { HonoRequest } from '../../src/request'

// Test just only minimal patterns.
// Because others are tested well in Cloudflare Workers environment already.
Expand Down Expand Up @@ -42,7 +43,7 @@ describe('Basic', () => {

describe('Environment Variables', () => {
it('Should return the environment variable', async () => {
const c = new Context(new HonoRequest(new Request('http://localhost/')))
const c = new Context(new Request('http://localhost/'))
const { NAME } = env<{ NAME: string }>(c)
expect(NAME).toBe('Bun')
})
Expand Down Expand Up @@ -107,6 +108,8 @@ describe('Serve Static Middleware', () => {
})
)

app.all('/static-absolute-root/*', serveStatic({ root: path.dirname(__filename) }))

beforeEach(() => onNotFound.mockClear())

it('Should return static file correctly', async () => {
Expand Down Expand Up @@ -157,6 +160,13 @@ describe('Serve Static Middleware', () => {
expect(res.status).toBe(200)
expect(await res.text()).toBe('Hi\n')
})

it('Should return 200 response - /static-absolute-root/plain.txt', async () => {
const res = await app.request('http://localhost/static-absolute-root/plain.txt')
expect(res.status).toBe(200)
expect(await res.text()).toBe('Bun!')
expect(onNotFound).not.toHaveBeenCalled()
})
})

// Bun support WebCrypto since v0.2.2
Expand Down Expand Up @@ -310,8 +320,6 @@ describe('WebSockets Helper', () => {
expect(receivedMessage).toBe(message)
})
})
const fs = require('fs').promises
const path = require('path')

async function deleteDirectory(dirPath) {
if (
Expand Down
1 change: 1 addition & 0 deletions runtime-tests/bun/static-absolute-root/plain.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Bun!
2 changes: 1 addition & 1 deletion runtime-tests/deno/.vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"eslint.validate": ["javascript", "javascriptreact", "typescript", "typescriptreact"],
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
"source.fixAll.eslint": "explicit"
},
"deno.enable": true
}
3 changes: 2 additions & 1 deletion runtime-tests/deno/deno.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
],
"imports": {
"@std/assert": "jsr:@std/assert@^1.0.3",
"@std/path": "jsr:@std/path@^1.0.3",
"@std/testing": "jsr:@std/testing@^1.0.1",
"hono/jsx/jsx-runtime": "../../src/jsx/jsx-runtime.ts"
}
}
}
26 changes: 24 additions & 2 deletions runtime-tests/deno/deno.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions runtime-tests/deno/middleware.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { assertEquals, assertMatch } from '@std/assert'
import { dirname, fromFileUrl } from '@std/path'
import { assertSpyCall, assertSpyCalls, spy } from '@std/testing/mock'
import { serveStatic } from '../../src/adapter/deno/index.ts'
import { Hono } from '../../src/hono.ts'
Expand Down Expand Up @@ -94,6 +95,16 @@ Deno.test('Serve Static middleware', async () => {
})
)

app.get('/static-absolute-root/*', serveStatic({ root: dirname(fromFileUrl(import.meta.url)) }))

app.get(
'/static/*',
serveStatic({
root: './runtime-tests/deno',
onNotFound,
})
)

let res = await app.request('http://localhost/favicon.ico')
assertEquals(res.status, 200)
assertEquals(res.headers.get('Content-Type'), 'image/x-icon')
Expand Down Expand Up @@ -132,6 +143,10 @@ Deno.test('Serve Static middleware', async () => {
res = await app.request('http://localhost/static/hello.world')
assertEquals(res.status, 200)
assertEquals(await res.text(), 'Hi\n')

res = await app.request('http://localhost/static-absolute-root/plain.txt')
assertEquals(res.status, 200)
assertEquals(await res.text(), 'Deno!')
})

Deno.test('JWT Authentication middleware', async () => {
Expand Down
1 change: 1 addition & 0 deletions runtime-tests/deno/static-absolute-root/plain.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deno!
4 changes: 2 additions & 2 deletions src/adapter/bun/serve-static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ export const serveStatic = <E extends Env = Env>(
): MiddlewareHandler => {
return async function serveStatic(c, next) {
const getContent = async (path: string) => {
path = `./${path}`
path = path.startsWith('/') ? path : `./${path}`
// @ts-ignore
const file = Bun.file(path)
return (await file.exists()) ? file : null
}
const pathResolve = (path: string) => {
return `./${path}`
return path.startsWith('/') ? path : `./${path}`
}
const isDir = async (path: string) => {
let isDir
Expand Down
2 changes: 1 addition & 1 deletion src/adapter/deno/serve-static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const serveStatic = <E extends Env = Env>(
}
}
const pathResolve = (path: string) => {
return `./${path}`
return path.startsWith('/') ? path : `./${path}`
}
const isDir = (path: string) => {
let isDir
Expand Down
58 changes: 58 additions & 0 deletions src/middleware/serve-static/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,62 @@ describe('Serve Static Middleware', () => {
expect(res.status).toBe(200)
expect(res.body).toBe(body)
})

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)

const res = await app.request('/static/html/hello.html')
expect(await res.text()).toBe('Hello in /home/hono/child/static/html/hello.html')
})

it('Should traverse the directories with absolute root path', async () => {
const app = new Hono()
const serveStatic = baseServeStatic({
getContent,
pathResolve,
root: '/home/hono/../parent',
})
app.get('/static/*', serveStatic)

const res = await app.request('/static/html/hello.html')
expect(await res.text()).toBe('Hello in /home/parent/static/html/hello.html')
})

it('Should treat the root path includes .. as relative path', async () => {
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')
})

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)
})
})
})
17 changes: 16 additions & 1 deletion src/middleware/serve-static/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ export const serveStatic = <E extends Env = Env>(
isDir?: (path: string) => boolean | undefined | Promise<boolean | undefined>
}
): MiddlewareHandler => {
let isAbsoluteRoot = false
let root: string

if (options.root) {
if (options.root.startsWith('/')) {
isAbsoluteRoot = true
root = new URL(`file://${options.root}`).pathname
} else {
root = options.root
}
}

return async (c, next) => {
// Do nothing if Response is already set
if (c.finalized) {
Expand All @@ -48,7 +60,6 @@ export const serveStatic = <E extends Env = Env>(

let filename = options.path ?? decodeURI(c.req.path)
filename = options.rewriteRequestPath ? options.rewriteRequestPath(filename) : filename
const root = options.root

// If it was Directory, force `/` on the end.
if (!filename.endsWith('/') && options.isDir) {
Expand All @@ -71,6 +82,10 @@ export const serveStatic = <E extends Env = Env>(
return await next()
}

if (isAbsoluteRoot) {
path = '/' + path
}

const getContent = options.getContent
const pathResolve = options.pathResolve ?? defaultPathResolve
path = pathResolve(path)
Expand Down
4 changes: 4 additions & 0 deletions src/utils/filepath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,9 @@ export const getFilePathWithoutDefaultDocument = (
let path = root ? root + '/' + filename : filename
path = path.replace(/^\.?\//, '')

if (root[0] !== '/' && path[0] === '/') {
return
}

return path
}

0 comments on commit 96114f8

Please sign in to comment.