fileserver: Improve and clarify file hiding logic (#3844)

* fileserver: Improve and clarify file hiding logic

* Oops, forgot to run integration tests

* Make this one integration test OS-agnostic

* See if this appeases the Windows gods

* D'oh
This commit is contained in:
Matt Holt 2020-11-02 14:20:12 -07:00 committed by GitHub
parent 937ec34201
commit 8d038ca515
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 119 additions and 39 deletions

View file

@ -1,9 +1,9 @@
:80
file_server
respond 200
@untrusted not remote_ip 10.1.1.0/24
file_server @untrusted
respond @untrusted 401
----------
{
"apps": {
@ -30,20 +30,16 @@ file_server @untrusted
],
"handle": [
{
"handler": "file_server",
"hide": [
"Caddyfile"
]
"handler": "static_response",
"status_code": 401
}
]
},
{
"handle": [
{
"handler": "file_server",
"hide": [
"Caddyfile"
]
"handler": "static_response",
"status_code": 200
}
]
}

View file

@ -15,6 +15,7 @@
package fileserver
import (
"path/filepath"
"strings"
"github.com/caddyserver/caddy/v2"
@ -85,7 +86,14 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
// hide the Caddyfile (and any imported Caddyfiles)
if configFiles := h.Caddyfiles(); len(configFiles) > 0 {
for _, file := range configFiles {
file = filepath.Clean(file)
if !fileHidden(file, fsrv.Hide) {
// if there's no path separator, the file server module will hide all
// files by that name, rather than a specific one; but we want to hide
// only this specific file, so ensure there's always a path separator
if !strings.Contains(file, separator) {
file = "." + separator + file
}
fsrv.Hide = append(fsrv.Hide, file)
}
}

View file

@ -19,7 +19,6 @@ import (
"net/http"
"os"
"path"
"path/filepath"
"strings"
"time"
@ -286,7 +285,7 @@ func strictFileExists(file string) (os.FileInfo, bool) {
// https://stackoverflow.com/a/12518877/1048862
return nil, false
}
if strings.HasSuffix(file, string(filepath.Separator)) {
if strings.HasSuffix(file, separator) {
// by convention, file paths ending
// in a path separator must be a directory
return stat, stat.IsDir()

View file

@ -46,7 +46,20 @@ type FileServer struct {
Root string `json:"root,omitempty"`
// A list of files or folders to hide; the file server will pretend as if
// they don't exist. Accepts globular patterns like "*.hidden" or "/foo/*/bar".
// they don't exist. Accepts globular patterns like "*.ext" or "/foo/*/bar"
// as well as placeholders. Because site roots can be dynamic, this list
// uses file system paths, not request paths. To clarify, the base of
// relative paths is the current working directory, NOT the site root.
//
// Entries without a path separator (`/` or `\` depending on OS) will match
// any file or directory of that name regardless of its path. To hide only a
// specific file with a name that may not be unique, always use a path
// separator. For example, to hide all files or folder trees named "hidden",
// put "hidden" in the list. To hide only ./hidden, put "./hidden" in the list.
//
// When possible, all paths are resolved to their absolute form before
// comparisons are made. For maximum clarity and explictness, use complete,
// absolute paths; or, for greater portability, use relative paths instead.
Hide []string `json:"hide,omitempty"`
// The names of files to try as index files if a folder is requested.
@ -101,6 +114,16 @@ func (fsrv *FileServer) Provision(ctx caddy.Context) error {
fsrv.Browse.template = tpl
}
// for hide paths that are static (i.e. no placeholders), we can transform them into
// absolute paths before the server starts for very slight performance improvement
for i, h := range fsrv.Hide {
if !strings.Contains(h, "{") && strings.Contains(h, separator) {
if abs, err := filepath.Abs(h); err == nil {
fsrv.Hide[i] = abs
}
}
}
return nil
}
@ -225,7 +248,7 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
}
}
w.WriteHeader(statusCode)
if r.Method != "HEAD" {
if r.Method != http.MethodHead {
io.Copy(w, file)
}
return nil
@ -273,12 +296,12 @@ func mapDirOpenError(originalErr error, name string) error {
return originalErr
}
parts := strings.Split(name, string(filepath.Separator))
parts := strings.Split(name, separator)
for i := range parts {
if parts[i] == "" {
continue
}
fi, err := os.Stat(strings.Join(parts[:i+1], string(filepath.Separator)))
fi, err := os.Stat(strings.Join(parts[:i+1], separator))
if err != nil {
return originalErr
}
@ -290,12 +313,19 @@ func mapDirOpenError(originalErr error, name string) error {
return originalErr
}
// transformHidePaths performs replacements for all the elements of
// fsrv.Hide and returns a new list of the transformed values.
// transformHidePaths performs replacements for all the elements of fsrv.Hide and
// makes them absolute paths (if they contain a path separator), then returns a
// new list of the transformed values.
func (fsrv *FileServer) transformHidePaths(repl *caddy.Replacer) []string {
hide := make([]string, len(fsrv.Hide))
for i := range fsrv.Hide {
hide[i] = repl.ReplaceAll(fsrv.Hide[i], "")
if strings.Contains(hide[i], separator) {
abs, err := filepath.Abs(hide[i])
if err == nil {
hide[i] = abs
}
}
}
return hide
}
@ -330,40 +360,50 @@ func sanitizedPathJoin(root, reqPath string) string {
// if the length is 1, then it's a path to the root,
// and that should return ".", so we don't append the separator.
if strings.HasSuffix(reqPath, "/") && len(reqPath) > 1 {
path += string(filepath.Separator)
path += separator
}
return path
}
// fileHidden returns true if filename is hidden
// according to the hide list.
// fileHidden returns true if filename is hidden according to the hide list.
// filename must be a relative or absolute file system path, not a request
// URI path. It is expected that all the paths in the hide list are absolute
// paths or are singular filenames (without a path separator).
func fileHidden(filename string, hide []string) bool {
sep := string(filepath.Separator)
if len(hide) == 0 {
return false
}
// all path comparisons use the complete absolute path if possible
filenameAbs, err := filepath.Abs(filename)
if err == nil {
filename = filenameAbs
}
var components []string
for _, h := range hide {
if !strings.Contains(h, sep) {
if !strings.Contains(h, separator) {
// if there is no separator in h, then we assume the user
// wants to hide any files or folders that match that
// name; thus we have to compare against each component
// of the filename, e.g. hiding "bar" would hide "/bar"
// as well as "/foo/bar/baz" but not "/barstool".
if len(components) == 0 {
components = strings.Split(filename, sep)
components = strings.Split(filename, separator)
}
for _, c := range components {
if c == h {
if hidden, _ := filepath.Match(h, c); hidden {
return true
}
}
} else if strings.HasPrefix(filename, h) {
// otherwise, if there is a separator in h, and
// filename is exactly prefixed with h, then we
// can do a prefix match so that "/foo" matches
// "/foo/bar" but not "/foobar".
// if there is a separator in h, and filename is exactly
// prefixed with h, then we can do a prefix match so that
// "/foo" matches "/foo/bar" but not "/foobar".
withoutPrefix := strings.TrimPrefix(filename, h)
if strings.HasPrefix(withoutPrefix, sep) {
if strings.HasPrefix(withoutPrefix, separator) {
return true
}
}
@ -414,7 +454,10 @@ var bufPool = sync.Pool{
},
}
const minBackoff, maxBackoff = 2, 5
const (
minBackoff, maxBackoff = 2, 5
separator = string(filepath.Separator)
)
// Interface guards
var (

View file

@ -17,6 +17,8 @@ package fileserver
import (
"net/url"
"path/filepath"
"runtime"
"strings"
"testing"
)
@ -44,7 +46,7 @@ func TestSanitizedPathJoin(t *testing.T) {
},
{
inputPath: "/foo/",
expect: "foo" + string(filepath.Separator),
expect: "foo" + separator,
},
{
inputPath: "/foo/bar",
@ -77,7 +79,7 @@ func TestSanitizedPathJoin(t *testing.T) {
{
inputRoot: "/a/b",
inputPath: "/%2e%2e%2f%2e%2e%2f",
expect: filepath.Join("/", "a", "b") + string(filepath.Separator),
expect: filepath.Join("/", "a", "b") + separator,
},
{
inputRoot: "C:\\www",
@ -154,6 +156,26 @@ func TestFileHidden(t *testing.T) {
inputPath: "/foo/asdf/bar",
expect: true,
},
{
inputHide: []string{"*.txt"},
inputPath: "/foo/bar.txt",
expect: true,
},
{
inputHide: []string{"/foo/bar/*.txt"},
inputPath: "/foo/bar/baz.txt",
expect: true,
},
{
inputHide: []string{"/foo/bar/*.txt"},
inputPath: "/foo/bar.txt",
expect: false,
},
{
inputHide: []string{"/foo/bar/*.txt"},
inputPath: "/foo/bar/index.html",
expect: false,
},
{
inputHide: []string{"/foo"},
inputPath: "/foo",
@ -164,17 +186,29 @@ func TestFileHidden(t *testing.T) {
inputPath: "/foobar",
expect: false,
},
{
inputHide: []string{"first", "second"},
inputPath: "/second",
expect: true,
},
} {
// for Windows' sake
tc.inputPath = filepath.FromSlash(tc.inputPath)
for i := range tc.inputHide {
tc.inputHide[i] = filepath.FromSlash(tc.inputHide[i])
if runtime.GOOS == "windows" {
if strings.HasPrefix(tc.inputPath, "/") {
tc.inputPath, _ = filepath.Abs(tc.inputPath)
}
tc.inputPath = filepath.FromSlash(tc.inputPath)
for i := range tc.inputHide {
if strings.HasPrefix(tc.inputHide[i], "/") {
tc.inputHide[i], _ = filepath.Abs(tc.inputHide[i])
}
tc.inputHide[i] = filepath.FromSlash(tc.inputHide[i])
}
}
actual := fileHidden(tc.inputPath, tc.inputHide)
if actual != tc.expect {
t.Errorf("Test %d: Is %s hidden in %v? Got %t but expected %t",
i, tc.inputPath, tc.inputHide, actual, tc.expect)
t.Errorf("Test %d: Does %v hide %s? Got %t but expected %t",
i, tc.inputHide, tc.inputPath, actual, tc.expect)
}
}
}