fileserver: Support glob expansion in file matcher #4993
Conversation
| case "http.request.uri.path.file.base": | ||
| return strings.TrimSuffix(path.Base(req.URL.Path), path.Ext(req.URL.Path)), true | ||
| case "http.request.uri.path.file.ext": | ||
| return path.Ext(req.URL.Path), true |
There was a problem hiding this comment.
Btw, with an input of foo.tar.gz, this will return .gz only. Should we write our own logic to repeat .Ext() to pop extensions off the end until none remain, and return that? 🤔
This is relevant when pairing with file.base because the base will probably end up being file.tar which is awkward
There was a problem hiding this comment.
Well, there are 2 extensions. Imagine a path of index.012abcdf.html -- there's only one file extension and the part in the middle is just a checksum. I'm not sure how to handle that better. It's arbitrary either way, and more commonly there is only 1 ext.
|
Re placeholders, considering we already have A realistic example might look like this: i.e. if the file exists as-is on disk, serve that. If the file exists with an infix in front of the extension, rewrite to that instead. If neither, 404. |
|
@mholt @francislavoie Wow guys! I wasn't expecting that, thank you. Family commitments today but will test it out tomorrow first chance I get. |
|
@iamjrock Did you have a chance to test this out? |
Sorry no not yet - hoping to get it tested tonight! |
|
@iamjrock I'll be merging this today in preparation for the first beta release. Hope to hear from you before then if it doesn't work! 😅 |
|
Hi @mholt I am really sorry, I've had to tool down for a few days due to an unexpected family matter. But I really appreciate what you've done here and really want to get you that feedback, so I have asked one of my colleagues to test it in my absence and I will follow up. We're in GMT+8 here so it's early morning and I would expect a reply by close of biz today, possibly earlier. |
|
@iamjrock Ah, no worries -- honestly it's not a big deal, I just wanted to let you know I don't mean to ignore your feedback. I am just going forward with a very unannounced schedule, so I want you to know that I'm not ignoring you or unappreciative of your collaboration with this! 👍 Hope your family is well. Don't go out of your way to test this; it's just going into the first beta, so if something is broken or doesn't meet your requirements we can still fix it later. Cheers! |
This adds:
- `{file.*}` -> `{http.request.uri.path.file.*}`
- `{file_match.*}` -> `{http.matchers.file.*}`
This is a follow-up to #4993 which introduces the new URI file placeholders, and a shortcut for using `file` matcher output.
For example, where the `try_files` directive is a shortcut for this:
```
@try_files file <files...>
rewrite @try_files {http.matchers.file.relative}
```
It could instead be:
```
@try_files file <files...>
rewrite @try_files {file_match.relative}
```
This adds:
- `{file.*}` -> `{http.request.uri.path.file.*}`
- `{file_match.*}` -> `{http.matchers.file.*}`
This is a follow-up to #4993 which introduces the new URI file placeholders, and a shortcut for using `file` matcher output.
For example, where the `try_files` directive is a shortcut for this:
```
@try_files file <files...>
rewrite @try_files {http.matchers.file.relative}
```
It could instead be:
```
@try_files file <files...>
rewrite @try_files {file_match.relative}
```
|
OK thx for the reprieve, all good now thx. My colleagues tested this and report that it is working great for our use case! |
|
Hurray, thanks for confirming! |
This change adds support for glob patterns in the file matcher's
try_filesparameter (and by extension, thetry_filesCaddyfile directive).For a directory like this:
and a config like this:
a request to
/index.htmlwould rewrite to/index.1.html.Also added the ability to change the policy on the
try_filesdirective:For the same request,
/index.html, this policy would rewrite to/index.3.html.It'd be nice if we could figure out Caddyfile shorthands for the new
{http.request.uri.path.file.base}and{http.request.uri.path.file.ext}placeholders. Maybe they should be like{http.request.uri.path.file_base}instead? We have{path.*}shorthands but that only works one more level deep. We could make it work 2 levels deep or just make different shorthands or use the underscore instead.I was careful to escape any special glob characters when evaluating placeholders, so a request to
/*can't lead to any surprising results. Not a security concern per-se, as the globbing is still within the site root, just unexpected.Closes #4988. /cc @iamjrock -- can you please try this out?