Default error handler; rename StaticFiles -> FileServer

8 files changed, 118 insertions, 76 deletions

diff --git a/modules/caddyhttp/staticfiles/browse.go b/modules/caddyhttp/fileserver/browse.go
index 2bb130f..ed5ad42 100644
--- a/modules/caddyhttp/staticfiles/browse.go
+++ b/modules/caddyhttp/fileserver/browse.go
@@ -1,4 +1,4 @@
-package staticfiles
+package fileserver
 
 import (
 	"bytes"
@@ -20,8 +20,8 @@ type Browse struct {
 	template *template.Template
 }
 
-func (sf *StaticFiles) serveBrowse(dirPath string, w http.ResponseWriter, r *http.Request) error {
-	dir, err := sf.openFile(dirPath, w)
+func (fsrv *FileServer) serveBrowse(dirPath string, w http.ResponseWriter, r *http.Request) error {
+	dir, err := fsrv.openFile(dirPath, w)
 	if err != nil {
 		return err
 	}
@@ -29,7 +29,7 @@ func (sf *StaticFiles) serveBrowse(dirPath string, w http.ResponseWriter, r *htt
 
 	repl := r.Context().Value(caddy2.ReplacerCtxKey).(caddy2.Replacer)
 
-	listing, err := sf.loadDirectoryContents(dir, r.URL.Path, repl)
+	listing, err := fsrv.loadDirectoryContents(dir, r.URL.Path, repl)
 	switch {
 	case os.IsPermission(err):
 		return caddyhttp.Error(http.StatusForbidden, err)
@@ -39,18 +39,18 @@ func (sf *StaticFiles) serveBrowse(dirPath string, w http.ResponseWriter, r *htt
 		return caddyhttp.Error(http.StatusInternalServerError, err)
 	}
 
-	sf.browseApplyQueryParams(w, r, &listing)
+	fsrv.browseApplyQueryParams(w, r, &listing)
 
 	// write response as either JSON or HTML
 	var buf *bytes.Buffer
 	acceptHeader := strings.ToLower(strings.Join(r.Header["Accept"], ","))
 	if strings.Contains(acceptHeader, "application/json") {
-		if buf, err = sf.browseWriteJSON(listing); err != nil {
+		if buf, err = fsrv.browseWriteJSON(listing); err != nil {
 			return caddyhttp.Error(http.StatusInternalServerError, err)
 		}
 		w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	} else {
-		if buf, err = sf.browseWriteHTML(listing); err != nil {
+		if buf, err = fsrv.browseWriteHTML(listing); err != nil {
 			return caddyhttp.Error(http.StatusInternalServerError, err)
 		}
 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
@@ -75,7 +75,7 @@ func (sf *StaticFiles) serveBrowse(dirPath string, w http.ResponseWriter, r *htt
 	// return b.ServeListing(w, r, requestedFilepath, bc)
 }
 
-func (sf *StaticFiles) loadDirectoryContents(dir *os.File, urlPath string, repl caddy2.Replacer) (browseListing, error) {
+func (fsrv *FileServer) loadDirectoryContents(dir *os.File, urlPath string, repl caddy2.Replacer) (browseListing, error) {
 	files, err := dir.Readdir(-1)
 	if err != nil {
 		return browseListing{}, err
@@ -83,14 +83,14 @@ func (sf *StaticFiles) loadDirectoryContents(dir *os.File, urlPath string, repl
 
 	// determine if user can browse up another folder
 	curPathDir := path.Dir(strings.TrimSuffix(urlPath, "/"))
-	canGoUp := strings.HasPrefix(curPathDir, sf.Root)
+	canGoUp := strings.HasPrefix(curPathDir, fsrv.Root)
 
-	return sf.directoryListing(files, canGoUp, urlPath, repl), nil
+	return fsrv.directoryListing(files, canGoUp, urlPath, repl), nil
 }
 
 // browseApplyQueryParams applies query parameters to the listing.
 // It mutates the listing and may set cookies.
-func (sf *StaticFiles) browseApplyQueryParams(w http.ResponseWriter, r *http.Request, listing *browseListing) {
+func (fsrv *FileServer) browseApplyQueryParams(w http.ResponseWriter, r *http.Request, listing *browseListing) {
 	sortParam := r.URL.Query().Get("sort")
 	orderParam := r.URL.Query().Get("order")
 	limitParam := r.URL.Query().Get("limit")
@@ -121,15 +121,15 @@ func (sf *StaticFiles) browseApplyQueryParams(w http.ResponseWriter, r *http.Req
 	listing.applySortAndLimit(sortParam, orderParam, limitParam)
 }
 
-func (sf *StaticFiles) browseWriteJSON(listing browseListing) (*bytes.Buffer, error) {
+func (fsrv *FileServer) browseWriteJSON(listing browseListing) (*bytes.Buffer, error) {
 	buf := new(bytes.Buffer)
 	err := json.NewEncoder(buf).Encode(listing.Items)
 	return buf, err
 }
 
-func (sf *StaticFiles) browseWriteHTML(listing browseListing) (*bytes.Buffer, error) {
+func (fsrv *FileServer) browseWriteHTML(listing browseListing) (*bytes.Buffer, error) {
 	buf := new(bytes.Buffer)
-	err := sf.Browse.template.Execute(buf, listing)
+	err := fsrv.Browse.template.Execute(buf, listing)
 	return buf, err
 }
 
diff --git a/modules/caddyhttp/staticfiles/browselisting.go b/modules/caddyhttp/fileserver/browselisting.go
index 11e6b9c..e17b7ae 100644
--- a/modules/caddyhttp/staticfiles/browselisting.go
+++ b/modules/caddyhttp/fileserver/browselisting.go
@@ -1,4 +1,4 @@
-package staticfiles
+package fileserver
 
 import (
 	"net/url"
@@ -13,8 +13,8 @@ import (
 	"github.com/dustin/go-humanize"
 )
 
-func (sf *StaticFiles) directoryListing(files []os.FileInfo, canGoUp bool, urlPath string, repl caddy2.Replacer) browseListing {
-	filesToHide := sf.transformHidePaths(repl)
+func (fsrv *FileServer) directoryListing(files []os.FileInfo, canGoUp bool, urlPath string, repl caddy2.Replacer) browseListing {
+	filesToHide := fsrv.transformHidePaths(repl)
 
 	var (
 		fileInfos           []fileInfo
diff --git a/modules/caddyhttp/staticfiles/browsetpl.go b/modules/caddyhttp/fileserver/browsetpl.go
index ff2a1e1..aebf146 100644
--- a/modules/caddyhttp/staticfiles/browsetpl.go
+++ b/modules/caddyhttp/fileserver/browsetpl.go
@@ -1,4 +1,4 @@
-package staticfiles
+package fileserver
 
 const defaultBrowseTemplate = `<!DOCTYPE html>
 <html>
diff --git a/modules/caddyhttp/staticfiles/matcher.go b/modules/caddyhttp/fileserver/matcher.go
index 9ce3f4c..fd994d0 100644
--- a/modules/caddyhttp/staticfiles/matcher.go
+++ b/modules/caddyhttp/fileserver/matcher.go
@@ -1,4 +1,4 @@
-package staticfiles
+package fileserver
 
 import (
 	"net/http"
diff --git a/modules/caddyhttp/staticfiles/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go
index f9fd8d2..e859abe 100644
--- a/modules/caddyhttp/staticfiles/staticfiles.go
+++ b/modules/caddyhttp/fileserver/staticfiles.go
@@ -1,4 +1,4 @@
-package staticfiles
+package fileserver
 
 import (
 	"fmt"
@@ -20,13 +20,13 @@ func init() {
 	weakrand.Seed(time.Now().UnixNano())
 
 	caddy2.RegisterModule(caddy2.Module{
-		Name: "http.responders.static_files",
-		New:  func() (interface{}, error) { return new(StaticFiles), nil },
+		Name: "http.responders.file_server",
+		New:  func() (interface{}, error) { return new(FileServer), nil },
 	})
 }
 
-// StaticFiles implements a static file server responder for Caddy.
-type StaticFiles struct {
+// FileServer implements a static file server responder for Caddy.
+type FileServer struct {
 	Root            string              `json:"root"` // default is current directory
 	Hide            []string            `json:"hide"`
 	IndexNames      []string            `json:"index_names"`
@@ -40,23 +40,23 @@ type StaticFiles struct {
 }
 
 // Provision sets up the static files responder.
-func (sf *StaticFiles) Provision(ctx caddy2.Context) error {
-	if sf.Fallback != nil {
-		err := sf.Fallback.Provision(ctx)
+func (fsrv *FileServer) Provision(ctx caddy2.Context) error {
+	if fsrv.Fallback != nil {
+		err := fsrv.Fallback.Provision(ctx)
 		if err != nil {
 			return fmt.Errorf("setting up fallback routes: %v", err)
 		}
 	}
 
-	if sf.IndexNames == nil {
-		sf.IndexNames = defaultIndexNames
+	if fsrv.IndexNames == nil {
+		fsrv.IndexNames = defaultIndexNames
 	}
 
-	if sf.Browse != nil {
+	if fsrv.Browse != nil {
 		var tpl *template.Template
 		var err error
-		if sf.Browse.TemplateFile != "" {
-			tpl, err = template.ParseFiles(sf.Browse.TemplateFile)
+		if fsrv.Browse.TemplateFile != "" {
+			tpl, err = template.ParseFiles(fsrv.Browse.TemplateFile)
 			if err != nil {
 				return fmt.Errorf("parsing browse template file: %v", err)
 			}
@@ -66,7 +66,7 @@ func (sf *StaticFiles) Provision(ctx caddy2.Context) error {
 				return fmt.Errorf("parsing default browse template: %v", err)
 			}
 		}
-		sf.Browse.template = tpl
+		fsrv.Browse.template = tpl
 	}
 
 	return nil
@@ -80,29 +80,31 @@ const (
 )
 
 // Validate ensures that sf has a valid configuration.
-func (sf *StaticFiles) Validate() error {
-	switch sf.SelectionPolicy {
+func (fsrv *FileServer) Validate() error {
+	switch fsrv.SelectionPolicy {
 	case "",
 		selectionPolicyFirstExisting,
 		selectionPolicyLargestSize,
 		selectionPolicySmallestSize,
 		selectionPolicyRecentlyMod:
 	default:
-		return fmt.Errorf("unknown selection policy %s", sf.SelectionPolicy)
+		return fmt.Errorf("unknown selection policy %s", fsrv.SelectionPolicy)
 	}
 	return nil
 }
 
-func (sf *StaticFiles) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
+func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
 	repl := r.Context().Value(caddy2.ReplacerCtxKey).(caddy2.Replacer)
 
+	filesToHide := fsrv.transformHidePaths(repl)
+
 	// map the request to a filename
 	pathBefore := r.URL.Path
-	filename := sf.selectFile(r, repl)
+	filename := fsrv.selectFile(r, repl, filesToHide)
 	if filename == "" {
 		// no files worked, so resort to fallback
-		if sf.Fallback != nil {
-			fallback := sf.Fallback.BuildCompositeRoute(w, r)
+		if fsrv.Fallback != nil {
+			fallback := fsrv.Fallback.BuildCompositeRoute(w, r)
 			return fallback.ServeHTTP(w, r)
 		}
 		return caddyhttp.Error(http.StatusNotFound, nil)
@@ -111,7 +113,7 @@ func (sf *StaticFiles) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
 	// if the ultimate destination has changed, submit
 	// this request for a rehandling (internal redirect)
 	// if configured to do so
-	if r.URL.Path != pathBefore && sf.Rehandle {
+	if r.URL.Path != pathBefore && fsrv.Rehandle {
 		return caddyhttp.ErrRehandle
 	}
 
@@ -130,10 +132,8 @@ func (sf *StaticFiles) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
 
 	// if the request mapped to a directory, see if
 	// there is an index file we can serve
-	if info.IsDir() && len(sf.IndexNames) > 0 {
-		filesToHide := sf.transformHidePaths(repl)
-
-		for _, indexPage := range sf.IndexNames {
+	if info.IsDir() && len(fsrv.IndexNames) > 0 {
+		for _, indexPage := range fsrv.IndexNames {
 			indexPath := sanitizedPathJoin(filename, indexPage)
 			if fileHidden(indexPath, filesToHide) {
 				// pretend this file doesn't exist
@@ -149,7 +149,7 @@ func (sf *StaticFiles) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
 			// so rewrite the request path and, if
 			// configured, do an internal redirect
 			r.URL.Path = path.Join(r.URL.Path, indexPage)
-			if sf.Rehandle {
+			if fsrv.Rehandle {
 				return caddyhttp.ErrRehandle
 			}
 
@@ -162,16 +162,22 @@ func (sf *StaticFiles) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
 	// if still referencing a directory, delegate
 	// to browse or return an error
 	if info.IsDir() {
-		if sf.Browse != nil {
-			return sf.serveBrowse(filename, w, r)
+		if fsrv.Browse != nil && !fileHidden(filename, filesToHide) {
+			return fsrv.serveBrowse(filename, w, r)
 		}
 		return caddyhttp.Error(http.StatusNotFound, nil)
 	}
 
 	// TODO: content negotiation (brotli sidecar files, etc...)
 
+	// one last check to ensure the file isn't hidden (we might
+	// have changed the filename from when we last checked)
+	if fileHidden(filename, filesToHide) {
+		return caddyhttp.Error(http.StatusNotFound, nil)
+	}
+
 	// open the file
-	file, err := sf.openFile(filename, w)
+	file, err := fsrv.openFile(filename, w)
 	if err != nil {
 		return err
 	}
@@ -179,6 +185,8 @@ func (sf *StaticFiles) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
 
 	// TODO: Etag
 
+	// TODO: Disable content-type sniffing by setting a content-type...
+
 	// let the standard library do what it does best; note, however,
 	// that errors generated by ServeContent are written immediately
 	// to the response, so we cannot handle them (but errors here
@@ -192,7 +200,7 @@ func (sf *StaticFiles) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
 // the response is configured to inform the client how to best handle it
 // and a well-described handler error is returned (do not wrap the
 // returned error value).
-func (sf *StaticFiles) openFile(filename string, w http.ResponseWriter) (*os.File, error) {
+func (fsrv *FileServer) openFile(filename string, w http.ResponseWriter) (*os.File, error) {
 	file, err := os.Open(filename)
 	if err != nil {
 		err = mapDirOpenError(err, filename)
@@ -239,11 +247,11 @@ func mapDirOpenError(originalErr error, name string) error {
 }
 
 // transformHidePaths performs replacements for all the elements of
-// sf.Hide and returns a new list of the transformed values.
-func (sf *StaticFiles) transformHidePaths(repl caddy2.Replacer) []string {
-	hide := make([]string, len(sf.Hide))
-	for i := range sf.Hide {
-		hide[i] = repl.ReplaceAll(sf.Hide[i], "")
+// fsrv.Hide and returns a new list of the transformed values.
+func (fsrv *FileServer) transformHidePaths(repl caddy2.Replacer) []string {
+	hide := make([]string, len(fsrv.Hide))
+	for i := range fsrv.Hide {
+		hide[i] = repl.ReplaceAll(fsrv.Hide[i], "")
 	}
 	return hide
 }
@@ -251,7 +259,8 @@ func (sf *StaticFiles) transformHidePaths(repl caddy2.Replacer) []string {
 // sanitizedPathJoin performs filepath.Join(root, reqPath) that
 // is safe against directory traversal attacks. It uses logic
 // similar to that in the Go standard library, specifically
-// in the implementation of http.Dir.
+// in the implementation of http.Dir. The root is assumed to
+// be a trusted path, but reqPath is not.
 func sanitizedPathJoin(root, reqPath string) string {
 	// TODO: Caddy 1 uses this:
 	// prevent absolute path access on Windows, e.g. http://localhost:5000/C:\Windows\notepad.exe
@@ -276,17 +285,17 @@ func sanitizedPathJoin(root, reqPath string) string {
 // by default) to map the request r to a filename. The full path to
 // the file is returned if one is found; otherwise, an empty string
 // is returned.
-func (sf *StaticFiles) selectFile(r *http.Request, repl caddy2.Replacer) string {
-	root := repl.ReplaceAll(sf.Root, "")
+func (fsrv *FileServer) selectFile(r *http.Request, repl caddy2.Replacer, filesToHide []string) string {
+	root := repl.ReplaceAll(fsrv.Root, "")
 
-	if sf.Files == nil {
+	if fsrv.Files == nil {
 		return sanitizedPathJoin(root, r.URL.Path)
 	}
 
-	switch sf.SelectionPolicy {
+	switch fsrv.SelectionPolicy {
 	case "", selectionPolicyFirstExisting:
-		filesToHide := sf.transformHidePaths(repl)
-		for _, f := range sf.Files {
+		filesToHide := fsrv.transformHidePaths(repl)
+		for _, f := range fsrv.Files {
 			suffix := repl.ReplaceAll(f, "")
 			fullpath := sanitizedPathJoin(root, suffix)
 			if !fileHidden(fullpath, filesToHide) && fileExists(fullpath) {
@@ -299,9 +308,12 @@ func (sf *StaticFiles) selectFile(r *http.Request, repl caddy2.Replacer) string
 		var largestSize int64
 		var largestFilename string
 		var largestSuffix string
-		for _, f := range sf.Files {
+		for _, f := range fsrv.Files {
 			suffix := repl.ReplaceAll(f, "")
 			fullpath := sanitizedPathJoin(root, suffix)
+			if fileHidden(fullpath, filesToHide) {
+				continue
+			}
 			info, err := os.Stat(fullpath)
 			if err == nil && info.Size() > largestSize {
 				largestSize = info.Size()
@@ -316,9 +328,12 @@ func (sf *StaticFiles) selectFile(r *http.Request, repl caddy2.Replacer) string
 		var smallestSize int64
 		var smallestFilename string
 		var smallestSuffix string
-		for _, f := range sf.Files {
+		for _, f := range fsrv.Files {
 			suffix := repl.ReplaceAll(f, "")
 			fullpath := sanitizedPathJoin(root, suffix)
+			if fileHidden(fullpath, filesToHide) {
+				continue
+			}
 			info, err := os.Stat(fullpath)
 			if err == nil && (smallestSize == 0 || info.Size() < smallestSize) {
 				smallestSize = info.Size()
@@ -333,9 +348,12 @@ func (sf *StaticFiles) selectFile(r *http.Request, repl caddy2.Replacer) string
 		var recentDate time.Time
 		var recentFilename string
 		var recentSuffix string
-		for _, f := range sf.Files {
+		for _, f := range fsrv.Files {
 			suffix := repl.ReplaceAll(f, "")
 			fullpath := sanitizedPathJoin(root, suffix)
+			if fileHidden(fullpath, filesToHide) {
+				continue
+			}
 			info, err := os.Stat(fullpath)
 			if err == nil &&
 				(recentDate.IsZero() || info.ModTime().After(recentDate)) {
@@ -395,4 +413,4 @@ var defaultIndexNames = []string{"index.html"}
 const minBackoff, maxBackoff = 2, 5
 
 // Interface guard
-var _ caddyhttp.Handler = (*StaticFiles)(nil)
+var _ caddyhttp.Handler = (*FileServer)(nil)
diff --git a/modules/caddyhttp/staticfiles/staticfiles_test.go b/modules/caddyhttp/fileserver/staticfiles_test.go
index f2e1c89..2a99c71 100644
--- a/modules/caddyhttp/staticfiles/staticfiles_test.go
+++ b/modules/caddyhttp/fileserver/staticfiles_test.go
@@ -1,4 +1,4 @@
-package staticfiles
+package fileserver
 
 import (
 	"net/url"
@@ -60,6 +60,7 @@ func TestSanitizedPathJoin(t *testing.T) {
 			inputPath: "/%2e%2e%2f%2e%2e%2f",
 			expect:    "/a/b",
 		},
+		// TODO: test windows paths... on windows... sigh.
 	} {
 		// we don't *need* to use an actual parsed URL, but it
 		// adds some authenticity to the tests since real-world
@@ -76,3 +77,5 @@ func TestSanitizedPathJoin(t *testing.T) {
 		}
 	}
 }
+
+// TODO: test fileHidden
diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go
index 5ab7693..fbbdae4 100644
--- a/modules/caddyhttp/server.go
+++ b/modules/caddyhttp/server.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log"
 	"net/http"
+	"strconv"
 
 	"bitbucket.org/lightcodelabs/caddy2"
 	"bitbucket.org/lightcodelabs/caddy2/modules/caddytls"
@@ -41,15 +42,27 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	stack := s.Routes.BuildCompositeRoute(w, r)
 	err := s.executeCompositeRoute(w, r, stack)
 	if err != nil {
-		// add the error value to the request context so
-		// it can be accessed by error handlers
+		// add the raw error value to the request context
+		// so it can be accessed by error handlers
 		c := context.WithValue(r.Context(), ErrorCtxKey, err)
 		r = r.WithContext(c)
-		// TODO: add error values to Replacer
+
+		// add error values to the replacer
+		repl.Set("http.error", err.Error())
+		if handlerErr, ok := err.(HandlerError); ok {
+			repl.Set("http.error.status_code", strconv.Itoa(handlerErr.StatusCode))
+			repl.Set("http.error.status_text", http.StatusText(handlerErr.StatusCode))
+			repl.Set("http.error.message", handlerErr.Message)
+			repl.Set("http.error.trace", handlerErr.Trace)
+			repl.Set("http.error.id", handlerErr.ID)
+		}
 
 		if len(s.Errors.Routes) == 0 {
-			// TODO: implement a default error handler?
-			log.Printf("[ERROR] %s", err)
+			// TODO: polish the default error handling
+			log.Printf("[ERROR] Handler: %s", err)
+			if handlerErr, ok := err.(HandlerError); ok {
+				w.WriteHeader(handlerErr.StatusCode)
+			}
 		} else {
 			errStack := s.Errors.Routes.BuildCompositeRoute(w, r)
 			err := s.executeCompositeRoute(w, r, errStack)
diff --git a/modules/caddyhttp/staticresp.go b/modules/caddyhttp/staticresp.go
index 69ec45b..e07084a 100644
--- a/modules/caddyhttp/staticresp.go
+++ b/modules/caddyhttp/staticresp.go
@@ -3,6 +3,7 @@ package caddyhttp
 import (
 	"fmt"
 	"net/http"
+	"strconv"
 
 	"bitbucket.org/lightcodelabs/caddy2"
 )
@@ -16,10 +17,11 @@ func init() {
 
 // Static implements a simple responder for static responses.
 type Static struct {
-	StatusCode int         `json:"status_code"`
-	Headers    http.Header `json:"headers"`
-	Body       string      `json:"body"`
-	Close      bool        `json:"close"`
+	StatusCode    int         `json:"status_code"`
+	StatusCodeStr string      `json:"status_code_str"`
+	Headers       http.Header `json:"headers"`
+	Body          string      `json:"body"`
+	Close         bool        `json:"close"`
 }
 
 func (s Static) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
@@ -39,6 +41,12 @@ func (s Static) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
 
 	// write the headers with a status code
 	statusCode := s.StatusCode
+	if statusCode == 0 && s.StatusCodeStr != "" {
+		intVal, err := strconv.Atoi(repl.ReplaceAll(s.StatusCodeStr, ""))
+		if err == nil {
+			statusCode = intVal
+		}
+	}
 	if statusCode == 0 {
 		statusCode = http.StatusOK
 	}