some cleanup on recent work serving files and resources

[sullivan-]

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)

[sullivan-]

I'm not really happy with this because we are passing off what should be a development-time (i.e., compile-time) error as a client-side HTTP error. Any ideas on how to handle this better?

[987Nabil]

I think we could have either a string interpolator or make fromResource a macro, if we want to check this at compile time.

But I don't think this is necessary. Just use an assert or throw an exception instead of returning the Handler.forbidden that way, it does not fail on request, but on start-up. Which should be good enough.

Also, I think the prefix should not be allowed to start with ~ or /. Maybe whitespace as well?

[erikvanoosten]

You could use Refined to limit the string at compile time, but I agree with @987Nabil that it would be overkill.

[erikvanoosten]

Also, I think the prefix should not be allowed to start with ~ or /. Maybe whitespace as well?

Just / should be forbidden, but something like /public/ should be fine. Does ~ means something in the context of resource paths?

[jdegoes]

Maybe Handler.die should exist??

[987Nabil]

@erikvanoosten ~ is the home path in Unix systems. And I would try to avoid some strange side effects. Maybe ~/ is what should be disallowed

[erikvanoosten]

@erikvanoosten ~ is the home path in Unix systems. And I would try to avoid some strange side effects. Maybe ~/ is what should be disallowed

~ might resolve to something in the file system, but it has no meaning in resource paths, see https://docs.oracle.com/javase/8/docs/technotes/guides/lang/resources.html#res_names.

[sullivan-]

Maybe Handler.die should exist??

That sounds good, but I don't really know how what an implementation would look like. I took a look at ZIO.orDie to get some ideas, but that method is incredibly complicated.

[jdegoes]

You can do, Handler.fromZIO(ZIO.die(...)). Composition FTW!

[erikvanoosten]

Since relative resources are a thing (relative to some class), IMHO we should encourage the users to start the resource prefix with a /. We can prepend one if none is present.

Then, instead of saying that . (or actually /.) is not allowed, I think it would be better to require that there is at least 1 character. Since resource names must be valid java identifiers, that character must be a valid as first character in a java indentifier.

Now we can express the requirement as a regular expression: ^/?\p{javaJavaIdentifierStart}.*$, or in code:

// validate that resourcePrefix starts with an optional slash, followed by at least 1 java identifier character
val rp = if (resourcePrefix.startsWith("/")) resourcePrefix else "/" + resourcePrefix
require(rp.length >= 2 && Character.isJavaIdentifierStart(rp.charAt(1)), "resourcePrefix must have at least 1 valid character")

This prevents a whole class of confusions about what . in a resource name actually means.

[erikvanoosten]

The scaladoc makes some assumptions that are not always true. It talks about
'directories' which is not a concept used by the JVM documentation for resources
(I guess the right term would be 'package', but they even avoid that).

Here is a proposal:

Creates a middleware for serving static files at URL path `path` from resources
with the given `resourcePrefix`.

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 resource `/webapp/folder/file1.jpg`. Note how the URL path is removed and
the resourcePrefix prepended.

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 valid java identifier character]]. The `/`
will be prepended if it is not present.

With this proposal the default value for resourcePrefix would change to /pubic.

[sullivan-]

I like @jdegoes's suggestion of Handler.die, but I'm not sure how to implement it.

I'm going to incorporate @erikvanoosten's suggestions. It'll just take a minute while my computer temporarily dies while I run sbt fmt test 😝 (I really need a new dev laptop)

[sullivan-]

Hmm, it looks like somebody maybe added Handler.die while I wasn't looking 😄