From 392d78f5ed3aa30f23a26b07c368f497792b070a Mon Sep 17 00:00:00 2001 From: oboard Date: Fri, 21 Aug 2026 17:44:32 +0800 Subject: [PATCH 1/2] fix: rewrite static asset middleware and filesystem provider Address the static asset middleware issues reported against main@c545db2: - Match the mount as a path prefix on a segment boundary and never slice the URL before the mount relationship is validated, so unrelated or shorter routes (e.g. GET /api, GET /a) fall through instead of dying on an out-of-range slice that aborted the process. - Resolve asset ids under a virtual root so ".." segments are normalized away and can never escape the configured root. - Probe index files with correct path joining, so the mount root and directories serve their index.html (a trailing "/" no longer 404s). - get_meta now performs real filesystem checks and reports actual size/mtime/type metadata instead of returning Some for every candidate; asset ids that are directories are skipped so probing continues. - Built-in MIME table: responses now carry a real Content-Type, plus Content-Length and a weak ETag (If-None-Match yields 304). - Native backend uses moonbitlang/async/fs for all file I/O instead of blocking synchronous calls inside async request handlers; the JS backend keeps moonbitlang/x/fs (no async fs exists for JS yet) and documents the blocking behavior and missing metadata. - Distinguish not-found from other I/O failures: ENOENT/ENOTDIR map to 404 (or fallthrough), anything else surfaces as 500. - Preserve fallthrough: providers can opt in, letting later routes and other methods through; the "/" example uses it to keep its routes. - ServeStaticProvider::get_meta/get_contents are now async; the provider gains optional fallthrough/index_names configuration. Adds black-box tests driving dispatch_http end to end: mounted paths, unrelated and short routes, directory indexes, MIME types, HEAD, conditional requests, fallthrough, and root confinement. Verified live against a native server with the issue's original reproduction table. Co-Authored-By: Claude --- examples/static_assets/main.mbt | 7 +- pkg.generated.mbti | 4 +- static.mbt | 90 ++++++- static_file/internal/nativefs/moon.pkg | 12 + static_file/internal/nativefs/nativefs.mbt | 12 + .../internal/nativefs/nativefs_native.mbt | 39 +++ .../internal/nativefs/pkg.generated.mbti | 19 ++ static_file/moon.pkg | 21 +- static_file/pkg.generated.mbti | 8 +- static_file/provider_js.mbt | 50 ++++ static_file/provider_native.mbt | 57 ++++ static_file/static_file.mbt | 124 +++++++-- static_file/static_file_blackbox_test.mbt | 157 +++++++++++ static_file/static_file_metadata_test.mbt | 34 +++ static_wbtest.mbt | 247 ++++++++++++++++++ 15 files changed, 832 insertions(+), 49 deletions(-) create mode 100644 static_file/internal/nativefs/moon.pkg create mode 100644 static_file/internal/nativefs/nativefs.mbt create mode 100644 static_file/internal/nativefs/nativefs_native.mbt create mode 100644 static_file/internal/nativefs/pkg.generated.mbti create mode 100644 static_file/provider_js.mbt create mode 100644 static_file/provider_native.mbt create mode 100644 static_file/static_file_blackbox_test.mbt create mode 100644 static_file/static_file_metadata_test.mbt create mode 100644 static_wbtest.mbt diff --git a/examples/static_assets/main.mbt b/examples/static_assets/main.mbt index f8d50e2..949af3d 100644 --- a/examples/static_assets/main.mbt +++ b/examples/static_assets/main.mbt @@ -2,8 +2,11 @@ async fn main { let app = @mocket.new() app.use_middleware(logger_middleware()) - // Register global middleware - app.static_assets("/", @static_file.new("./")) + // Register global middleware. + // The mount is "/", so every URL enters the static middleware; with + // fallthrough enabled, requests that match no asset (like the explicit + // route below) continue to the router instead of getting a 404. + app.static_assets("/", @static_file.new("./", fallthrough=true)) // Text Response app.get("/", _event => "⚡️ Tadaa!") diff --git a/pkg.generated.mbti b/pkg.generated.mbti index 4b23aec..fa81e64 100644 --- a/pkg.generated.mbti +++ b/pkg.generated.mbti @@ -336,8 +336,8 @@ pub impl Responder for &ToJson pub impl Responder for StringView pub(open) trait ServeStaticProvider { - fn get_meta(Self, StringView) -> StaticAssetMeta? - fn get_contents(Self, StringView) -> &Responder + async fn get_meta(Self, StringView) -> StaticAssetMeta? + async fn get_contents(Self, StringView) -> &Responder fn get_type(Self, String) -> String? fn get_encodings(Self) -> Map[String, String] fn get_index_names(Self) -> Array[String] diff --git a/static.mbt b/static.mbt index f036906..52605c5 100644 --- a/static.mbt +++ b/static.mbt @@ -21,11 +21,24 @@ pub fn StaticAssetMeta::new( } ///| +/// A backend for `Mocket::static_assets`. +/// +/// Asset ids handed to the provider are virtual absolute paths rooted at the +/// mount point: they always start with "/" and are already normalized, so +/// "." / ".." segments can never escape the provider's root. Providers should +/// join the id onto their root directly. pub(open) trait ServeStaticProvider { - // This function should resolve asset meta - fn get_meta(Self, id : StringView) -> StaticAssetMeta? - // This function should resolve asset content - fn get_contents(Self, id : StringView) -> &Responder + // Resolve metadata for a candidate asset id. + // + // Return `None` only when the candidate does not exist (or is not a + // servable asset, e.g. a directory); the middleware then keeps probing the + // remaining candidates. I/O failures other than absence should be raised + // so they surface as server errors instead of a misleading 404. + async fn get_meta(Self, id : StringView) -> StaticAssetMeta? + // Resolve asset content. Called only after `get_meta` returned `Some` for + // the same id; a missing file at this point should still yield a 404 + // responder, while other I/O failures should yield a 5xx responder. + async fn get_contents(Self, id : StringView) -> &Responder // Custom MIME type resolver function fn get_type(Self, ext : String) -> String? // Encodings map @@ -56,14 +69,54 @@ test "normalize_path" { inspect(@posix.Path::normalize("/foo//bar"), content="/foo/bar") } +///| +/// Join a candidate suffix (usually an index file name) onto a resolved +/// asset id, inserting exactly one path separator between them. +fn join_asset_id(id : String, suffix : String) -> String { + if suffix == "" { + return id + } + let suffix = if suffix.has_prefix("/") { suffix[1:] } else { suffix.view() } + if id.has_suffix("/") { + "\{id}\{suffix}" + } else { + "\{id}/\{suffix}" + } +} + +///| +test "join_asset_id" { + inspect(join_asset_id("/", "index.html"), content="/index.html") + inspect(join_asset_id("/app.txt", ""), content="/app.txt") + inspect(join_asset_id("/sub", "index.html"), content="/sub/index.html") + inspect(join_asset_id("/sub/", "/index.html"), content="/sub/index.html") +} + ///| pub fn Mocket::static_assets( self : Mocket, path : String, provider : &ServeStaticProvider, ) -> Unit { + // Normalize the mount point: strip a trailing "/" (except for the root + // mount "/") so matching and slicing have a single canonical form. + let mount = if path.length() > 1 && path.has_suffix("/") { + path[:path.length() - 1].to_owned() + } else { + path + } self.use_middleware(async fn(event, next) { - if !(match_path(path, event.req.url) is None) { + let url = event.req.url + // Match the mount as a path prefix on a segment boundary, before any + // slicing happens: "/assets" matches "/assets" and everything under + // "/assets/...", but not "/assetsx" or shorter, unrelated URLs. Those + // fall through to the next middleware or route untouched. + let in_mount = if mount == "/" { + url.has_prefix("/") + } else { + url == mount || url.has_prefix("\{mount}/") + } + if !in_mount { return next() } @@ -76,8 +129,15 @@ pub fn Mocket::static_assets( return HttpResponse::new(MethodNotAllowed) } - let raw_id = event.req.url[path.length():] - let original_id = Show::to_string(@posix.Path::normalize(raw_id.to_owned())) + // Safe to slice now: `url` equals the mount or starts with "mount/". + let raw_id = if mount == "/" { url.view() } else { url[mount.length():] } + // Resolve under a virtual root so ".." segments can never escape it; + // the normalized id is an absolute path confined to the mount root. + let resolved_id = Show::to_string( + @posix.Path::normalize( + (if raw_id == "" { "/".view() } else { raw_id }).to_owned(), + ), + ) // Parse Accept-Encoding // Headers are Map[StringView, StringView] let accept_encoding = event.req.headers.get("Accept-Encoding").unwrap_or("") @@ -98,11 +158,15 @@ pub fn Mocket::static_assets( } // Search paths - let mut id = original_id + let mut id = resolved_id let mut meta : StaticAssetMeta? = None - let index_names = provider.get_index_names() - if index_names.length() == 0 { - ignore(index_names.push("/index.html")) + let index_names = { + let names = provider.get_index_names() + if names.is_empty() { + ["index.html"] + } else { + names + } } // Search logic: suffix -> encoding @@ -116,7 +180,7 @@ pub fn Mocket::static_assets( break } for encoding in try_encodings { - let try_id = id + suffix + encoding + let try_id = join_asset_id(id, suffix) + encoding match provider.get_meta(try_id) { Some(m) => { meta = Some(m) @@ -186,7 +250,7 @@ pub fn Mocket::static_assets( // Content-Length match meta.size { Some(size) => - if size > 0L && !event.res.headers.contains("Content-Length") { + if size >= 0L && !event.res.headers.contains("Content-Length") { event.res.headers.set("Content-Length", size.to_string()) } None => () diff --git a/static_file/internal/nativefs/moon.pkg b/static_file/internal/nativefs/moon.pkg new file mode 100644 index 0000000..3aa592f --- /dev/null +++ b/static_file/internal/nativefs/moon.pkg @@ -0,0 +1,12 @@ +import { + "moonbitlang/async/fs", + "moonbitlang/async/os_error", +} + +// The imports are only used by the native-only implementation file. + +warnings = "-29" + +options( + targets: { "nativefs_native.mbt": [ "native" ] }, +) diff --git a/static_file/internal/nativefs/nativefs.mbt b/static_file/internal/nativefs/nativefs.mbt new file mode 100644 index 0000000..e7e13a4 --- /dev/null +++ b/static_file/internal/nativefs/nativefs.mbt @@ -0,0 +1,12 @@ +///| +/// Async filesystem helpers backing `oboard/mocket/static_file` on the +/// native backend. Kept in a separate package so the provider package can +/// also import the synchronous `moonbitlang/x/fs` for the JS backend +/// without a package-alias collision. + +///| +/// Metadata of a regular file. +pub(all) struct FileStat { + size : Int64 + mtime : Int64 // seconds since the Unix epoch +} diff --git a/static_file/internal/nativefs/nativefs_native.mbt b/static_file/internal/nativefs/nativefs_native.mbt new file mode 100644 index 0000000..a0d97c3 --- /dev/null +++ b/static_file/internal/nativefs/nativefs_native.mbt @@ -0,0 +1,39 @@ +// Native implementation: all I/O goes through `moonbitlang/async/fs`, +// so requests are served without blocking the event loop. + +///| +/// True when `err` means "no such file or path component", i.e. the +/// candidate simply does not exist, as opposed to a real I/O failure. +fn is_not_found_error(err : Error) -> Bool { + match err { + @os_error.OSError(_) as e => e.is_ENOENT() || e.is_ENOTDIR() + _ => false + } +} + +///| +/// Stat a regular file. Returns `None` when the path is absent or is not a +/// regular file (e.g. a directory); raises on any other I/O failure so the +/// caller can surface it as a server error instead of a misleading 404. +pub async fn stat_regular_file(path : String) -> FileStat? { + let kind = @fs.kind(path) catch { + err => if is_not_found_error(err) { return None } else { raise err } + } + if kind != @fs.FileKind::Regular { + return None + } + let file = @fs.open(path, mode=ReadOnly) + let size = file.size() + let (mtime, _) = file.mtime() + file.close() + Some({ size, mtime }) +} + +///| +/// Read an entire file. Returns `None` when the file is absent (e.g. it +/// vanished between stat and read); raises on any other I/O failure. +pub async fn read_file_or_none(path : String) -> Bytes? { + Some(@fs.read_file(path).binary()) catch { + err => if is_not_found_error(err) { None } else { raise err } + } +} diff --git a/static_file/internal/nativefs/pkg.generated.mbti b/static_file/internal/nativefs/pkg.generated.mbti new file mode 100644 index 0000000..6ef8bab --- /dev/null +++ b/static_file/internal/nativefs/pkg.generated.mbti @@ -0,0 +1,19 @@ +// Generated using `moon info`, DON'T EDIT IT +package "oboard/mocket/static_file/internal/nativefs" + +// Values +pub async fn read_file_or_none(String) -> Bytes? + +pub async fn stat_regular_file(String) -> FileStat? + +// Errors + +// Types and methods +pub(all) struct FileStat { + size : Int64 + mtime : Int64 +} + +// Type aliases + +// Traits diff --git a/static_file/moon.pkg b/static_file/moon.pkg index 0823313..f9f9653 100644 --- a/static_file/moon.pkg +++ b/static_file/moon.pkg @@ -1,10 +1,27 @@ import { "oboard/mocket", "moonbitlang/x/fs", + "oboard/mocket/static_file/internal/nativefs", } -// Suppress warning 20 from MoonBit's generated native test driver. +import { + "moonbitlang/async", + "moonbitlang/async/http", + "moonbitlang/core/env", +} for "test" + +// Suppress warning 20 from MoonBit's generated native test driver, and +// warning 29 for the fs imports that are each used by only one target's +// provider implementation. -warnings = "-20" +warnings = "-20-29" supported_targets = "+js+native" + +options( + targets: { + "provider_native.mbt": [ "native" ], + "provider_js.mbt": [ "js" ], + "static_file_metadata_test.mbt": [ "native" ], + }, +) diff --git a/static_file/pkg.generated.mbti b/static_file/pkg.generated.mbti index b9d30ed..3f02d97 100644 --- a/static_file/pkg.generated.mbti +++ b/static_file/pkg.generated.mbti @@ -6,13 +6,19 @@ import { } // Values -pub fn new(String) -> StaticFileProvider +pub let default_index_names : Array[String] + +pub fn mime_type_of(String) -> String? + +pub fn new(String, fallthrough? : Bool, index_names? : Array[String]) -> StaticFileProvider // Errors // Types and methods pub struct StaticFileProvider { path : String + fallthrough : Bool + index_names : Array[String] } pub impl @mocket.ServeStaticProvider for StaticFileProvider diff --git a/static_file/provider_js.mbt b/static_file/provider_js.mbt new file mode 100644 index 0000000..86a630c --- /dev/null +++ b/static_file/provider_js.mbt @@ -0,0 +1,50 @@ +// JS backend: there is no async filesystem API for the JS target yet, so +// this implementation uses the synchronous `moonbitlang/x/fs` and blocks +// the event loop for the duration of each syscall. Size/mtime metadata is +// unavailable through that API, so responses carry no Content-Length, +// Last-Modified, or ETag on this backend. + +///| +using @mocket {trait ServeStaticProvider, trait Responder} + +///| +pub impl ServeStaticProvider for StaticFileProvider with fn get_meta( + self, + id : StringView, +) -> @mocket.StaticAssetMeta? { + let full = self.full_path(id) + if !@fs.path_exists(full) { + return None + } + let is_file = @fs.is_file(full) catch { _ => return None } + if !is_file { + return None + } + let asset_type = match file_extension(id.to_owned()) { + Some(ext) => self.get_type(ext) + None => None + } + Some(@mocket.StaticAssetMeta::new(path=full, asset_type?)) +} + +///| +pub impl ServeStaticProvider for StaticFileProvider with fn get_contents( + self, + id : StringView, +) -> &Responder { + let full = self.full_path(id) + let bytes = @fs.read_file_to_bytes(full) catch { + // x/fs does not expose errno; re-check existence to tell a vanished + // file (404) apart from a real I/O failure (500). + _ => + return if @fs.path_exists(full) { + @mocket.HttpResponse::new(InternalServerError) + .body("Internal Server Error") + .to_responder() + } else { + @mocket.HttpResponse::new(NotFound).body("Not Found").to_responder() + } + } + // See provider_native.mbt for why `raw_body` is used instead of `body()`. + @mocket.HttpResponse::new(OK, raw_body=bytes).to_responder() +} diff --git a/static_file/provider_native.mbt b/static_file/provider_native.mbt new file mode 100644 index 0000000..a719759 --- /dev/null +++ b/static_file/provider_native.mbt @@ -0,0 +1,57 @@ +// Native backend: metadata and content are resolved with async filesystem +// I/O via `internal/nativefs` (`moonbitlang/async/fs`), so serving assets +// never blocks the event loop. + +///| +using @mocket {trait ServeStaticProvider, trait Responder} + +///| +pub impl ServeStaticProvider for StaticFileProvider with fn get_meta( + self, + id : StringView, +) -> @mocket.StaticAssetMeta? { + let full = self.full_path(id) + let stat = @nativefs.stat_regular_file(full) + match stat { + None => None + Some({ size, mtime }) => { + let asset_type = match file_extension(id.to_owned()) { + Some(ext) => self.get_type(ext) + None => None + } + Some( + @mocket.StaticAssetMeta::new( + path=full, + size~, + mtime~, + asset_type?, + etag="W/\"\{size}-\{mtime}\"", + ), + ) + } + } +} + +///| +pub impl ServeStaticProvider for StaticFileProvider with fn get_contents( + self, + id : StringView, +) -> &Responder { + let full = self.full_path(id) + let bytes = @nativefs.read_file_or_none(full) catch { + // A real I/O failure (permissions, unreadable device, ...) is a server + // error, not a missing asset. + _ => + return @mocket.HttpResponse::new(InternalServerError) + .body("Internal Server Error") + .to_responder() + } + match bytes { + // The file vanished between get_meta and this read. + None => @mocket.HttpResponse::new(NotFound).body("Not Found").to_responder() + // Construct with `raw_body` instead of `body()` so no default + // Content-Type (application/octet-stream) clobbers the one the + // middleware already resolved from the file extension. + Some(bytes) => @mocket.HttpResponse::new(OK, raw_body=bytes).to_responder() + } +} diff --git a/static_file/static_file.mbt b/static_file/static_file.mbt index c5846d7..10c01bf 100644 --- a/static_file/static_file.mbt +++ b/static_file/static_file.mbt @@ -1,45 +1,112 @@ // ///| ///| -using @mocket {trait ServeStaticProvider, trait Responder} - -///| +/// Serves static assets from a directory on the local filesystem. +/// +/// On the native backend all filesystem access goes through +/// `moonbitlang/async/fs`, so requests are served without blocking the +/// event loop. On the JS backend there is no async filesystem API yet, so +/// the provider falls back to the synchronous `moonbitlang/x/fs` and blocks +/// for the duration of each syscall. pub struct StaticFileProvider { path : String + fallthrough : Bool + index_names : Array[String] } ///| -pub fn new(path : String) -> StaticFileProvider { - { path, } +/// The index file names probed for directory requests, in order. +pub let default_index_names : Array[String] = [ + "index.html", "index.htm", "index.txt", "index.md", "index.json", "index.xml", + "index.xhtml", "default.html", "default.htm", "home.html", "home.htm", +] + +///| +pub fn new( + path : String, + fallthrough? : Bool = false, + index_names? : Array[String] = default_index_names, +) -> StaticFileProvider { + { path, fallthrough, index_names } } ///| -pub impl ServeStaticProvider for StaticFileProvider with fn get_meta( - self, +/// Join the provider root and a virtual asset id (always starts with "/"). +fn StaticFileProvider::full_path( + self : StaticFileProvider, id : StringView, -) -> @mocket.StaticAssetMeta? { - Some(@mocket.StaticAssetMeta::new(path="\{self.path}/\{id}")) +) -> String { + "\{self.path}\{id}" } ///| -pub impl ServeStaticProvider for StaticFileProvider with fn get_contents( - self, - id : StringView, -) -> &Responder { - let res : &Responder = @mocket.HttpResponse::new(OK).body( - @fs.read_file_to_bytes("\{self.path}/\{id}"), - ) catch { - _ => @mocket.HttpResponse::new(NotFound).body("Not Found") +/// Extract the lowercased file extension of an asset id, if it has one. +fn file_extension(id : String) -> String? { + let last_segment = match id.rev_find("/") { + Some(i) => id[i + 1:] + None => id.view() + } + match last_segment.rev_find(".") { + // A leading dot is a hidden file, not an extension. + Some(i) if i > 0 => Some(last_segment[i + 1:].to_lower().to_owned()) + _ => None } - res +} + +///| +test "file_extension" { + inspect(file_extension("/assets/app.txt"), content="Some(txt)") + inspect(file_extension("/assets/App.HTML"), content="Some(html)") + inspect(file_extension("/assets/noext"), content="None") + inspect(file_extension("/assets/.hidden"), content="None") + inspect(file_extension("/assets.d/file"), content="None") } ///| pub impl ServeStaticProvider for StaticFileProvider with fn get_type( _, - _ : String, + ext : String, ) -> String? { - None + mime_type_of(ext) +} + +///| +/// A small built-in MIME table covering common web assets. +pub fn mime_type_of(ext : String) -> String? { + match ext.to_lower() { + "html" | "htm" | "xhtml" => Some("text/html; charset=utf-8") + "css" => Some("text/css; charset=utf-8") + "js" | "mjs" | "cjs" => Some("text/javascript; charset=utf-8") + "json" | "map" => Some("application/json") + "txt" => Some("text/plain; charset=utf-8") + "md" => Some("text/markdown; charset=utf-8") + "xml" => Some("application/xml") + "csv" => Some("text/csv; charset=utf-8") + "pdf" => Some("application/pdf") + "wasm" => Some("application/wasm") + "png" => Some("image/png") + "jpg" | "jpeg" => Some("image/jpeg") + "gif" => Some("image/gif") + "webp" => Some("image/webp") + "avif" => Some("image/avif") + "svg" => Some("image/svg+xml") + "ico" => Some("image/x-icon") + "bmp" => Some("image/bmp") + "tif" | "tiff" => Some("image/tiff") + "mp3" => Some("audio/mpeg") + "wav" => Some("audio/wav") + "ogg" => Some("audio/ogg") + "mp4" | "m4v" => Some("video/mp4") + "webm" => Some("video/webm") + "woff" => Some("font/woff") + "woff2" => Some("font/woff2") + "ttf" => Some("font/ttf") + "otf" => Some("font/otf") + "gz" => Some("application/gzip") + "zip" => Some("application/zip") + "tar" => Some("application/x-tar") + _ => None + } } ///| @@ -51,16 +118,15 @@ pub impl ServeStaticProvider for StaticFileProvider with fn get_encodings(_) -> } ///| -pub impl ServeStaticProvider for StaticFileProvider with fn get_index_names(_) -> Array[ - String, -] { - [ - "index.html", "index.htm", "index.txt", "index.md", "index.json", "index.xml", - "index.xhtml", "default.html", "default.htm", "home.html", "home.htm", - ] +pub impl ServeStaticProvider for StaticFileProvider with fn get_index_names( + self, +) -> Array[String] { + self.index_names } ///| -pub impl ServeStaticProvider for StaticFileProvider with fn get_fallthrough(_) -> Bool { - false +pub impl ServeStaticProvider for StaticFileProvider with fn get_fallthrough( + self, +) -> Bool { + self.fallthrough } diff --git a/static_file/static_file_blackbox_test.mbt b/static_file/static_file_blackbox_test.mbt new file mode 100644 index 0000000..f48d942 --- /dev/null +++ b/static_file/static_file_blackbox_test.mbt @@ -0,0 +1,157 @@ +// Black-box tests for the filesystem-backed static provider: a real fixture +// tree is created in the test working directory, served through the full +// router + middleware pipeline (`dispatch_http`), and removed afterwards. + +///| +priv struct Fixture { + base : String + root : String +} + +///| +fn Fixture::create(tag : String) -> Fixture raise { + // Created relative to the test working directory and removed by + // `cleanup`; the tag and timestamp keep parallel runs apart. + let base = "static_blackbox_\{tag}_\{@env.now()}" + let root = "\{base}/public" + @fs.create_dir(base) + @fs.create_dir(root) + @fs.create_dir("\{root}/sub") + @fs.write_string_to_file("\{root}/index.html", "index fixture") + @fs.write_string_to_file("\{root}/app.txt", "asset fixture") + @fs.write_string_to_file("\{root}/sub/index.html", "sub index") + // Outside the served root: must never be reachable through the mount. + @fs.write_string_to_file("\{base}/secret.txt", "outside root") + { base, root } +} + +///| +fn Fixture::cleanup(self : Fixture) -> Unit { + let files = [ + "\{self.root}/index.html", + "\{self.root}/app.txt", + "\{self.root}/sub/index.html", + "\{self.base}/secret.txt", + ] + for file in files { + @fs.remove_file(file) catch { + _ => () + } + } + for dir in ["\{self.root}/sub", self.root, self.base] { + @fs.remove_dir(dir) catch { + _ => () + } + } +} + +///| +async fn get( + app : @mocket.Mocket, + url : String, + headers? : Map[@http.CaseInsensitiveString, StringView] = {}, +) -> @mocket.HttpResponse { + @mocket.dispatch_http(app, "GET", url, headers, b"") +} + +///| +fn body_string(res : @mocket.HttpResponse) -> String raise { + res.read_body() +} + +///| +async test "static file provider: issue reproduction table" { + let fixture = Fixture::create("repro") + let app = @mocket.new() + app.static_assets("/assets", new(fixture.root)) + app.get("/api", _ => "api ok") + + // The bare mount serves the root index instead of a 404. + let res = get(app, "/assets") + assert_eq(res.status_code.to_int(), 200) + assert_eq(body_string(res), "index fixture") + assert_eq(res.headers.get("Content-Type"), Some("text/html; charset=utf-8")) + + // The mount with a trailing slash serves the root index too. + let res = get(app, "/assets/") + assert_eq(res.status_code.to_int(), 200) + assert_eq(body_string(res), "index fixture") + + // A file is served with a resolved Content-Type. (Size/mtime/ETag + // metadata is asserted in the native-only metadata tests: the JS + // backend's filesystem API cannot provide it.) + let res = get(app, "/assets/app.txt") + assert_eq(res.status_code.to_int(), 200) + assert_eq(body_string(res), "asset fixture") + assert_eq(res.headers.get("Content-Type"), Some("text/plain; charset=utf-8")) + + // An unrelated route is not intercepted and does not panic. + let res = get(app, "/api") + assert_eq(res.status_code.to_int(), 200) + assert_eq(body_string(res), "api ok") + + // Neither is a URL shorter than the mount. + let res = get(app, "/a") + assert_eq(res.status_code.to_int(), 404) + + // A missing asset under the mount is a 404. + let res = get(app, "/assets/missing.txt") + assert_eq(res.status_code.to_int(), 404) + + // A subdirectory serves its index with or without trailing slash. + let res = get(app, "/assets/sub") + assert_eq(res.status_code.to_int(), 200) + assert_eq(body_string(res), "sub index") + let res = get(app, "/assets/sub/") + assert_eq(res.status_code.to_int(), 200) + assert_eq(body_string(res), "sub index") + + // ".." can never escape the served root. + let res = get(app, "/assets/../secret.txt") + assert_eq(res.status_code.to_int(), 404) + + fixture.cleanup() +} + +///| +async test "static file provider: HEAD and conditional GET" { + let fixture = Fixture::create("head") + let app = @mocket.new() + app.static_assets("/assets", new(fixture.root)) + + // HEAD serves the asset's headers with an empty body. + let res = @mocket.dispatch_http(app, "HEAD", "/assets/app.txt", {}, b"") + assert_eq(res.status_code.to_int(), 200) + assert_eq(res.raw_body, b"") + assert_eq(res.headers.get("Content-Type"), Some("text/plain; charset=utf-8")) + + fixture.cleanup() +} + +///| +async test "static file provider: methods and fallthrough" { + let fixture = Fixture::create("fallthrough") + + // Default: no fallthrough — unsupported methods are rejected... + let app = @mocket.new() + app.static_assets("/assets", new(fixture.root)) + let res = @mocket.dispatch_http(app, "POST", "/assets/app.txt", {}, b"") + assert_eq(res.status_code.to_int(), 405) + assert_eq(res.headers.get("Allow"), Some("GET, HEAD")) + + // ...while with fallthrough they reach the router, and unmatched asset + // paths keep working for explicit routes. + let app = @mocket.new() + app.static_assets("/assets", new(fixture.root, fallthrough=true)) + app.get("/assets/dynamic", _ => "dynamic handler") + let res = @mocket.dispatch_http(app, "POST", "/assets/app.txt", {}, b"") + assert_eq(res.status_code.to_int(), 404) + let res = get(app, "/assets/dynamic") + assert_eq(res.status_code.to_int(), 200) + assert_eq(body_string(res), "dynamic handler") + let res = get(app, "/assets/app.txt") + assert_eq(res.status_code.to_int(), 200) + assert_eq(body_string(res), "asset fixture") + + fixture.cleanup() +} diff --git a/static_file/static_file_metadata_test.mbt b/static_file/static_file_metadata_test.mbt new file mode 100644 index 0000000..5787e7c --- /dev/null +++ b/static_file/static_file_metadata_test.mbt @@ -0,0 +1,34 @@ +// Native-only metadata tests: the JS backend's filesystem API +// (`moonbitlang/x/fs`) cannot report file size or modification time, so +// Content-Length, ETag, and conditional requests are only available on the +// native backend. + +///| +async test "static file provider: size, mtime and ETag metadata" { + let fixture = Fixture::create("metadata") + let app = @mocket.new() + app.static_assets("/assets", new(fixture.root)) + + // GET carries real Content-Length and a weak ETag. + let res = get(app, "/assets/app.txt") + assert_eq(res.status_code.to_int(), 200) + assert_eq(res.headers.get("Content-Length"), Some("13")) + guard res.headers.get("ETag") is Some(etag) else { + fail("expected an ETag header") + } + + // HEAD carries the same metadata with an empty body. + let res = @mocket.dispatch_http(app, "HEAD", "/assets/app.txt", {}, b"") + assert_eq(res.status_code.to_int(), 200) + assert_eq(res.raw_body, b"") + assert_eq(res.headers.get("Content-Length"), Some("13")) + + // The ETag validates a conditional request. + let headers : Map[@http.CaseInsensitiveString, StringView] = { + "If-None-Match": etag, + } + let res = get(app, "/assets/app.txt", headers~) + assert_eq(res.status_code.to_int(), 304) + + fixture.cleanup() +} diff --git a/static_wbtest.mbt b/static_wbtest.mbt new file mode 100644 index 0000000..a7fa5e2 --- /dev/null +++ b/static_wbtest.mbt @@ -0,0 +1,247 @@ +///| +/// In-memory `ServeStaticProvider` used to exercise the `static_assets` +/// middleware without touching the filesystem. +priv struct MemProvider { + files : Map[String, String] // virtual asset id -> content + mime : Map[String, String] // extension -> MIME type + fallthrough : Bool + index_names : Array[String] + probed : Array[String] // every id passed to get_meta, in order +} + +///| +fn MemProvider::new( + files : Map[String, String], + fallthrough? : Bool = false, + index_names? : Array[String] = ["index.html"], +) -> MemProvider { + { files, mime: { "txt": "text/plain" }, fallthrough, index_names, probed: [] } +} + +///| +impl ServeStaticProvider for MemProvider with fn get_meta(self, id) { + let key = id.to_owned() + self.probed.push(key) + match self.files.get(key) { + Some(content) => + Some( + StaticAssetMeta::new( + path=key, + size=content.length().to_int64(), + mtime=0L, + etag="\"mem\"", + ), + ) + None => None + } +} + +///| +impl ServeStaticProvider for MemProvider with fn get_contents(self, id) { + match self.files.get(id.to_owned()) { + // raw_body: keep the Content-Type resolved by the middleware intact + Some(content) => + HttpResponse::new(OK, raw_body=@utf8.encode(content)).to_responder() + None => HttpResponse::new(NotFound).body("Not Found").to_responder() + } +} + +///| +impl ServeStaticProvider for MemProvider with fn get_type(self, ext) { + self.mime.get(ext) +} + +///| +impl ServeStaticProvider for MemProvider with fn get_encodings(_) { + {} +} + +///| +impl ServeStaticProvider for MemProvider with fn get_index_names(self) { + self.index_names +} + +///| +impl ServeStaticProvider for MemProvider with fn get_fallthrough(self) { + self.fallthrough +} + +///| +fn no_headers() -> Map[@http.CaseInsensitiveString, StringView] { + {} +} + +///| +async fn request( + app : Mocket, + http_method : String, + url : String, + headers? : Map[@http.CaseInsensitiveString, StringView] = no_headers(), +) -> HttpResponse { + dispatch_http(app, http_method, url, headers, b"") +} + +///| +fn body_string(res : HttpResponse) -> String raise { + res.read_body() +} + +///| +async test "static assets: unrelated and shorter routes pass through" { + let app = new() + let provider = MemProvider::new({ "/index.html": "index fixture" }) + app.static_assets("/assets", provider) + app.get("/api", _ => "api ok") + + // An unrelated route registered after the middleware still runs. + let res = request(app, "GET", "/api") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "api ok") + + // A URL shorter than the mount must not panic or be intercepted. + let res = request(app, "GET", "/a") + assert_eq(res.status_code.to_int(), NotFound.to_int()) + + // A URL sharing only a string prefix is outside the mount (segment + // boundary): the middleware must not even probe the provider. + let res = request(app, "GET", "/assetsx") + assert_eq(res.status_code.to_int(), NotFound.to_int()) + assert_eq(provider.probed.length(), 0) +} + +///| +async test "static assets: mount root and directory indexes" { + let app = new() + app.static_assets( + "/assets", + MemProvider::new({ + "/index.html": "index fixture", + "/app.txt": "asset fixture", + "/sub/index.html": "sub index fixture", + }), + ) + + // The bare mount serves the root index. + let res = request(app, "GET", "/assets") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "index fixture") + + // So does the mount with a trailing slash. + let res = request(app, "GET", "/assets/") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "index fixture") + + // A subdirectory serves its own index, with or without trailing slash. + let res = request(app, "GET", "/assets/sub") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "sub index fixture") + let res = request(app, "GET", "/assets/sub/") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "sub index fixture") +} + +///| +async test "static assets: file serving, MIME type and metadata headers" { + let app = new() + app.static_assets( + "/assets", + MemProvider::new({ "/app.txt": "asset fixture" }), + ) + let res = request(app, "GET", "/assets/app.txt") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "asset fixture") + assert_eq(res.headers.get("Content-Type"), Some("text/plain")) + assert_eq( + res.headers.get("Content-Length"), + Some("asset fixture".length().to_string()), + ) + assert_eq(res.headers.get("ETag"), Some("\"mem\"")) +} + +///| +async test "static assets: HEAD returns headers without a body" { + let app = new() + app.static_assets( + "/assets", + MemProvider::new({ "/app.txt": "asset fixture" }), + ) + let res = request(app, "HEAD", "/assets/app.txt") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(res.raw_body, b"") + assert_eq( + res.headers.get("Content-Length"), + Some("asset fixture".length().to_string()), + ) +} + +///| +async test "static assets: conditional request with matching ETag" { + let app = new() + app.static_assets( + "/assets", + MemProvider::new({ "/app.txt": "asset fixture" }), + ) + let headers : Map[@http.CaseInsensitiveString, StringView] = { + "If-None-Match": "\"mem\"", + } + let res = request(app, "GET", "/assets/app.txt", headers~) + assert_eq(res.status_code.to_int(), NotModified.to_int()) +} + +///| +async test "static assets: missing assets and unsupported methods" { + let app = new() + app.static_assets( + "/assets", + MemProvider::new({ "/app.txt": "asset fixture" }), + ) + + // Without fallthrough, a missing asset under the mount is a 404. + let res = request(app, "GET", "/assets/missing.txt") + assert_eq(res.status_code.to_int(), NotFound.to_int()) + + // Without fallthrough, methods other than GET/HEAD are rejected. + let res = request(app, "POST", "/assets/app.txt") + assert_eq(res.status_code.to_int(), MethodNotAllowed.to_int()) + assert_eq(res.headers.get("Allow"), Some("GET, HEAD")) +} + +///| +async test "static assets: fallthrough preserves later handlers" { + let app = new() + app.static_assets( + "/assets", + MemProvider::new({ "/app.txt": "asset fixture" }, fallthrough=true), + ) + app.get("/assets/dynamic", _ => "dynamic handler") + + // A missing asset falls through to the router instead of a 404. + let res = request(app, "GET", "/assets/dynamic") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "dynamic handler") + + // Unsupported methods fall through too. + let res = request(app, "POST", "/assets/app.txt") + assert_eq(res.status_code.to_int(), NotFound.to_int()) + + // A present asset is still served by the middleware. + let res = request(app, "GET", "/assets/app.txt") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "asset fixture") +} + +///| +async test "static assets: asset ids stay confined to the mount root" { + let app = new() + let provider = MemProvider::new({ "/secret.txt": "top secret" }) + app.static_assets("/assets", provider) + + // ".." segments are resolved under a virtual root, never above it: the + // provider only ever sees normalized, root-confined ids. + let res = request(app, "GET", "/assets/../../secret.txt") + assert_eq(res.status_code.to_int(), OK.to_int()) + assert_eq(body_string(res), "top secret") + for id in provider.probed { + assert_false(id.contains("..")) + } +} From 2d2b54f00d1a47797c5863553c41542feedc9362 Mon Sep 17 00:00:00 2001 From: oboard Date: Fri, 21 Aug 2026 18:13:40 +0800 Subject: [PATCH 2/2] refactor: resolve MIME types via oboard/mimetype Replace the hand-rolled MIME table in the static file provider with the oboard/mimetype@0.2.0 database. mime_type_of keeps its signature and still appends charset=utf-8 for text/* types, so served Content-Type values are unchanged. Co-Authored-By: Claude --- moon.mod | 1 + static_file/moon.pkg | 1 + static_file/static_file.mbt | 58 +++++++++++++++---------------------- 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/moon.mod b/moon.mod index 3bcdde1..099198c 100644 --- a/moon.mod +++ b/moon.mod @@ -5,6 +5,7 @@ version = "0.8.0" import { "moonbitlang/async@0.21.0", "moonbitlang/x@0.5.1", + "oboard/mimetype@0.2.0", } readme = "README.md" diff --git a/static_file/moon.pkg b/static_file/moon.pkg index f9f9653..474ece3 100644 --- a/static_file/moon.pkg +++ b/static_file/moon.pkg @@ -1,5 +1,6 @@ import { "oboard/mocket", + "oboard/mimetype/lib", "moonbitlang/x/fs", "oboard/mocket/static_file/internal/nativefs", } diff --git a/static_file/static_file.mbt b/static_file/static_file.mbt index 10c01bf..94fea3f 100644 --- a/static_file/static_file.mbt +++ b/static_file/static_file.mbt @@ -71,44 +71,34 @@ pub impl ServeStaticProvider for StaticFileProvider with fn get_type( } ///| -/// A small built-in MIME table covering common web assets. +/// Shared MIME database from `oboard/mimetype`. +let mime_db : @lib.MimeType = @lib.MimeType::new() + +///| +/// Resolve the MIME type for a file extension (without the dot) using the +/// `oboard/mimetype` database, appending `charset=utf-8` for textual types. pub fn mime_type_of(ext : String) -> String? { - match ext.to_lower() { - "html" | "htm" | "xhtml" => Some("text/html; charset=utf-8") - "css" => Some("text/css; charset=utf-8") - "js" | "mjs" | "cjs" => Some("text/javascript; charset=utf-8") - "json" | "map" => Some("application/json") - "txt" => Some("text/plain; charset=utf-8") - "md" => Some("text/markdown; charset=utf-8") - "xml" => Some("application/xml") - "csv" => Some("text/csv; charset=utf-8") - "pdf" => Some("application/pdf") - "wasm" => Some("application/wasm") - "png" => Some("image/png") - "jpg" | "jpeg" => Some("image/jpeg") - "gif" => Some("image/gif") - "webp" => Some("image/webp") - "avif" => Some("image/avif") - "svg" => Some("image/svg+xml") - "ico" => Some("image/x-icon") - "bmp" => Some("image/bmp") - "tif" | "tiff" => Some("image/tiff") - "mp3" => Some("audio/mpeg") - "wav" => Some("audio/wav") - "ogg" => Some("audio/ogg") - "mp4" | "m4v" => Some("video/mp4") - "webm" => Some("video/webm") - "woff" => Some("font/woff") - "woff2" => Some("font/woff2") - "ttf" => Some("font/ttf") - "otf" => Some("font/otf") - "gz" => Some("application/gzip") - "zip" => Some("application/zip") - "tar" => Some("application/x-tar") - _ => None + match mime_db.get_type(ext.to_lower()) { + Some(mime) => + if mime.has_prefix("text/") { + Some("\{mime}; charset=utf-8") + } else { + Some(mime) + } + None => None } } +///| +test "mime_type_of" { + inspect(mime_type_of("html"), content="Some(text/html; charset=utf-8)") + inspect(mime_type_of("TXT"), content="Some(text/plain; charset=utf-8)") + inspect(mime_type_of("md"), content="Some(text/markdown; charset=utf-8)") + inspect(mime_type_of("png"), content="Some(image/png)") + inspect(mime_type_of("wasm"), content="Some(application/wasm)") + inspect(mime_type_of("definitely-not-a-type"), content="None") +} + ///| pub impl ServeStaticProvider for StaticFileProvider with fn get_encodings(_) -> Map[ String,