fix: rewrite static asset middleware and filesystem provider

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!")

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"

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]

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 => ()

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" ] },
+)

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
+}

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 }
+  }
+}

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

static_file/moon.pkg

@@ -1,10 +1,28 @@
 import {
   "oboard/mocket",
+  "oboard/mimetype/lib",
   "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" ],
+  },
+)

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
 

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()
+}