From 8c843ceefd781a407475e4756a2b6b5cec25a2da Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Wed, 16 Sep 2015 21:30:56 -0600 Subject: [PATCH 01/15] middleware: Add StripExt to Context type for stripping extensions from paths --- middleware/context.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/middleware/context.go b/middleware/context.go index 8820c4e5f..ade03e112 100644 --- a/middleware/context.go +++ b/middleware/context.go @@ -131,6 +131,19 @@ func (c Context) Truncate(input string, length int) string { return input } +// StripExt returns the input string without the extension, +// which is the suffix starting with the final '.' character +// but not before the final path separator ('/') character. +// If there is no extension, the whole input is returned. +func (c Context) StripExt(path string) string { + for i := len(path) - 1; i >= 0 && path[i] != '/'; i-- { + if path[i] == '.' { + return path[:i] + } + } + return path +} + // Replace replaces instances of find in input with replacement. func (c Context) Replace(input, find, replacement string) string { return strings.Replace(input, find, replacement, -1) From 840bc505f608e92f4e33e0a79dd8217a3d1e183b Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Wed, 16 Sep 2015 21:31:45 -0600 Subject: [PATCH 02/15] This is a pretty cool change --- dist/CHANGES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/dist/CHANGES.txt b/dist/CHANGES.txt index f21eeda92..eb611e00e 100644 --- a/dist/CHANGES.txt +++ b/dist/CHANGES.txt @@ -3,6 +3,7 @@ CHANGES - basicauth: Support for legacy htpasswd files - browse: JSON response with file listing given Accept header +- core: Caddyfile as command line argument 0.7.5 (August 5, 2015) From 30b19190dc4e1a2d82ca89c89cec1cd696f5c815 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Thu, 17 Sep 2015 20:33:39 +0100 Subject: [PATCH 03/15] add ignoreIndexes option to browse --- middleware/browse/browse.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index 8036896f0..c4ff6ef59 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -23,9 +23,10 @@ import ( // Browse is an http.Handler that can show a file listing when // directories in the given paths are specified. type Browse struct { - Next middleware.Handler - Root string - Configs []Config + Next middleware.Handler + Root string + Configs []Config + IgnoreIndexes bool } // Config is a configuration for browsing in a particular path. @@ -142,16 +143,18 @@ var IndexPages = []string{ "default.txt", } -func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root string) (Listing, error) { +func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root string, ignoreIndexes bool) (Listing, error) { var fileinfos []FileInfo var urlPath = r.URL.Path for _, f := range files { name := f.Name() // Directory is not browsable if it contains index file - for _, indexName := range IndexPages { - if name == indexName { - return Listing{}, errors.New("Directory contains index file, not browsable!") + if !ignoreIndexes { + for _, indexName := range IndexPages { + if name == indexName { + return Listing{}, errors.New("Directory contains index file, not browsable!") + } } } @@ -234,7 +237,7 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { } } // Assemble listing of directory contents - listing, err := directoryListing(files, r, canGoUp, b.Root) + listing, err := directoryListing(files, r, canGoUp, b.Root, b.IgnoreIndexes) if err != nil { // directory isn't browsable continue } From 4c642e9d3c8ae382c32421462e9f163110e8f621 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Thu, 17 Sep 2015 20:37:49 +0100 Subject: [PATCH 04/15] browse IgnoreIndexes option --- config/setup/browse.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/config/setup/browse.go b/config/setup/browse.go index a98ec313d..fe2f9f9e8 100644 --- a/config/setup/browse.go +++ b/config/setup/browse.go @@ -17,8 +17,9 @@ func Browse(c *Controller) (middleware.Middleware, error) { } browse := browse.Browse{ - Root: c.Root, - Configs: configs, + Root: c.Root, + Configs: configs, + IgnoreIndexes: false, } return func(next middleware.Handler) middleware.Handler { From 9e2bef146e9d8a43c2025bc9d04df400330874d5 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 17 Sep 2015 16:23:30 -0600 Subject: [PATCH 05/15] middleware: Added StripHTML to Context type --- middleware/context.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/middleware/context.go b/middleware/context.go index ade03e112..6c45d0337 100644 --- a/middleware/context.go +++ b/middleware/context.go @@ -131,6 +131,40 @@ func (c Context) Truncate(input string, length int) string { return input } +// StripHTML returns s without HTML tags. It is fairly naive +// but works with most valid HTML inputs. +func (c Context) StripHTML(s string) string { + var buf bytes.Buffer + var inTag, inQuotes bool + var tagStart int + for i, ch := range s { + if inTag { + if ch == '>' && !inQuotes { + inTag = false + } else if ch == '<' && !inQuotes { + // false start + buf.WriteString(s[tagStart:i]) + tagStart = i + } else if ch == '"' { + inQuotes = !inQuotes + } + continue + } + if ch == '<' { + inTag = true + tagStart = i + continue + } + buf.WriteRune(ch) + } + if inTag { + // false start + buf.WriteString(s[tagStart:]) + inTag = false + } + return buf.String() +} + // StripExt returns the input string without the extension, // which is the suffix starting with the final '.' character // but not before the final path separator ('/') character. From 8120e57850b3f92c8680d1184425247e634e2eb0 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Fri, 18 Sep 2015 08:52:12 +0100 Subject: [PATCH 06/15] add user defined variables into browse template --- middleware/browse/browse.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index c4ff6ef59..dbbe25fc5 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -32,6 +32,7 @@ type Browse struct { // Config is a configuration for browsing in a particular path. type Config struct { PathScope string + Variables interface{} Template *template.Template } @@ -55,6 +56,9 @@ type Listing struct { // And which order Order string + // User defined costum variables + User interface{} + middleware.Context } @@ -143,7 +147,7 @@ var IndexPages = []string{ "default.txt", } -func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root string, ignoreIndexes bool) (Listing, error) { +func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root string, ignoreIndexes bool, vars interface{}) (Listing, error) { var fileinfos []FileInfo var urlPath = r.URL.Path for _, f := range files { @@ -184,6 +188,7 @@ func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root s Req: r, URL: r.URL, }, + User: vars, }, nil } @@ -237,7 +242,7 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { } } // Assemble listing of directory contents - listing, err := directoryListing(files, r, canGoUp, b.Root, b.IgnoreIndexes) + listing, err := directoryListing(files, r, canGoUp, b.Root, b.IgnoreIndexes, bc.Variables) if err != nil { // directory isn't browsable continue } From ee893325c44e52184e2dcb240cf9ff15ff61028a Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sat, 19 Sep 2015 11:24:44 -0600 Subject: [PATCH 07/15] Update change list --- dist/CHANGES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/dist/CHANGES.txt b/dist/CHANGES.txt index eb611e00e..c01c2ddcd 100644 --- a/dist/CHANGES.txt +++ b/dist/CHANGES.txt @@ -4,6 +4,7 @@ CHANGES - basicauth: Support for legacy htpasswd files - browse: JSON response with file listing given Accept header - core: Caddyfile as command line argument +- templates: Added .StripExt and .StripHTML methods 0.7.5 (August 5, 2015) From 10ab0378332debbf958617fc236fc70c24e1d69c Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sat, 19 Sep 2015 20:34:23 -0600 Subject: [PATCH 08/15] Moved fileServer and browse.IndexPages into middleware package --- .../{controllertest.go => controller_test.go} | 0 dist/CHANGES.txt | 2 ++ middleware/browse/browse.go | 13 ++----- {server => middleware}/fileserver.go | 36 ++++++++++++++----- server/virtualhost.go | 2 +- 5 files changed, 33 insertions(+), 20 deletions(-) rename config/setup/{controllertest.go => controller_test.go} (100%) rename {server => middleware}/fileserver.go (79%) diff --git a/config/setup/controllertest.go b/config/setup/controller_test.go similarity index 100% rename from config/setup/controllertest.go rename to config/setup/controller_test.go diff --git a/dist/CHANGES.txt b/dist/CHANGES.txt index c01c2ddcd..bf4c3c23a 100644 --- a/dist/CHANGES.txt +++ b/dist/CHANGES.txt @@ -4,7 +4,9 @@ CHANGES - basicauth: Support for legacy htpasswd files - browse: JSON response with file listing given Accept header - core: Caddyfile as command line argument +- errors, log: Roll log files after certain size or age - templates: Added .StripExt and .StripHTML methods +- Internal improvements and minor bug fixes 0.7.5 (August 5, 2015) diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index dbbe25fc5..a86c5f022 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -56,7 +56,7 @@ type Listing struct { // And which order Order string - // User defined costum variables + // Optional custom variables for use in browse templates User interface{} middleware.Context @@ -138,15 +138,6 @@ func (fi FileInfo) HumanModTime(format string) string { return fi.ModTime.Format(format) } -var IndexPages = []string{ - "index.html", - "index.htm", - "index.txt", - "default.html", - "default.htm", - "default.txt", -} - func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root string, ignoreIndexes bool, vars interface{}) (Listing, error) { var fileinfos []FileInfo var urlPath = r.URL.Path @@ -155,7 +146,7 @@ func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root s // Directory is not browsable if it contains index file if !ignoreIndexes { - for _, indexName := range IndexPages { + for _, indexName := range middleware.IndexPages { if name == indexName { return Listing{}, errors.New("Directory contains index file, not browsable!") } diff --git a/server/fileserver.go b/middleware/fileserver.go similarity index 79% rename from server/fileserver.go rename to middleware/fileserver.go index 4d12905bb..d8c59b2ea 100644 --- a/server/fileserver.go +++ b/middleware/fileserver.go @@ -1,25 +1,34 @@ -package server +package middleware import ( "net/http" "os" "path" "strings" - - "github.com/mholt/caddy/middleware" - "github.com/mholt/caddy/middleware/browse" ) +// This file contains a standard way for Caddy middleware +// to load files from the file system given a request +// URI and path to site root. Other middleware that load +// files should use these facilities. + +// FileServer implements a production-ready file server +// and is the 'default' handler for all requests to Caddy. +// It simply loads and serves the URI requested. If Caddy is +// run without any extra configuration/directives, this is the +// only middleware handler that runs. It is not in its own +// folder like most other middleware handlers because it does +// not require a directive. It is a special case. +// // FileServer is adapted from the one in net/http by // the Go authors. Significant modifications have been made. // -// -// License: +// Original license: // // Copyright 2009 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -func FileServer(root http.FileSystem, hide []string) middleware.Handler { +func FileServer(root http.FileSystem, hide []string) Handler { return &fileHandler{root: root, hide: hide} } @@ -82,7 +91,7 @@ func (fh *fileHandler) serveFile(w http.ResponseWriter, r *http.Request, name st // use contents of an index file, if present, for directory if d.IsDir() { - for _, indexPage := range browse.IndexPages { + for _, indexPage := range IndexPages { index := strings.TrimSuffix(name, "/") + "/" + indexPage ff, err := fh.root.Open(index) if err == nil { @@ -134,3 +143,14 @@ func redirect(w http.ResponseWriter, r *http.Request, newPath string) { } http.Redirect(w, r, newPath, http.StatusMovedPermanently) } + +// IndexPages is a list of pages that may be understood as +// the "index" files to directories. +var IndexPages = []string{ + "index.html", + "index.htm", + "index.txt", + "default.html", + "default.htm", + "default.txt", +} diff --git a/server/virtualhost.go b/server/virtualhost.go index d4fe9c85e..b0d157971 100644 --- a/server/virtualhost.go +++ b/server/virtualhost.go @@ -20,7 +20,7 @@ type virtualHost struct { // on its config. This method should be called last before // ListenAndServe begins. func (vh *virtualHost) buildStack() error { - vh.fileServer = FileServer(http.Dir(vh.config.Root), []string{vh.config.ConfigFile}) + vh.fileServer = middleware.FileServer(http.Dir(vh.config.Root), []string{vh.config.ConfigFile}) // TODO: We only compile middleware for the "/" scope. // Partial support for multiple location contexts already From 0e039a18684634a01aa3886a77c0a910e9678120 Mon Sep 17 00:00:00 2001 From: Abiola Ibrahim Date: Sun, 20 Sep 2015 08:49:55 +0100 Subject: [PATCH 09/15] Rewrite: Use middleware.Replacer. Bug fix for regexps starting with '/'. --- middleware/replacer.go | 10 ++++++++ middleware/rewrite/rewrite.go | 43 ++++++++--------------------------- 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/middleware/replacer.go b/middleware/replacer.go index df1f6e010..9d34253cf 100644 --- a/middleware/replacer.go +++ b/middleware/replacer.go @@ -3,6 +3,7 @@ package middleware import ( "net" "net/http" + "path" "strconv" "strings" "time" @@ -40,6 +41,7 @@ func NewReplacer(r *http.Request, rr *responseRecorder, emptyValue string) Repla "{host}": r.Host, "{path}": r.URL.Path, "{query}": r.URL.RawQuery, + "{frag}": r.URL.Fragment, "{fragment}": r.URL.Fragment, "{proto}": r.Proto, "{remote}": func() string { @@ -63,6 +65,14 @@ func NewReplacer(r *http.Request, rr *responseRecorder, emptyValue string) Repla "{when}": func() string { return time.Now().Format(timeFormat) }(), + "{file}": func() string { + _, file := path.Split(r.URL.Path) + return file + }(), + "{dir}": func() string { + dir, _ := path.Split(r.URL.Path) + return dir + }(), }, emptyValue: emptyValue, } diff --git a/middleware/rewrite/rewrite.go b/middleware/rewrite/rewrite.go index bf987b938..ed2e9bd9f 100644 --- a/middleware/rewrite/rewrite.go +++ b/middleware/rewrite/rewrite.go @@ -3,9 +3,8 @@ package rewrite import ( - "net/http" - "fmt" + "net/http" "net/url" "path" "path/filepath" @@ -96,15 +95,6 @@ func NewRegexpRule(base, pattern, to string, ext []string) (*RegexpRule, error) }, nil } -// regexpVars are variables that can be used for To (rewrite destination path). -var regexpVars = []string{ - "{path}", - "{query}", - "{file}", - "{dir}", - "{frag}", -} - // Rewrite rewrites the internal location of the current request. func (r *RegexpRule) Rewrite(req *http.Request) bool { rPath := req.URL.Path @@ -119,32 +109,19 @@ func (r *RegexpRule) Rewrite(req *http.Request) bool { return false } + // include trailing slash in regexp if present + start := len(r.Base) + if strings.HasSuffix(r.Base, "/") { + start -= 1 + } + // validate regexp - if !r.MatchString(rPath[len(r.Base):]) { + if !r.MatchString(rPath[start:]) { return false } - to := r.To - - // check variables - for _, v := range regexpVars { - if strings.Contains(r.To, v) { - switch v { - case "{path}": - to = strings.Replace(to, v, req.URL.Path[1:], -1) - case "{query}": - to = strings.Replace(to, v, req.URL.RawQuery, -1) - case "{frag}": - to = strings.Replace(to, v, req.URL.Fragment, -1) - case "{file}": - _, file := path.Split(req.URL.Path) - to = strings.Replace(to, v, file, -1) - case "{dir}": - dir, _ := path.Split(req.URL.Path) - to = path.Clean(strings.Replace(to, v, dir, -1)) - } - } - } + // replace variables + to := path.Clean(middleware.NewReplacer(req, nil, "").Replace(r.To)) // validate resulting path url, err := url.Parse(to) From bdccc51437badbf0a680825948cba24ec4c7567f Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sun, 20 Sep 2015 10:55:16 -0600 Subject: [PATCH 10/15] More consistent error messages --- middleware/errors/errors.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/middleware/errors/errors.go b/middleware/errors/errors.go index 44451ab12..22209e914 100644 --- a/middleware/errors/errors.go +++ b/middleware/errors/errors.go @@ -53,7 +53,8 @@ func (h ErrorHandler) errorPage(w http.ResponseWriter, code int) { errorPage, err := os.Open(pagePath) if err != nil { // An error handling an error... - h.Log.Printf("HTTP %d could not load error page %s: %v", code, pagePath, err) + h.Log.Printf("%s [HTTP %d] could not load error page %s: %v", + time.Now().Format(timeFormat), code, pagePath, err) http.Error(w, defaultBody, code) return } @@ -66,7 +67,8 @@ func (h ErrorHandler) errorPage(w http.ResponseWriter, code int) { if err != nil { // Epic fail... sigh. - h.Log.Printf("HTTP %d could not respond with %s: %v", code, pagePath, err) + h.Log.Printf("%s [HTTP %d] could not respond with %s: %v", + time.Now().Format(timeFormat), code, pagePath, err) http.Error(w, defaultBody, code) } From 7f9fa5730b61064d26e723ff132a18ea79ce4f35 Mon Sep 17 00:00:00 2001 From: Abiola Ibrahim Date: Sun, 20 Sep 2015 18:13:53 +0100 Subject: [PATCH 11/15] Rewrite: Use only `fragment`, remove `frag`. --- middleware/replacer.go | 1 - middleware/rewrite/rewrite_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/middleware/replacer.go b/middleware/replacer.go index 9d34253cf..29c695b77 100644 --- a/middleware/replacer.go +++ b/middleware/replacer.go @@ -41,7 +41,6 @@ func NewReplacer(r *http.Request, rr *responseRecorder, emptyValue string) Repla "{host}": r.Host, "{path}": r.URL.Path, "{query}": r.URL.RawQuery, - "{frag}": r.URL.Fragment, "{fragment}": r.URL.Fragment, "{proto}": r.Proto, "{remote}": func() string { diff --git a/middleware/rewrite/rewrite_test.go b/middleware/rewrite/rewrite_test.go index d9056b431..b4845f106 100644 --- a/middleware/rewrite/rewrite_test.go +++ b/middleware/rewrite/rewrite_test.go @@ -28,7 +28,7 @@ func TestRewrite(t *testing.T) { []string{"/ab/", "ab", "/ab?type=html&{query}", ".html|"}, []string{"/abc/", "ab", "/abc/{file}", ".html|"}, []string{"/abcd/", "ab", "/a/{dir}/{file}", ".html|"}, - []string{"/abcde/", "ab", "/a#{frag}", ".html|"}, + []string{"/abcde/", "ab", "/a#{fragment}", ".html|"}, []string{"/ab/", `.*\.jpg`, "/ajpg", ""}, } From 6cbd3ab096ab53541bccaf7b9de98b042e180af9 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 22 Sep 2015 16:47:39 -0600 Subject: [PATCH 12/15] proxy: 64-bit word alignment for 32-bit systems (fixes #252) --- middleware/proxy/proxy.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/middleware/proxy/proxy.go b/middleware/proxy/proxy.go index 25f2a45dc..9b8fed21f 100644 --- a/middleware/proxy/proxy.go +++ b/middleware/proxy/proxy.go @@ -33,10 +33,9 @@ type UpstreamHostDownFunc func(*UpstreamHost) bool // UpstreamHost represents a single proxy upstream type UpstreamHost struct { - // The hostname of this upstream host - Name string + Conns int64 // must be first field to be 64-bit aligned on 32-bit systems + Name string // hostname of this upstream host ReverseProxy *ReverseProxy - Conns int64 Fails int32 FailTimeout time.Duration Unhealthy bool From 6001c94f30993c834fe8bc0096bb66a760632304 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 24 Sep 2015 11:26:52 -0600 Subject: [PATCH 13/15] errors: Fix test --- middleware/errors/errors_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/middleware/errors/errors_test.go b/middleware/errors/errors_test.go index 7f975bc92..2168e6b72 100644 --- a/middleware/errors/errors_test.go +++ b/middleware/errors/errors_test.go @@ -82,8 +82,8 @@ func TestErrors(t *testing.T) { expectedCode: 0, expectedBody: fmt.Sprintf("%d %s\n", http.StatusForbidden, http.StatusText(http.StatusForbidden)), - expectedLog: fmt.Sprintf("HTTP %d could not load error page %s: %v\n", - http.StatusForbidden, "not_exist_file", notExistErr), + expectedLog: fmt.Sprintf("could not load error page %s: %v\n", + "not_exist_file", notExistErr), expectedErr: nil, }, } From da7562367c8b47f453e2afad8d15a8e0911d2fab Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 24 Sep 2015 14:01:08 -0600 Subject: [PATCH 14/15] errors: Restore http status text in test --- middleware/errors/errors_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/middleware/errors/errors_test.go b/middleware/errors/errors_test.go index 2168e6b72..e2e82cab1 100644 --- a/middleware/errors/errors_test.go +++ b/middleware/errors/errors_test.go @@ -82,8 +82,8 @@ func TestErrors(t *testing.T) { expectedCode: 0, expectedBody: fmt.Sprintf("%d %s\n", http.StatusForbidden, http.StatusText(http.StatusForbidden)), - expectedLog: fmt.Sprintf("could not load error page %s: %v\n", - "not_exist_file", notExistErr), + expectedLog: fmt.Sprintf("[HTTP %d] could not load error page %s: %v\n", + http.StatusForbidden, "not_exist_file", notExistErr), expectedErr: nil, }, } From 4f5a29d6d13e91e052e8a715d571300ec201bb49 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 24 Sep 2015 16:21:28 -0600 Subject: [PATCH 15/15] errors: New 'visible' mode to write stack trace to response Also updated change list and added/improved tests --- config/setup/errors.go | 46 ++++++++++++++++++---------- config/setup/errors_test.go | 31 +++++++++++++++---- dist/CHANGES.txt | 2 ++ middleware/errors/errors.go | 46 +++++++++++++++++++--------- middleware/errors/errors_test.go | 51 ++++++++++++++++++++++++++++---- 5 files changed, 134 insertions(+), 42 deletions(-) diff --git a/config/setup/errors.go b/config/setup/errors.go index ae42f381b..bc131976b 100644 --- a/config/setup/errors.go +++ b/config/setup/errors.go @@ -25,16 +25,24 @@ func Errors(c *Controller) (middleware.Middleware, error) { var err error var writer io.Writer - if handler.LogFile == "stdout" { + switch handler.LogFile { + case "visible": + handler.Debug = true + case "stdout": writer = os.Stdout - } else if handler.LogFile == "stderr" { + case "stderr": writer = os.Stderr - } else if handler.LogFile == "syslog" { + case "syslog": writer, err = gsyslog.NewLogger(gsyslog.LOG_ERR, "LOCAL0", "caddy") if err != nil { return err } - } else if handler.LogFile != "" { + default: + if handler.LogFile == "" { + writer = os.Stderr // default + break + } + var file *os.File file, err = os.OpenFile(handler.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) if err != nil { @@ -80,15 +88,19 @@ func errorsParse(c *Controller) (*errors.ErrorHandler, error) { where := c.Val() if what == "log" { - handler.LogFile = where - if c.NextArg() { - if c.Val() == "{" { - c.IncrNest() - logRoller, err := parseRoller(c) - if err != nil { - return hadBlock, err + if where == "visible" { + handler.Debug = true + } else { + handler.LogFile = where + if c.NextArg() { + if c.Val() == "{" { + c.IncrNest() + logRoller, err := parseRoller(c) + if err != nil { + return hadBlock, err + } + handler.LogRoller = logRoller } - handler.LogRoller = logRoller } } } else { @@ -121,12 +133,14 @@ func errorsParse(c *Controller) (*errors.ErrorHandler, error) { return handler, err } - // Otherwise, the only argument would be an error log file name + // Otherwise, the only argument would be an error log file name or 'visible' if !hadBlock { if c.NextArg() { - handler.LogFile = c.Val() - } else { - handler.LogFile = errors.DefaultLogFilename + if c.Val() == "visible" { + handler.Debug = true + } else { + handler.LogFile = c.Val() + } } } } diff --git a/config/setup/errors_test.go b/config/setup/errors_test.go index f161eaf3c..216c5de85 100644 --- a/config/setup/errors_test.go +++ b/config/setup/errors_test.go @@ -8,9 +8,7 @@ import ( ) func TestErrors(t *testing.T) { - c := NewTestController(`errors`) - mid, err := Errors(c) if err != nil { @@ -28,8 +26,8 @@ func TestErrors(t *testing.T) { t.Fatalf("Expected handler to be type ErrorHandler, got: %#v", handler) } - if myHandler.LogFile != errors.DefaultLogFilename { - t.Errorf("Expected %s as the default LogFile", errors.DefaultLogFilename) + if myHandler.LogFile != "" { + t.Errorf("Expected '%s' as the default LogFile", "") } if myHandler.LogRoller != nil { t.Errorf("Expected LogRoller to be nil, got: %v", *myHandler.LogRoller) @@ -37,6 +35,15 @@ func TestErrors(t *testing.T) { if !SameNext(myHandler.Next, EmptyNext) { t.Error("'Next' field of handler was not set properly") } + + // Test Startup function + if len(c.Startup) == 0 { + t.Fatal("Expected 1 startup function, had 0") + } + err = c.Startup[0]() + if myHandler.Log == nil { + t.Error("Expected Log to be non-nil after startup because Debug is not enabled") + } } func TestErrorsParse(t *testing.T) { @@ -46,11 +53,19 @@ func TestErrorsParse(t *testing.T) { expectedErrorHandler errors.ErrorHandler }{ {`errors`, false, errors.ErrorHandler{ - LogFile: errors.DefaultLogFilename, + LogFile: "", }}, {`errors errors.txt`, false, errors.ErrorHandler{ LogFile: "errors.txt", }}, + {`errors visible`, false, errors.ErrorHandler{ + LogFile: "", + Debug: true, + }}, + {`errors { log visible }`, false, errors.ErrorHandler{ + LogFile: "", + Debug: true, + }}, {`errors { log errors.txt 404 404.html 500 500.html @@ -101,9 +116,13 @@ func TestErrorsParse(t *testing.T) { t.Errorf("Test %d errored, but it shouldn't have; got '%v'", i, err) } if actualErrorsRule.LogFile != test.expectedErrorHandler.LogFile { - t.Errorf("Test %d expected LogFile to be %s , but got %s", + t.Errorf("Test %d expected LogFile to be %s, but got %s", i, test.expectedErrorHandler.LogFile, actualErrorsRule.LogFile) } + if actualErrorsRule.Debug != test.expectedErrorHandler.Debug { + t.Errorf("Test %d expected Debug to be %v, but got %v", + i, test.expectedErrorHandler.Debug, actualErrorsRule.Debug) + } if actualErrorsRule.LogRoller != nil && test.expectedErrorHandler.LogRoller == nil || actualErrorsRule.LogRoller == nil && test.expectedErrorHandler.LogRoller != nil { t.Fatalf("Test %d expected LogRoller to be %v, but got %v", i, test.expectedErrorHandler.LogRoller, actualErrorsRule.LogRoller) diff --git a/dist/CHANGES.txt b/dist/CHANGES.txt index bf4c3c23a..9a1c0036f 100644 --- a/dist/CHANGES.txt +++ b/dist/CHANGES.txt @@ -4,7 +4,9 @@ CHANGES - basicauth: Support for legacy htpasswd files - browse: JSON response with file listing given Accept header - core: Caddyfile as command line argument +- errors: Can write full stack trace to HTTP response for debugging - errors, log: Roll log files after certain size or age +- proxy: Fix for 32-bit architectures - templates: Added .StripExt and .StripHTML methods - Internal improvements and minor bug fixes diff --git a/middleware/errors/errors.go b/middleware/errors/errors.go index 22209e914..e148899a8 100644 --- a/middleware/errors/errors.go +++ b/middleware/errors/errors.go @@ -14,13 +14,14 @@ import ( "github.com/mholt/caddy/middleware" ) -// ErrorHandler handles HTTP errors (or errors from other middleware). +// ErrorHandler handles HTTP errors (and errors from other middleware). type ErrorHandler struct { Next middleware.Handler ErrorPages map[int]string // map of status code to filename LogFile string Log *log.Logger LogRoller *middleware.LogRoller + Debug bool // if true, errors are written out to client rather than to a log } func (h ErrorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { @@ -29,12 +30,21 @@ func (h ErrorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, er status, err := h.Next.ServeHTTP(w, r) if err != nil { - h.Log.Printf("%s [ERROR %d %s] %v", time.Now().Format(timeFormat), status, r.URL.Path, err) + errMsg := fmt.Sprintf("%s [ERROR %d %s] %v", time.Now().Format(timeFormat), status, r.URL.Path, err) + + if h.Debug { + // Write error to response instead of to log + w.WriteHeader(status) + fmt.Fprintln(w, errMsg) + return 0, err // returning < 400 signals that a response has been written + } else { + h.Log.Println(errMsg) + } } if status >= 400 { - h.errorPage(w, status) - return 0, err // status < 400 signals that a response has been written + h.errorPage(w, r, status) + return 0, err } return status, err @@ -43,7 +53,7 @@ func (h ErrorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, er // errorPage serves a static error page to w according to the status // code. If there is an error serving the error page, a plaintext error // message is written instead, and the extra error is logged. -func (h ErrorHandler) errorPage(w http.ResponseWriter, code int) { +func (h ErrorHandler) errorPage(w http.ResponseWriter, r *http.Request, code int) { defaultBody := fmt.Sprintf("%d %s", code, http.StatusText(code)) // See if an error page for this status code was specified @@ -52,9 +62,9 @@ func (h ErrorHandler) errorPage(w http.ResponseWriter, code int) { // Try to open it errorPage, err := os.Open(pagePath) if err != nil { - // An error handling an error... - h.Log.Printf("%s [HTTP %d] could not load error page %s: %v", - time.Now().Format(timeFormat), code, pagePath, err) + // An additional error handling an error... + h.Log.Printf("%s [NOTICE %d %s] could not load error page: %v", + time.Now().Format(timeFormat), code, r.URL.String(), err) http.Error(w, defaultBody, code) return } @@ -67,8 +77,8 @@ func (h ErrorHandler) errorPage(w http.ResponseWriter, code int) { if err != nil { // Epic fail... sigh. - h.Log.Printf("%s [HTTP %d] could not respond with %s: %v", - time.Now().Format(timeFormat), code, pagePath, err) + h.Log.Printf("%s [NOTICE %d %s] could not respond with %s: %v", + time.Now().Format(timeFormat), code, r.URL.String(), pagePath, err) http.Error(w, defaultBody, code) } @@ -110,10 +120,18 @@ func (h ErrorHandler) recovery(w http.ResponseWriter, r *http.Request) { file = file[pkgPathPos+len(delim):] } - // Currently we don't use the function name, as file:line is more conventional - h.Log.Printf("%s [PANIC %s] %s:%d - %v", time.Now().Format(timeFormat), r.URL.String(), file, line, rec) - h.errorPage(w, http.StatusInternalServerError) + panicMsg := fmt.Sprintf("%s [PANIC %s] %s:%d - %v", time.Now().Format(timeFormat), r.URL.String(), file, line, rec) + if h.Debug { + // Write error and stack trace to the response rather than to a log + var stackBuf [4096]byte + stack := stackBuf[:runtime.Stack(stackBuf[:], false)] + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintf(w, "%s\n\n%s", panicMsg, stack) + } else { + // Currently we don't use the function name, since file:line is more conventional + h.Log.Printf(panicMsg) + h.errorPage(w, r, http.StatusInternalServerError) + } } -const DefaultLogFilename = "error.log" const timeFormat = "02/Jan/2006:15:04:05 -0700" diff --git a/middleware/errors/errors_test.go b/middleware/errors/errors_test.go index e2e82cab1..8afa6bff5 100644 --- a/middleware/errors/errors_test.go +++ b/middleware/errors/errors_test.go @@ -33,11 +33,12 @@ func TestErrors(t *testing.T) { buf := bytes.Buffer{} em := ErrorHandler{ - ErrorPages: make(map[int]string), - Log: log.New(&buf, "", 0), + ErrorPages: map[int]string{ + http.StatusNotFound: path, + http.StatusForbidden: "not_exist_file", + }, + Log: log.New(&buf, "", 0), } - em.ErrorPages[http.StatusNotFound] = path - em.ErrorPages[http.StatusForbidden] = "not_exist_file" _, notExistErr := os.Open("not_exist_file") testErr := errors.New("test error") @@ -82,8 +83,8 @@ func TestErrors(t *testing.T) { expectedCode: 0, expectedBody: fmt.Sprintf("%d %s\n", http.StatusForbidden, http.StatusText(http.StatusForbidden)), - expectedLog: fmt.Sprintf("[HTTP %d] could not load error page %s: %v\n", - http.StatusForbidden, "not_exist_file", notExistErr), + expectedLog: fmt.Sprintf("[NOTICE %d /] could not load error page: %v\n", + http.StatusForbidden, notExistErr), expectedErr: nil, }, } @@ -117,6 +118,44 @@ func TestErrors(t *testing.T) { } } +func TestVisibleErrorWithPanic(t *testing.T) { + const panicMsg = "I'm a panic" + eh := ErrorHandler{ + ErrorPages: make(map[int]string), + Debug: true, + Next: middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) { + panic(panicMsg) + }), + } + + req, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + + code, err := eh.ServeHTTP(rec, req) + + if code != 0 { + t.Errorf("Expected error handler to return 0 (it should write to response), got status %d", code) + } + if err != nil { + t.Errorf("Expected error handler to return nil error (it should panic!), but got '%v'", err) + } + + body := rec.Body.String() + + if !strings.Contains(body, "[PANIC /] middleware/errors/errors_test.go") { + t.Errorf("Expected response body to contain error log line, but it didn't:\n%s", body) + } + if !strings.Contains(body, panicMsg) { + t.Errorf("Expected response body to contain panic message, but it didn't:\n%s", body) + } + if len(body) < 500 { + t.Errorf("Expected response body to contain stack trace, but it was too short: len=%d", len(body)) + } +} + func genErrorHandler(status int, err error, body string) middleware.Handler { return middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) { fmt.Fprint(w, body)