Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some cleanup on recent work serving files and resources #2928

Merged
merged 2 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions zio-http/jvm/src/test/scala/zio/http/StaticFileRoutesSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)))),
)
Expand All @@ -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
}
34 changes: 22 additions & 12 deletions zio-http/shared/src/main/scala/zio/http/Middleware.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
}
}

}
Expand Down Expand Up @@ -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))

/**
Expand Down
23 changes: 15 additions & 8 deletions zio-http/shared/src/main/scala/zio/http/Routes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down
Loading