Skip to content

Commit

Permalink
use URL for options.root
Browse files Browse the repository at this point in the history
  • Loading branch information
yusukebe committed Sep 21, 2024
1 parent 5f07dd7 commit 613b3c1
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 79 deletions.
6 changes: 1 addition & 5 deletions runtime-tests/bun/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,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 @@ -109,10 +108,7 @@ describe('Serve Static Middleware', () => {
})
)

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

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

Expand Down
5 changes: 1 addition & 4 deletions runtime-tests/deno/middleware.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,7 @@ Deno.test('Serve Static middleware', async () => {
})
)

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

app.get(
'/static/*',
Expand Down
17 changes: 17 additions & 0 deletions src/middleware/serve-static/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ describe('Serve Static Middleware', () => {

app.get('/static/*', serveStatic)

const serveStaticAbsoluteRoot = baseServeStatic({
getContent,
pathResolve: (path) => {
return path
},
root: '/home/hono/../foo',
allowAbsoluteRoot: true,

Check failure on line 36 in src/middleware/serve-static/index.test.ts

View workflow job for this annotation

GitHub Actions / Main

Object literal may only specify known properties, and 'allowAbsoluteRoot' does not exist in type 'ServeStaticOptions<Env> & { getContent: (path: string, c: Context<Env, any, {}>) => Promise<Response | Data | null>; pathResolve?: ((path: string) => string) | undefined; isDir?: ((path: string) => boolean | ... 1 more ... | undefined) | undefined; }'.
})

app.get('/static-absolute/*', serveStaticAbsoluteRoot)

beforeEach(() => {
getContent.mockClear()
})
Expand Down Expand Up @@ -226,4 +237,10 @@ describe('Serve Static Middleware', () => {
expect(res.status).toBe(200)
expect(res.body).toBe(body)
})

it('Should traverse directories with absolute root path', async () => {
const res = await app.request('/static-absolute/bar/hello.html')
expect(res.status).toBe(200)
expect(await res.text()).toBe('Hello in /home/foo/static-absolute/bar/hello.html')
})
})
20 changes: 14 additions & 6 deletions src/middleware/serve-static/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { getMimeType } from '../../utils/mime'
export type ServeStaticOptions<E extends Env = Env> = {
root?: string
path?: string
allowAbsoluteRoot?: boolean
precompressed?: boolean
mimes?: Record<string, string>
rewriteRequestPath?: (path: string) => string
Expand Down Expand Up @@ -40,6 +39,16 @@ 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
}

return async (c, next) => {
// Do nothing if Response is already set
if (c.finalized) {
Expand All @@ -49,16 +58,12 @@ 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

const allowAbsoluteRoot = options.allowAbsoluteRoot ?? false

// If it was Directory, force `/` on the end.
if (!filename.endsWith('/') && options.isDir) {
const path = getFilePathWithoutDefaultDocument({
filename,
root,
allowAbsoluteRoot,
})
if (path && (await options.isDir(path))) {
filename += '/'
Expand All @@ -68,14 +73,17 @@ export const serveStatic = <E extends Env = Env>(
let path = getFilePath({
filename,
root,
allowAbsoluteRoot,
defaultDocument: DEFAULT_DOCUMENT,
})

if (!path) {
return await next()
}

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

const getContent = options.getContent
const pathResolve = options.pathResolve ?? defaultPathResolve
path = pathResolve(path)
Expand Down
32 changes: 0 additions & 32 deletions src/utils/filepath.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,38 +60,6 @@ describe('getFilePath', () => {
expect(getFilePath({ filename: 'filename.suffix_index' })).toBe('filename.suffix_index')
expect(getFilePath({ filename: 'filename.suffix-index' })).toBe('filename.suffix-index')
})

it('Should return file path correctly with allowAbsoluteRoot', async () => {
const allowAbsoluteRoot = true
expect(getFilePath({ filename: 'foo.txt', allowAbsoluteRoot })).toBe('/foo.txt')
expect(getFilePath({ filename: 'foo.txt', root: '/p', allowAbsoluteRoot })).toBe('/p/foo.txt')
expect(getFilePath({ filename: 'foo', root: '/p', allowAbsoluteRoot })).toBe(
'/p/foo/index.html'
)
expect(getFilePath({ filename: 'foo.txt', root: '/p/../p2', allowAbsoluteRoot })).toBe(
'/p2/foo.txt'
)
expect(getFilePath({ filename: 'foo', root: '/p/bar', allowAbsoluteRoot })).toBe(
'/p/bar/foo/index.html'
)
expect(
getFilePath({ filename: 'foo.txt', root: slashToBackslash('/p'), allowAbsoluteRoot })
).toBe('/p/foo.txt')
expect(
getFilePath({ filename: 'foo.txt', root: slashToBackslash('/p/../p2'), allowAbsoluteRoot })
).toBe('/p2/foo.txt')
expect(
getFilePath({ filename: 'foo.txt', root: slashToBackslash('/p/.../p2'), allowAbsoluteRoot })
).toBe('/p/.../p2/foo.txt')

expect(
getFilePathWithoutDefaultDocument({
filename: '/%2e%2e/%2e%2e/%2e%2e/%2e%2e/etc/passwd',
root: '/p/p2',
allowAbsoluteRoot: true,
})
).toBe('/p/p2/%2e%2e/%2e%2e/%2e%2e/%2e%2e/etc/passwd') // /etc/passwd
})
})

function slashToBackslash(filename: string) {
Expand Down
33 changes: 1 addition & 32 deletions src/utils/filepath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ type FilePathOptions = {
filename: string
root?: string
defaultDocument?: string
allowAbsoluteRoot?: boolean
}

export const getFilePath = (options: FilePathOptions): string | undefined => {
Expand All @@ -24,31 +23,12 @@ export const getFilePath = (options: FilePathOptions): string | undefined => {

const path = getFilePathWithoutDefaultDocument({
root: options.root,
allowAbsoluteRoot: options.allowAbsoluteRoot,
filename,
})

return path
}

const normalizeFilePath = (filePath: string) => {
const parts = filePath.split(/[\/\\]/)

const result = []

for (const part of parts) {
if (part === '' || part === '.') {
continue
} else if (part === '..') {
result.pop()
} else {
result.push(part)
}
}

return '/' + (result.length === 1 ? result[0] : result.join('/'))
}

export const getFilePathWithoutDefaultDocument = (
options: Omit<FilePathOptions, 'defaultDocument'>
): string | undefined => {
Expand All @@ -62,9 +42,6 @@ export const getFilePathWithoutDefaultDocument = (
// /foo.html => foo.html
filename = filename.replace(/^\.?[\/\\]/, '')

// assets\foo => assets/foo
root = root.replace(/\\/, '/')

// foo\bar.txt => foo/bar.txt
filename = filename.replace(/\\/, '/')

Expand All @@ -73,15 +50,7 @@ export const getFilePathWithoutDefaultDocument = (

// ./assets/foo.html => assets/foo.html
let path = root ? root + '/' + filename : filename

if (!options.allowAbsoluteRoot) {
path = path.replace(/^\.?\//, '')
} else {
// assets => /assets
path = path.replace(/^(?!\/)/, '/')
// /assets/foo/../bar => /assets/bar
path = normalizeFilePath(path)
}
path = path.replace(/^\.?\//, '')

return path
}

0 comments on commit 613b3c1

Please sign in to comment.