some cleanup on recent work serving files and resources (#2928)

* some cleanup on recent work serving files and resources

1. Change the default path from "." to "public".
2. Add a restriction that the path does not have "." as a prefix.

as discussed in a prior PR, here:

#2887 (comment)

* incorporate @erikvanoosten's suggestions

- replace `Handler.forbidden` with `require`
- firming up logic for resource prefix limitations
- some Scaladoc improvements

I also fixed a `resourcePrefix` default value from `"."` to `"public"` that I somehow missed before.

zio-http/jvm/src/test/scala/zio/http/StaticFileRoutesSpec.scala

@@ -66,17 +66,17 @@ object StaticFileRoutesSpec extends HttpRunnableSpec {
     ),
     suite("serveResources")(
       test("serve an existing resource") {
-        val existing = "TestFile.txt"
+        val existing = "TestFile1.txt"
         val path     = Path.root / "assets"
-        val routes   = Routes.serveResources(path, ".").sandbox.deploy
+        val routes   = Routes.serveResources(path, "TestStatic").sandbox.deploy
         val request  = Request.get(URL(path / existing))
         for {
           response <- routes(request)
           body     <- response.body.asString
         } yield {
           assert(response.status)(equalTo(Status.Ok)) &&
-          assert(response.header(Header.ContentLength))(isSome(equalTo(Header.ContentLength(7L)))) &&
-          assert(body)(equalTo("foo\nbar")) &&
+          assert(response.header(Header.ContentLength))(isSome(equalTo(Header.ContentLength(50L)))) &&
+          assert(body)(equalTo("This file is added for testing Static File Server.")) &&
           assert(response.header(Header.ContentType))(
             isSome(equalTo(Header.ContentType(MediaType.text.plain, charset = Some(Charsets.Utf8)))),
           )
@@ -85,10 +85,17 @@ object StaticFileRoutesSpec extends HttpRunnableSpec {
       test("serve a non-existing resource") {
         val nonExisting = "Nothing.txt"
         val path        = Path.root / "assets"
-        val routes      = Routes.serveResources(path, ".").sandbox.deploy
+        val routes      = Routes.serveResources(path, "TestStatic").sandbox.deploy
         val request     = Request.get(URL(path / nonExisting))
         assertZIO(routes(request).map(_.status))(equalTo(Status.NotFound))
       },
+      test("insecurely serve a resource from \".\"") {
+        val existing = "TestFile.txt"
+        val path     = Path.root / "assets"
+        val routes   = Routes.serveResources(path, ".")
+        val request  = Request.get(URL(path / existing))
+        assertZIO(routes(request).map(_.status))(equalTo(Status.InternalServerError))
+      },
     ),
   ) @@ TestAspect.blocking
 }

zio-http/shared/src/main/scala/zio/http/Middleware.scala

@@ -337,7 +337,13 @@ object Middleware extends HandlerAspects {
     }
 
     def fromResource(resourcePrefix: String)(implicit trace: Trace): StaticServe[Any, Throwable] = make { (path, _) =>
-      Handler.fromResource(s"${resourcePrefix}/${path.dropLeadingSlash.encode}")
+      // validate that resourcePrefix starts with an optional slash, followed by at least 1 java identifier character
+      val rp = if (resourcePrefix.startsWith("/")) resourcePrefix else "/" + resourcePrefix
+      if (rp.length < 2 || !Character.isJavaIdentifierStart(rp.charAt(1))) {
+        Handler.die(new IllegalArgumentException("resourcePrefix must have at least 1 valid character"))
+      } else {
+        Handler.fromResource(s"${resourcePrefix}/${path.dropLeadingSlash.encode}")
+      }
     }
 
   }
@@ -391,23 +397,27 @@ object Middleware extends HandlerAspects {
     toMiddleware(path, StaticServe.fromDirectory(docRoot))
 
   /**
-   * Creates a middleware for serving static files from resources at the path
-   * `path`.
+   * Creates a middleware for serving static files at URL path `path` from
+   * resources with the given `resourcePrefix`.
    *
-   * Example: `val serveResources = Middleware.serveResources(Path.empty /
-   * "assets")`
+   * Example: `Middleware.serveResources(Path.empty / "assets", "webapp")`
    *
    * With this middleware in place, a request to
    * `https://www.domain.com/assets/folder/file1.jpg` would serve the file
-   * `src/main/resources/folder/file1.jpg`.
+   * `src/main/resources/webapp/folder/file1.jpg`. Note how the URL path is
+   * removed and the resourcePrefix prepended.
    *
-   * Provide a `resourcePrefix` if you want to limit the the resource files
-   * served. For instance, with `Middleware.serveResources(Path.empty /
-   * "assets", "public")`, a request to
-   * `https://www.domain.com/assets/folder/file1.jpg` would serve the file
-   * `src/main/resources/public/folder/file1.jpg`.
+   * Most build systems support resources in the `src/main/resources` directory.
+   * In the above example, the file `src/main/resources/webapp/folder/file1.jpg`
+   * would be served.
+   *
+   * * The `resourcePrefix` defaults to `"public"`. To prevent insecure sharing
+   * of * resource files, `resourcePrefix` must start with a `/` followed by at
+   * least 1 *
+   * [[java.lang.Character.isJavaIdentifierStart(x\$1:Char)* valid java identifier character]].
+   * The `/` * will be prepended if it is not present.
    */
-  def serveResources(path: Path, resourcePrefix: String = ".")(implicit trace: Trace): Middleware[Any] =
+  def serveResources(path: Path, resourcePrefix: String = "public")(implicit trace: Trace): Middleware[Any] =
     toMiddleware(path, StaticServe.fromResource(resourcePrefix))
 
   /**

zio-http/shared/src/main/scala/zio/http/Routes.scala

@@ -311,20 +311,27 @@ object Routes extends RoutesCompanionVersionSpecific {
     empty @@ Middleware.serveDirectory(path, docRoot)
 
   /**
-   * Creates routes for serving static files from resources at the path `path`.
+   * Creates routes for serving static files at URL path `path` from resources
+   * with the given `resourcePrefix`.
    *
-   * Example: `Routes.serveResources(Path.empty / "assets")`
+   * Example: `Routes.serveResources(Path.empty / "assets", "webapp")`
    *
    * With this routes in place, a request to
    * `https://www.domain.com/assets/folder/file1.jpg` would serve the file
-   * `src/main/resources/folder/file1.jpg`.
+   * `src/main/resources/webapp/folder/file1.jpg`. Note how the URL path is
+   * removed and the resourcePrefix prepended.
    *
-   * Provide a `resourcePrefix` if you want to limit the the resource files
-   * served. For instance, with `Routes.serveResources(Path.empty / "assets",
-   * "public")`, a request to `https://www.domain.com/assets/folder/file1.jpg`
-   * would serve the file `src/main/resources/public/folder/file1.jpg`.
+   * Most build systems support resources in the `src/main/resources` directory.
+   * In the above example, the file `src/main/resources/webapp/folder/file1.jpg`
+   * would be served.
+   *
+   * The `resourcePrefix` defaults to `"public"`. To prevent insecure sharing of
+   * resource files, `resourcePrefix` must start with a `/` followed by at least
+   * 1
+   * [[java.lang.Character.isJavaIdentifierStart(x\$1:Char)* valid java identifier character]].
+   * The `/` will be prepended if it is not present.
    */
-  def serveResources(path: Path, resourcePrefix: String = ".")(implicit trace: Trace): Routes[Any, Nothing] =
+  def serveResources(path: Path, resourcePrefix: String = "public")(implicit trace: Trace): Routes[Any, Nothing] =
     empty @@ Middleware.serveResources(path, resourcePrefix)
 
   private[http] final case class Tree[-Env](tree: RoutePattern.Tree[RequestHandler[Env, Response]]) { self =>