From a762bec06deea849a62bce3383c1ca0b84ab314d Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Sat, 16 Apr 2016 19:03:56 +0200 Subject: [PATCH 01/28] Add hostname placeholder. Header uses replacer On matched header rules, replacer is used to replace any placeholders defined in header rules iex. X-Backend {hostname} where {hostname} will be replaced by the hostname key present in the replacer hostname key added to replacer. The value is determined by the output of `os.Hostname()` --- middleware/headers/headers.go | 3 ++- middleware/headers/headers_test.go | 7 +++++++ middleware/replacer.go | 8 ++++++++ middleware/replacer_test.go | 9 +++++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/middleware/headers/headers.go b/middleware/headers/headers.go index e71e9fe3..831a1afb 100644 --- a/middleware/headers/headers.go +++ b/middleware/headers/headers.go @@ -20,13 +20,14 @@ type Headers struct { // ServeHTTP implements the middleware.Handler interface and serves requests, // setting headers on the response according to the configured rules. func (h Headers) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { + replacer := middleware.NewReplacer(r, nil, "") for _, rule := range h.Rules { if middleware.Path(r.URL.Path).Matches(rule.Path) { for _, header := range rule.Headers { if strings.HasPrefix(header.Name, "-") { w.Header().Del(strings.TrimLeft(header.Name, "-")) } else { - w.Header().Set(header.Name, header.Value) + w.Header().Set(header.Name, replacer.Replace(header.Value)) } } } diff --git a/middleware/headers/headers_test.go b/middleware/headers/headers_test.go index e6285897..0627902d 100644 --- a/middleware/headers/headers_test.go +++ b/middleware/headers/headers_test.go @@ -3,12 +3,17 @@ package headers import ( "net/http" "net/http/httptest" + "os" "testing" "github.com/mholt/caddy/middleware" ) func TestHeaders(t *testing.T) { + hostname, err := os.Hostname() + if err != nil { + t.Fatalf("Could not determine hostname: %v", err) + } for i, test := range []struct { from string name string @@ -17,6 +22,7 @@ func TestHeaders(t *testing.T) { {"/a", "Foo", "Bar"}, {"/a", "Bar", ""}, {"/a", "Baz", ""}, + {"/a", "ServerName", hostname}, {"/b", "Foo", ""}, {"/b", "Bar", "Removed in /a"}, } { @@ -27,6 +33,7 @@ func TestHeaders(t *testing.T) { Rules: []Rule{ {Path: "/a", Headers: []Header{ {Name: "Foo", Value: "Bar"}, + {Name: "ServerName", Value: "{hostname}"}, {Name: "-Bar"}, }}, }, diff --git a/middleware/replacer.go b/middleware/replacer.go index 85659ca0..6748f606 100644 --- a/middleware/replacer.go +++ b/middleware/replacer.go @@ -4,6 +4,7 @@ import ( "net" "net/http" "net/url" + "os" "path" "strconv" "strings" @@ -52,6 +53,13 @@ func NewReplacer(r *http.Request, rr *ResponseRecorder, emptyValue string) Repla } return "http" }(), + "{hostname}": func() string { + name, err := os.Hostname() + if err != nil { + return "" + } + return name + }(), "{host}": r.Host, "{path}": r.URL.Path, "{path_escaped}": url.QueryEscape(r.URL.Path), diff --git a/middleware/replacer_test.go b/middleware/replacer_test.go index 4ffdfba1..f5d50b04 100644 --- a/middleware/replacer_test.go +++ b/middleware/replacer_test.go @@ -3,6 +3,7 @@ package middleware import ( "net/http" "net/http/httptest" + "os" "strings" "testing" ) @@ -53,6 +54,14 @@ func TestReplace(t *testing.T) { request.Header.Set("ShorterVal", "1") repl := NewReplacer(request, recordRequest, "-") + hostname, err := os.Hostname() + if err != nil { + t.Fatal("Failed to determine hostname\n") + } + if expected, actual := "This hostname is "+hostname, repl.Replace("This hostname is {hostname}"); expected != actual { + t.Errorf("{hostname} replacement: expected '%s', got '%s'", expected, actual) + } + if expected, actual := "This host is localhost.", repl.Replace("This host is {host}."); expected != actual { t.Errorf("{host} replacement: expected '%s', got '%s'", expected, actual) } From 4a6121f9891d608137eacbf446b2c9d1f826ec27 Mon Sep 17 00:00:00 2001 From: W-Mark Kubacki Date: Sat, 16 Apr 2016 18:57:16 +0200 Subject: [PATCH 02/28] Tell usage of 'path' from 'filepath' and fix *path checking --- caddy/restart.go | 4 +- caddy/setup/ext.go | 3 +- middleware/browse/browse.go | 248 ++++++++++++++++++---------------- middleware/fileserver.go | 13 +- middleware/fileserver_test.go | 47 +++++-- server/server.go | 17 ++- 6 files changed, 183 insertions(+), 149 deletions(-) diff --git a/caddy/restart.go b/caddy/restart.go index 28b79d7a..717bd3a3 100644 --- a/caddy/restart.go +++ b/caddy/restart.go @@ -11,7 +11,7 @@ import ( "net" "os" "os/exec" - "path" + "path/filepath" "sync/atomic" "github.com/mholt/caddy/caddy/https" @@ -138,7 +138,7 @@ func Restart(newCaddyfile Input) error { func getCertsForNewCaddyfile(newCaddyfile Input) error { // parse the new caddyfile only up to (and including) TLS // so we can know what we need to get certs for. - configs, _, _, err := loadConfigsUpToIncludingTLS(path.Base(newCaddyfile.Path()), bytes.NewReader(newCaddyfile.Body())) + configs, _, _, err := loadConfigsUpToIncludingTLS(filepath.Base(newCaddyfile.Path()), bytes.NewReader(newCaddyfile.Body())) if err != nil { return errors.New("loading Caddyfile: " + err.Error()) } diff --git a/caddy/setup/ext.go b/caddy/setup/ext.go index 4495da66..bfd4cba5 100644 --- a/caddy/setup/ext.go +++ b/caddy/setup/ext.go @@ -2,6 +2,7 @@ package setup import ( "os" + "path/filepath" "github.com/mholt/caddy/middleware" "github.com/mholt/caddy/middleware/extensions" @@ -47,7 +48,7 @@ func extParse(c *Controller) ([]string, error) { // resourceExists returns true if the file specified at // root + path exists; false otherwise. func resourceExists(root, path string) bool { - _, err := os.Stat(root + path) + _, err := os.Stat(filepath.Join(root, path)) // technically we should use os.IsNotExist(err) // but we don't handle any other kinds of errors anyway return err == nil diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index 80ae8881..0a874ec6 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -10,6 +10,7 @@ import ( "net/url" "os" "path" + "path/filepath" "sort" "strconv" "strings" @@ -173,9 +174,11 @@ func (l Listing) applySort() { } func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root string, ignoreIndexes bool, vars interface{}) (Listing, error) { - var fileinfos []FileInfo - var dirCount, fileCount int - var urlPath = r.URL.Path + var ( + fileinfos []FileInfo + dirCount, fileCount int + urlPath = r.URL.Path + ) for _, f := range files { name := f.Name() @@ -226,143 +229,150 @@ func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root s // ServeHTTP implements the middleware.Handler interface. func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { - filename := b.Root + r.URL.Path - info, err := os.Stat(filename) - if err != nil { + var bc *Config + // See if there's a browse configuration to match the path + for i := range b.Configs { + if middleware.Path(r.URL.Path).Matches(b.Configs[i].PathScope) { + bc = &b.Configs[i] + break + } + } + if bc == nil { return b.Next.ServeHTTP(w, r) } + // Browse works on existing directories; delegate everything else + requestedFilepath := filepath.Join(b.Root, r.URL.Path) + info, err := os.Stat(requestedFilepath) + if err != nil { + switch { + case os.IsPermission(err): + return http.StatusForbidden, err + case os.IsExist(err): + return http.StatusNotFound, err + default: + return b.Next.ServeHTTP(w, r) + } + } if !info.IsDir() { return b.Next.ServeHTTP(w, r) } - // See if there's a browse configuration to match the path - for _, bc := range b.Configs { - if !middleware.Path(r.URL.Path).Matches(bc.PathScope) { - continue - } - switch r.Method { - case http.MethodGet, http.MethodHead: - default: - return http.StatusMethodNotAllowed, nil - } + // Do not reply to anything else because it might be nonsensical + switch r.Method { + case http.MethodGet, http.MethodHead: + default: + return http.StatusMethodNotAllowed, nil + } - // Browsing navigation gets messed up if browsing a directory - // that doesn't end in "/" (which it should, anyway) - if r.URL.Path[len(r.URL.Path)-1] != '/' { - http.Redirect(w, r, r.URL.Path+"/", http.StatusTemporaryRedirect) - return 0, nil - } + // Browsing navigation gets messed up if browsing a directory + // that doesn't end in "/" (which it should, anyway) + if !strings.HasSuffix(r.URL.Path, "/") { + http.Redirect(w, r, r.URL.Path+"/", http.StatusTemporaryRedirect) + return 0, nil + } - // Load directory contents - file, err := os.Open(b.Root + r.URL.Path) - if err != nil { - if os.IsPermission(err) { - return http.StatusForbidden, err - } - return http.StatusNotFound, err - } - defer file.Close() - - files, err := file.Readdir(-1) - if err != nil { + // Load directory contents + file, err := os.Open(requestedFilepath) + if err != nil { + if os.IsPermission(err) { return http.StatusForbidden, err } + return http.StatusInternalServerError, err + } + defer file.Close() - // Determine if user can browse up another folder - var canGoUp bool - curPath := strings.TrimSuffix(r.URL.Path, "/") - for _, other := range b.Configs { - if strings.HasPrefix(path.Dir(curPath), other.PathScope) { - canGoUp = true - break + files, err := file.Readdir(-1) + if err != nil { + if os.IsPermission(err) { + return http.StatusForbidden, err + } + return http.StatusInternalServerError, err + } + + // Determine if user can browse up another folder + var canGoUp bool + curPath := strings.TrimSuffix(r.URL.Path, "/") + for _, other := range b.Configs { + if strings.HasPrefix(path.Dir(curPath), other.PathScope) { + canGoUp = true + break + } + } + // Assemble listing of directory contents + listing, err := directoryListing(files, r, canGoUp, b.Root, b.IgnoreIndexes, bc.Variables) + if err != nil { // directory isn't browsable + return http.StatusInternalServerError, err + } + + // Copy the query values into the Listing struct + listing.Sort, listing.Order = r.URL.Query().Get("sort"), r.URL.Query().Get("order") + + // If the query 'sort' or 'order' is empty, use defaults or any values previously saved in Cookies + if listing.Sort == "" { + listing.Sort = "name" + if sortCookie, sortErr := r.Cookie("sort"); sortErr == nil { + listing.Sort = sortCookie.Value + } + } else { // Save the query value of 'sort' and 'order' as cookies. + http.SetCookie(w, &http.Cookie{Name: "sort", Value: listing.Sort, Path: "/"}) + http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: "/"}) + } + + if listing.Order == "" { + listing.Order = "asc" + if orderCookie, orderErr := r.Cookie("order"); orderErr == nil { + listing.Order = orderCookie.Value + } + } else { + http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: "/"}) + } + + listing.applySort() + + var buf bytes.Buffer + // Check if we should provide json + acceptHeader := strings.Join(r.Header["Accept"], ",") + if strings.Contains(strings.ToLower(acceptHeader), "application/json") { + var marsh []byte + // Check if we are limited + if limitQuery := r.URL.Query().Get("limit"); limitQuery != "" { + limit, err := strconv.Atoi(limitQuery) + if err != nil { // if the 'limit' query can't be interpreted as a number, return err + return http.StatusBadRequest, err } - } - // Assemble listing of directory contents - listing, err := directoryListing(files, r, canGoUp, b.Root, b.IgnoreIndexes, bc.Variables) - if err != nil { // directory isn't browsable - continue - } - - // Get the query vales and store them in the Listing struct - listing.Sort, listing.Order = r.URL.Query().Get("sort"), r.URL.Query().Get("order") - - // If the query 'sort' or 'order' is empty, check the cookies - if listing.Sort == "" { - sortCookie, sortErr := r.Cookie("sort") - // if there's no sorting values in the cookies, default to "name" and "asc" - if sortErr != nil { - listing.Sort = "name" - } else { // if we have values in the cookies, use them - listing.Sort = sortCookie.Value - } - } else { // save the query value of 'sort' and 'order' as cookies - http.SetCookie(w, &http.Cookie{Name: "sort", Value: listing.Sort, Path: "/"}) - http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: "/"}) - } - - if listing.Order == "" { - orderCookie, orderErr := r.Cookie("order") - // if there's no sorting values in the cookies, default to "name" and "asc" - if orderErr != nil { - listing.Order = "asc" - } else { // if we have values in the cookies, use them - listing.Order = orderCookie.Value - } - } else { // save the query value of 'sort' and 'order' as cookies - http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: "/"}) - } - - // Apply the sorting - listing.applySort() - - var buf bytes.Buffer - // check if we should provide json - acceptHeader := strings.Join(r.Header["Accept"], ",") - if strings.Contains(strings.ToLower(acceptHeader), "application/json") { - var marsh []byte - // check if we are limited - if limitQuery := r.URL.Query().Get("limit"); limitQuery != "" { - limit, err := strconv.Atoi(limitQuery) - if err != nil { // if the 'limit' query can't be interpreted as a number, return err - return http.StatusBadRequest, err - } - // if `limit` is equal or less than len(listing.Items) and bigger than 0, list them - if limit <= len(listing.Items) && limit > 0 { - marsh, err = json.Marshal(listing.Items[:limit]) - } else { // if the 'limit' query is empty, or has the wrong value, list everything - marsh, err = json.Marshal(listing.Items) - } - if err != nil { - return http.StatusInternalServerError, err - } - } else { // there's no 'limit' query, list them all + // if `limit` is equal or less than len(listing.Items) and bigger than 0, list them + if limit <= len(listing.Items) && limit > 0 { + marsh, err = json.Marshal(listing.Items[:limit]) + } else { // if the 'limit' query is empty, or has the wrong value, list everything marsh, err = json.Marshal(listing.Items) - if err != nil { - return http.StatusInternalServerError, err - } } - - // write the marshaled json to buf - if _, err = buf.Write(marsh); err != nil { - return http.StatusInternalServerError, err - } - w.Header().Set("Content-Type", "application/json; charset=utf-8") - - } else { // there's no 'application/json' in the 'Accept' header, browse normally - err = bc.Template.Execute(&buf, listing) if err != nil { return http.StatusInternalServerError, err } - w.Header().Set("Content-Type", "text/html; charset=utf-8") - + } else { // There's no 'limit' query; list them all + marsh, err = json.Marshal(listing.Items) + if err != nil { + return http.StatusInternalServerError, err + } } - buf.WriteTo(w) + // Write the marshaled json to buf + if _, err = buf.Write(marsh); err != nil { + return http.StatusInternalServerError, err + } + w.Header().Set("Content-Type", "application/json; charset=utf-8") + + } else { // There's no 'application/json' in the 'Accept' header; browse normally + err = bc.Template.Execute(&buf, listing) + if err != nil { + return http.StatusInternalServerError, err + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") - return http.StatusOK, nil } - // Didn't qualify; pass-thru - return b.Next.ServeHTTP(w, r) + buf.WriteTo(w) + + return http.StatusOK, nil } diff --git a/middleware/fileserver.go b/middleware/fileserver.go index cba4da09..cb619dc5 100644 --- a/middleware/fileserver.go +++ b/middleware/fileserver.go @@ -40,12 +40,11 @@ type fileHandler struct { } func (fh *fileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { - upath := r.URL.Path - if !strings.HasPrefix(upath, "/") { - upath = "/" + upath - r.URL.Path = upath + // r.URL.Path has already been cleaned in caddy/server by path.Clean(). + if r.URL.Path == "" { + r.URL.Path = "/" } - return fh.serveFile(w, r, path.Clean(upath)) + return fh.serveFile(w, r, r.URL.Path) } // serveFile writes the specified file to the HTTP response. @@ -86,13 +85,13 @@ func (fh *fileHandler) serveFile(w http.ResponseWriter, r *http.Request, name st url := r.URL.Path if d.IsDir() { // Ensure / at end of directory url - if url[len(url)-1] != '/' { + if !strings.HasSuffix(url, "/") { redirect(w, r, path.Base(url)+"/") return http.StatusMovedPermanently, nil } } else { // Ensure no / at end of file url - if url[len(url)-1] == '/' { + if strings.HasSuffix(url, "/") { redirect(w, r, "../"+path.Base(url)) return http.StatusMovedPermanently, nil } diff --git a/middleware/fileserver_test.go b/middleware/fileserver_test.go index 96292bdc..40d369a8 100644 --- a/middleware/fileserver_test.go +++ b/middleware/fileserver_test.go @@ -4,6 +4,7 @@ import ( "errors" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "strings" @@ -11,23 +12,30 @@ import ( "time" ) -var testDir = filepath.Join(os.TempDir(), "caddy_testdir") -var ErrCustom = errors.New("Custom Error") +var ( + ErrCustom = errors.New("Custom Error") + + testDir = filepath.Join(os.TempDir(), "caddy_testdir") + testWebRoot = filepath.Join(testDir, "webroot") +) // testFiles is a map with relative paths to test files as keys and file content as values. // The map represents the following structure: // - $TEMP/caddy_testdir/ -// '-- file1.html -// '-- dirwithindex/ -// '---- index.html -// '-- dir/ -// '---- file2.html -// '---- hidden.html +// '-- unreachable.html +// '-- webroot/ +// '---- file1.html +// '---- dirwithindex/ +// '------ index.html +// '---- dir/ +// '------ file2.html +// '------ hidden.html var testFiles = map[string]string{ - "file1.html": "

file1.html

", - filepath.Join("dirwithindex", "index.html"): "

dirwithindex/index.html

", - filepath.Join("dir", "file2.html"): "

dir/file2.html

", - filepath.Join("dir", "hidden.html"): "

dir/hidden.html

", + "unreachable.html": "

must not leak

", + filepath.Join("webroot", "file1.html"): "

file1.html

", + filepath.Join("webroot", "dirwithindex", "index.html"): "

dirwithindex/index.html

", + filepath.Join("webroot", "dir", "file2.html"): "

dir/file2.html

", + filepath.Join("webroot", "dir", "hidden.html"): "

dir/hidden.html

", } // TestServeHTTP covers positive scenarios when serving files. @@ -36,7 +44,7 @@ func TestServeHTTP(t *testing.T) { beforeServeHTTPTest(t) defer afterServeHTTPTest(t) - fileserver := FileServer(http.Dir(testDir), []string{"dir/hidden.html"}) + fileserver := FileServer(http.Dir(testWebRoot), []string{"dir/hidden.html"}) movedPermanently := "Moved Permanently" @@ -142,11 +150,20 @@ func TestServeHTTP(t *testing.T) { url: "https://foo/hidden.html", expectedStatus: http.StatusNotFound, }, + // Test 17 - try to get below the root directory. + { + url: "https://foo/%2f..%2funreachable.html", + expectedStatus: http.StatusNotFound, + }, } for i, test := range tests { responseRecorder := httptest.NewRecorder() - request, err := http.NewRequest("GET", test.url, strings.NewReader("")) + request, err := http.NewRequest("GET", test.url, nil) + // prevent any URL sanitization within Go: we need unmodified paths here + if u, _ := url.Parse(test.url); u.RawPath != "" { + request.URL.Path = u.RawPath + } status, err := fileserver.ServeHTTP(responseRecorder, request) etag := responseRecorder.Header().Get("Etag") @@ -176,7 +193,7 @@ func TestServeHTTP(t *testing.T) { // beforeServeHTTPTest creates a test directory with the structure, defined in the variable testFiles func beforeServeHTTPTest(t *testing.T) { // make the root test dir - err := os.Mkdir(testDir, os.ModePerm) + err := os.MkdirAll(testWebRoot, os.ModePerm) if err != nil { if !os.IsExist(err) { t.Fatalf("Failed to create test dir. Error was: %v", err) diff --git a/server/server.go b/server/server.go index 41d1a637..684d6151 100644 --- a/server/server.go +++ b/server/server.go @@ -14,7 +14,7 @@ import ( "net" "net/http" "os" - "path/filepath" + "path" "runtime" "strings" "sync" @@ -336,11 +336,18 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Use URL.RawPath If you need the original, "raw" URL.Path in your middleware. // Collapse any ./ ../ /// madness here instead of doing that in every plugin. if r.URL.Path != "/" { - path := filepath.Clean(r.URL.Path) - if !strings.HasPrefix(path, "/") { - path = "/" + path + cleanedPath := path.Clean(r.URL.Path) + if cleanedPath == "." { + r.URL.Path = "/" + } else { + if !strings.HasPrefix(cleanedPath, "/") { + cleanedPath = "/" + cleanedPath + } + if strings.HasSuffix(r.URL.Path, "/") && !strings.HasSuffix(cleanedPath, "/") { + cleanedPath = cleanedPath + "/" + } + r.URL.Path = cleanedPath } - r.URL.Path = path } // Execute the optional request callback if it exists and it's not disabled From 3513b6f2f7f9008458c101ffc98d6da607416428 Mon Sep 17 00:00:00 2001 From: W-Mark Kubacki Date: Sat, 16 Apr 2016 20:59:24 +0200 Subject: [PATCH 03/28] fileserver: When out of filedescriptors, spread retry attempts --- middleware/fileserver.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/middleware/fileserver.go b/middleware/fileserver.go index cb619dc5..b1c3d66d 100644 --- a/middleware/fileserver.go +++ b/middleware/fileserver.go @@ -2,10 +2,12 @@ package middleware import ( "fmt" + "math/rand" "net/http" "os" "path" "path/filepath" + "strconv" "strings" ) @@ -65,7 +67,8 @@ func (fh *fileHandler) serveFile(w http.ResponseWriter, r *http.Request, name st return http.StatusForbidden, err } // Likely the server is under load and ran out of file descriptors - w.Header().Set("Retry-After", "5") // TODO: 5 seconds enough delay? Or too much? + backoff := int(3 + rand.Int31()%3) // 3–5 seconds to prevent a stampede + w.Header().Set("Retry-After", strconv.Itoa(backoff)) return http.StatusServiceUnavailable, err } defer f.Close() From c05c5163e293c92f7e5562df558fe955c7075f7a Mon Sep 17 00:00:00 2001 From: W-Mark Kubacki Date: Sat, 16 Apr 2016 21:37:06 +0200 Subject: [PATCH 04/28] browse: Don't leak Cookies to sessions in HTTP from HTTPS --- middleware/browse/browse.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index 0a874ec6..8caed54c 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -315,8 +315,8 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { listing.Sort = sortCookie.Value } } else { // Save the query value of 'sort' and 'order' as cookies. - http.SetCookie(w, &http.Cookie{Name: "sort", Value: listing.Sort, Path: "/"}) - http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: "/"}) + http.SetCookie(w, &http.Cookie{Name: "sort", Value: listing.Sort, Path: bc.PathScope, Secure: r.TLS != nil}) + http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: bc.PathScope, Secure: r.TLS != nil}) } if listing.Order == "" { @@ -325,7 +325,7 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { listing.Order = orderCookie.Value } } else { - http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: "/"}) + http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: bc.PathScope, Secure: r.TLS != nil}) } listing.applySort() From aba3d37c881f6575f866169abff7229694c16cca Mon Sep 17 00:00:00 2001 From: elcore Date: Sun, 17 Apr 2016 18:19:41 +0200 Subject: [PATCH 05/28] Error if we are unable to marshal the ECDSA private key --- caddy/https/crypto_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/caddy/https/crypto_test.go b/caddy/https/crypto_test.go index f61669d7..d1facd44 100644 --- a/caddy/https/crypto_test.go +++ b/caddy/https/crypto_test.go @@ -105,7 +105,12 @@ func PrivateKeyBytes(key crypto.PrivateKey) []byte { case *rsa.PrivateKey: keyBytes = x509.MarshalPKCS1PrivateKey(key) case *ecdsa.PrivateKey: - keyBytes, _ = x509.MarshalECPrivateKey(key) + var err error + var t *testing.T + keyBytes, err = x509.MarshalECPrivateKey(key) + if err != nil { + t.Error(err) + } } return keyBytes } From 376e1090a3f15223fba5e99da2f9f4ce39bc8445 Mon Sep 17 00:00:00 2001 From: Tobias Weingartner Date: Sun, 17 Apr 2016 13:17:42 -0700 Subject: [PATCH 06/28] Fix PrivateKeyBytes to error out and fail tests on error. --- caddy/https/crypto_test.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/caddy/https/crypto_test.go b/caddy/https/crypto_test.go index d1facd44..936f7349 100644 --- a/caddy/https/crypto_test.go +++ b/caddy/https/crypto_test.go @@ -8,6 +8,7 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" + "errors" "os" "runtime" "testing" @@ -95,22 +96,25 @@ func TestSaveAndLoadECCPrivateKey(t *testing.T) { // PrivateKeysSame compares the bytes of a and b and returns true if they are the same. func PrivateKeysSame(a, b crypto.PrivateKey) bool { - return bytes.Equal(PrivateKeyBytes(a), PrivateKeyBytes(b)) + var abytes, bbytes []byte + var err error + + if abytes, err = PrivateKeyBytes(a); err != nil { + return false + } + if bbytes, err = PrivateKeyBytes(b); err != nil { + return false + } + return bytes.Equal(abytes, bbytes) } // PrivateKeyBytes returns the bytes of DER-encoded key. -func PrivateKeyBytes(key crypto.PrivateKey) []byte { - var keyBytes []byte +func PrivateKeyBytes(key crypto.PrivateKey) ([]byte, error) { switch key := key.(type) { case *rsa.PrivateKey: - keyBytes = x509.MarshalPKCS1PrivateKey(key) + return x509.MarshalPKCS1PrivateKey(key), nil case *ecdsa.PrivateKey: - var err error - var t *testing.T - keyBytes, err = x509.MarshalECPrivateKey(key) - if err != nil { - t.Error(err) - } + return x509.MarshalECPrivateKey(key) } - return keyBytes + return nil, errors.New("Bad juju") } From 2b51be7fd770d9acb52ca3ae4a84d1abf0eda7c5 Mon Sep 17 00:00:00 2001 From: Tobias Weingartner Date: Sun, 17 Apr 2016 14:15:14 -0700 Subject: [PATCH 07/28] Better error message. (#768) * Fix PrivateKeyBytes to error out and fail tests on error. * Better error message. --- caddy/https/crypto_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caddy/https/crypto_test.go b/caddy/https/crypto_test.go index 936f7349..efa45c65 100644 --- a/caddy/https/crypto_test.go +++ b/caddy/https/crypto_test.go @@ -116,5 +116,5 @@ func PrivateKeyBytes(key crypto.PrivateKey) ([]byte, error) { case *ecdsa.PrivateKey: return x509.MarshalECPrivateKey(key) } - return nil, errors.New("Bad juju") + return nil, errors.New("Unknown private key type") } From 924b53eb3c952d7a455c06a255187e158d540d58 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Mon, 18 Apr 2016 09:43:21 -0600 Subject: [PATCH 08/28] Minor changes --- dist/CHANGES.txt | 4 ++-- middleware/context.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dist/CHANGES.txt b/dist/CHANGES.txt index ada02c2c..c30b08c5 100644 --- a/dist/CHANGES.txt +++ b/dist/CHANGES.txt @@ -1,8 +1,8 @@ CHANGES -- Built with Go 1.6.1 -- New pprof directive for exposing process performance profile +- Built with Go 1.6.2 +- New pprof directive for exposing process profiling endpoints - New expvar directive for exposing memory/GC performance - New -restart option to force in-process restarts on Unix systems - Only fail to start if managed certificate is expired (issue #642) diff --git a/middleware/context.go b/middleware/context.go index 3facb953..a42dbddc 100644 --- a/middleware/context.go +++ b/middleware/context.go @@ -231,7 +231,7 @@ func (c Context) ToUpper(s string) string { return strings.ToUpper(s) } -// Split is a passthrough to strings.Split. It will split the first argument at each instance of the seperator and return a slice of strings. +// Split is a passthrough to strings.Split. It will split the first argument at each instance of the separator and return a slice of strings. func (c Context) Split(s string, sep string) []string { return strings.Split(s, sep) } From 2f2d357fb6f5ce0510adae10340bab3b88927223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?W=2E=E2=80=89Mark=20Kubacki?= Date: Mon, 18 Apr 2016 19:42:36 +0200 Subject: [PATCH 09/28] browse: Fix known bugs (#770) * browse: Catch the case of a directory disappearing before having been read * browse: Revert to old pass-through behaviour PROPFIND is a request for an alternate view on a directory's contents, which response is indeed not implemented but ideally allowed to ask for. OPTIONS would ideally return (at least) what methods the requestor could use, which is an allowed request method, too. This addresses #767. --- middleware/browse/browse.go | 30 ++++++++++++++++++++---------- middleware/browse/browse_test.go | 13 +++---------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index 8caed54c..32e42dda 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -234,12 +234,11 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { for i := range b.Configs { if middleware.Path(r.URL.Path).Matches(b.Configs[i].PathScope) { bc = &b.Configs[i] - break + goto inScope } } - if bc == nil { - return b.Next.ServeHTTP(w, r) - } + return b.Next.ServeHTTP(w, r) +inScope: // Browse works on existing directories; delegate everything else requestedFilepath := filepath.Join(b.Root, r.URL.Path) @@ -261,8 +260,11 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { // Do not reply to anything else because it might be nonsensical switch r.Method { case http.MethodGet, http.MethodHead: + // proceed, noop + case "PROPFIND", http.MethodOptions: + return http.StatusNotImplemented, nil default: - return http.StatusMethodNotAllowed, nil + return b.Next.ServeHTTP(w, r) } // Browsing navigation gets messed up if browsing a directory @@ -275,19 +277,27 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { // Load directory contents file, err := os.Open(requestedFilepath) if err != nil { - if os.IsPermission(err) { + switch { + case os.IsPermission(err): return http.StatusForbidden, err + case os.IsExist(err): + return http.StatusGone, err + default: + return http.StatusInternalServerError, err } - return http.StatusInternalServerError, err } defer file.Close() files, err := file.Readdir(-1) if err != nil { - if os.IsPermission(err) { + switch { + case os.IsPermission(err): return http.StatusForbidden, err + case os.IsExist(err): + return http.StatusGone, err + default: + return http.StatusInternalServerError, err } - return http.StatusInternalServerError, err } // Determine if user can browse up another folder @@ -302,7 +312,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, bc.Variables) if err != nil { // directory isn't browsable - return http.StatusInternalServerError, err + return b.Next.ServeHTTP(w, r) } // Copy the query values into the Listing struct diff --git a/middleware/browse/browse_test.go b/middleware/browse/browse_test.go index a9983bd2..56af8454 100644 --- a/middleware/browse/browse_test.go +++ b/middleware/browse/browse_test.go @@ -112,8 +112,7 @@ func TestBrowseHTTPMethods(t *testing.T) { b := Browse{ Next: middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) { - t.Fatalf("Next shouldn't be called") - return 0, nil + return http.StatusTeapot, nil // not t.Fatalf, or we will not see what other methods yield }), Root: "./testdata", Configs: []Config{ @@ -128,14 +127,8 @@ func TestBrowseHTTPMethods(t *testing.T) { for method, expected := range map[string]int{ http.MethodGet: http.StatusOK, http.MethodHead: http.StatusOK, - http.MethodOptions: http.StatusMethodNotAllowed, - http.MethodPost: http.StatusMethodNotAllowed, - http.MethodPut: http.StatusMethodNotAllowed, - http.MethodPatch: http.StatusMethodNotAllowed, - http.MethodDelete: http.StatusMethodNotAllowed, - "COPY": http.StatusMethodNotAllowed, - "MOVE": http.StatusMethodNotAllowed, - "MKCOL": http.StatusMethodNotAllowed, + http.MethodOptions: http.StatusNotImplemented, + "PROPFIND": http.StatusNotImplemented, } { req, err := http.NewRequest(method, "/photos/", nil) if err != nil { From 69081360928587fd0c246ac1a9153cc2d46e695b Mon Sep 17 00:00:00 2001 From: W-Mark Kubacki Date: Mon, 18 Apr 2016 21:01:47 +0200 Subject: [PATCH 10/28] browse: Split ServeHTTP into small specialized functions --- middleware/browse/browse.go | 202 ++++++++++++++++++------------- middleware/browse/browse_test.go | 2 +- 2 files changed, 118 insertions(+), 86 deletions(-) diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index 32e42dda..3a727d96 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -5,7 +5,6 @@ package browse import ( "bytes" "encoding/json" - "errors" "net/http" "net/url" "os" @@ -173,22 +172,20 @@ func (l Listing) applySort() { } } -func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root string, ignoreIndexes bool, vars interface{}) (Listing, error) { +func directoryListing(files []os.FileInfo, canGoUp bool, urlPath string) (Listing, bool) { var ( fileinfos []FileInfo dirCount, fileCount int - urlPath = r.URL.Path + hasIndexFile bool ) for _, f := range files { name := f.Name() - // Directory is not browsable if it contains index file - if !ignoreIndexes { - for _, indexName := range middleware.IndexPages { - if name == indexName { - return Listing{}, errors.New("Directory contains index file, not browsable!") - } + for _, indexName := range middleware.IndexPages { + if name == indexName { + hasIndexFile = true + break } } @@ -218,16 +215,11 @@ func directoryListing(files []os.FileInfo, r *http.Request, canGoUp bool, root s Items: fileinfos, NumDirs: dirCount, NumFiles: fileCount, - Context: middleware.Context{ - Root: http.Dir(root), - Req: r, - URL: r.URL, - }, - User: vars, - }, nil + }, hasIndexFile } -// ServeHTTP implements the middleware.Handler interface. +// ServeHTTP determines if the request is for this plugin, and if all prerequisites are met. +// If so, control is handed over to ServeListing. func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { var bc *Config // See if there's a browse configuration to match the path @@ -274,21 +266,69 @@ inScope: return 0, nil } + return b.ServeListing(w, r, requestedFilepath, bc) +} + +func (b Browse) loadDirectoryContents(requestedFilepath, urlPath string) (*Listing, bool, error) { // Load directory contents file, err := os.Open(requestedFilepath) if err != nil { - switch { - case os.IsPermission(err): - return http.StatusForbidden, err - case os.IsExist(err): - return http.StatusGone, err - default: - return http.StatusInternalServerError, err - } + return nil, false, err } defer file.Close() files, err := file.Readdir(-1) + if err != nil { + return nil, false, err + } + + // Determine if user can browse up another folder + var canGoUp bool + curPathDir := path.Dir(strings.TrimSuffix(urlPath, "/")) + for _, other := range b.Configs { + if strings.HasPrefix(curPathDir, other.PathScope) { + canGoUp = true + break + } + } + + // Assemble listing of directory contents + listing, hasIndex := directoryListing(files, canGoUp, urlPath) + + return &listing, hasIndex, nil +} + +// handleSortOrder gets and stores for a Listing the 'sort' and 'order'. +// +// This sets Cookies. +func (b Browse) handleSortOrder(w http.ResponseWriter, r *http.Request, scope string) (string, string) { + sort, order := r.URL.Query().Get("sort"), r.URL.Query().Get("order") + + // If the query 'sort' or 'order' is empty, use defaults or any values previously saved in Cookies + if sort == "" { + sort = "name" + if sortCookie, sortErr := r.Cookie("sort"); sortErr == nil { + sort = sortCookie.Value + } + } else { + http.SetCookie(w, &http.Cookie{Name: "sort", Value: sort, Path: scope, Secure: r.TLS != nil}) + } + + if order == "" { + order = "asc" + if orderCookie, orderErr := r.Cookie("order"); orderErr == nil { + order = orderCookie.Value + } + } else { + http.SetCookie(w, &http.Cookie{Name: "order", Value: order, Path: scope, Secure: r.TLS != nil}) + } + + return sort, order +} + +// ServeListing returns a formatted view of 'requestedFilepath' contents'. +func (b Browse) ServeListing(w http.ResponseWriter, r *http.Request, requestedFilepath string, bc *Config) (int, error) { + listing, containsIndex, err := b.loadDirectoryContents(requestedFilepath, r.URL.Path) if err != nil { switch { case os.IsPermission(err): @@ -299,83 +339,40 @@ inScope: return http.StatusInternalServerError, err } } - - // Determine if user can browse up another folder - var canGoUp bool - curPath := strings.TrimSuffix(r.URL.Path, "/") - for _, other := range b.Configs { - if strings.HasPrefix(path.Dir(curPath), other.PathScope) { - canGoUp = true - break - } - } - // Assemble listing of directory contents - listing, err := directoryListing(files, r, canGoUp, b.Root, b.IgnoreIndexes, bc.Variables) - if err != nil { // directory isn't browsable + if containsIndex && !b.IgnoreIndexes { // directory isn't browsable return b.Next.ServeHTTP(w, r) } + listing.Context = middleware.Context{ + Root: http.Dir(b.Root), + Req: r, + URL: r.URL, + } + listing.User = bc.Variables // Copy the query values into the Listing struct - listing.Sort, listing.Order = r.URL.Query().Get("sort"), r.URL.Query().Get("order") - - // If the query 'sort' or 'order' is empty, use defaults or any values previously saved in Cookies - if listing.Sort == "" { - listing.Sort = "name" - if sortCookie, sortErr := r.Cookie("sort"); sortErr == nil { - listing.Sort = sortCookie.Value - } - } else { // Save the query value of 'sort' and 'order' as cookies. - http.SetCookie(w, &http.Cookie{Name: "sort", Value: listing.Sort, Path: bc.PathScope, Secure: r.TLS != nil}) - http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: bc.PathScope, Secure: r.TLS != nil}) - } - - if listing.Order == "" { - listing.Order = "asc" - if orderCookie, orderErr := r.Cookie("order"); orderErr == nil { - listing.Order = orderCookie.Value - } - } else { - http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: bc.PathScope, Secure: r.TLS != nil}) - } + listing.Sort, listing.Order = b.handleSortOrder(w, r, bc.PathScope) listing.applySort() - var buf bytes.Buffer - // Check if we should provide json - acceptHeader := strings.Join(r.Header["Accept"], ",") - if strings.Contains(strings.ToLower(acceptHeader), "application/json") { - var marsh []byte - // Check if we are limited + var buf *bytes.Buffer + acceptHeader := strings.ToLower(strings.Join(r.Header["Accept"], ",")) + switch { + case strings.Contains(acceptHeader, "application/json"): + var limit int if limitQuery := r.URL.Query().Get("limit"); limitQuery != "" { - limit, err := strconv.Atoi(limitQuery) + limit, err = strconv.Atoi(limitQuery) if err != nil { // if the 'limit' query can't be interpreted as a number, return err return http.StatusBadRequest, err } - // if `limit` is equal or less than len(listing.Items) and bigger than 0, list them - if limit <= len(listing.Items) && limit > 0 { - marsh, err = json.Marshal(listing.Items[:limit]) - } else { // if the 'limit' query is empty, or has the wrong value, list everything - marsh, err = json.Marshal(listing.Items) - } - if err != nil { - return http.StatusInternalServerError, err - } - } else { // There's no 'limit' query; list them all - marsh, err = json.Marshal(listing.Items) - if err != nil { - return http.StatusInternalServerError, err - } } - // Write the marshaled json to buf - if _, err = buf.Write(marsh); err != nil { + if buf, err = b.formatAsJSON(listing, bc, limit); err != nil { return http.StatusInternalServerError, err } w.Header().Set("Content-Type", "application/json; charset=utf-8") - } else { // There's no 'application/json' in the 'Accept' header; browse normally - err = bc.Template.Execute(&buf, listing) - if err != nil { + default: // There's no 'application/json' in the 'Accept' header; browse normally + if buf, err = b.formatAsHTML(listing, bc); err != nil { return http.StatusInternalServerError, err } w.Header().Set("Content-Type", "text/html; charset=utf-8") @@ -386,3 +383,38 @@ inScope: return http.StatusOK, nil } + +func (b Browse) formatAsJSON(listing *Listing, bc *Config, limit int) (*bytes.Buffer, error) { + buf := new(bytes.Buffer) + var marsh []byte + var err error + + // Check if we are limited + if limit > 0 { + // if `limit` is equal or less than len(listing.Items) and bigger than 0, list them + if limit <= len(listing.Items) { + marsh, err = json.Marshal(listing.Items[:limit]) + } else { // if the 'limit' query is empty, or has the wrong value, list everything + marsh, err = json.Marshal(listing.Items) + } + if err != nil { + return nil, err + } + } else { // There's no 'limit' query; list them all + marsh, err = json.Marshal(listing.Items) + if err != nil { + return nil, err + } + } + + // Write the marshaled json to buf + _, err = buf.Write(marsh) + + return buf, err +} + +func (b Browse) formatAsHTML(listing *Listing, bc *Config) (*bytes.Buffer, error) { + buf := new(bytes.Buffer) + err := bc.Template.Execute(buf, listing) + return buf, err +} diff --git a/middleware/browse/browse_test.go b/middleware/browse/browse_test.go index 56af8454..c8792524 100644 --- a/middleware/browse/browse_test.go +++ b/middleware/browse/browse_test.go @@ -315,7 +315,7 @@ func TestBrowseJson(t *testing.T) { code, err := b.ServeHTTP(rec, req) if code != http.StatusOK { - t.Fatalf("Wrong status, expected %d, got %d", http.StatusOK, code) + t.Fatalf("In test %d: Wrong status, expected %d, got %d", i, http.StatusOK, code) } if rec.HeaderMap.Get("Content-Type") != "application/json; charset=utf-8" { t.Fatalf("Expected Content type to be application/json; charset=utf-8, but got %s ", rec.HeaderMap.Get("Content-Type")) From 1d38d113f8e760872c22d10b67402f20d159661c Mon Sep 17 00:00:00 2001 From: W-Mark Kubacki Date: Mon, 18 Apr 2016 21:51:37 +0200 Subject: [PATCH 11/28] browse: Move predicate 'limit' to ServeListing This keeps the interface of all available formatters honest, and allows for truncated listings all formats. --- caddy/setup/browse.go | 21 +++++----- middleware/browse/browse.go | 78 ++++++++++++++++++------------------- 2 files changed, 49 insertions(+), 50 deletions(-) diff --git a/caddy/setup/browse.go b/caddy/setup/browse.go index 28cb2582..ff921b9b 100644 --- a/caddy/setup/browse.go +++ b/caddy/setup/browse.go @@ -309,6 +309,9 @@ footer {
{{.NumDirs}} director{{if eq 1 .NumDirs}}y{{else}}ies{{end}} {{.NumFiles}} file{{if ne 1 .NumFiles}}s{{end}} + {{- if ne 0 .ItemsLimitedTo}} + (of which only {{.ItemsLimitedTo}} are displayed) + {{- end}}
@@ -316,29 +319,29 @@ footer { {{if and (eq .Sort "name") (ne .Order "desc")}} - Name + Name {{else if and (eq .Sort "name") (ne .Order "asc")}} - Name + Name {{else}} - Name + Name {{end}} {{if and (eq .Sort "size") (ne .Order "desc")}} - Size + Size {{else if and (eq .Sort "size") (ne .Order "asc")}} - Size + Size {{else}} - Size + Size {{end}} {{if and (eq .Sort "time") (ne .Order "desc")}} - Modified + Modified {{else if and (eq .Sort "time") (ne .Order "asc")}} - Modified + Modified {{else}} - Modified + Modified {{end}} diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index 3a727d96..e350c07a 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -62,6 +62,9 @@ type Listing struct { // And which order Order string + // If ≠0 then Items have been limited to that many elements + ItemsLimitedTo int + // Optional custom variables for use in browse templates User interface{} @@ -298,32 +301,42 @@ func (b Browse) loadDirectoryContents(requestedFilepath, urlPath string) (*Listi return &listing, hasIndex, nil } -// handleSortOrder gets and stores for a Listing the 'sort' and 'order'. +// handleSortOrder gets and stores for a Listing the 'sort' and 'order', +// and reads 'limit' if given. The latter is 0 if not given. // // This sets Cookies. -func (b Browse) handleSortOrder(w http.ResponseWriter, r *http.Request, scope string) (string, string) { - sort, order := r.URL.Query().Get("sort"), r.URL.Query().Get("order") +func (b Browse) handleSortOrder(w http.ResponseWriter, r *http.Request, scope string) (sort string, order string, limit int, err error) { + sort, order, limitQuery := r.URL.Query().Get("sort"), r.URL.Query().Get("order"), r.URL.Query().Get("limit") // If the query 'sort' or 'order' is empty, use defaults or any values previously saved in Cookies - if sort == "" { + switch sort { + case "": sort = "name" if sortCookie, sortErr := r.Cookie("sort"); sortErr == nil { sort = sortCookie.Value } - } else { + case "name", "size", "type": http.SetCookie(w, &http.Cookie{Name: "sort", Value: sort, Path: scope, Secure: r.TLS != nil}) } - if order == "" { + switch order { + case "": order = "asc" if orderCookie, orderErr := r.Cookie("order"); orderErr == nil { order = orderCookie.Value } - } else { + case "asc", "desc": http.SetCookie(w, &http.Cookie{Name: "order", Value: order, Path: scope, Secure: r.TLS != nil}) } - return sort, order + if limitQuery != "" { + limit, err = strconv.Atoi(limitQuery) + if err != nil { // if the 'limit' query can't be interpreted as a number, return err + return + } + } + + return } // ServeListing returns a formatted view of 'requestedFilepath' contents'. @@ -350,23 +363,24 @@ func (b Browse) ServeListing(w http.ResponseWriter, r *http.Request, requestedFi listing.User = bc.Variables // Copy the query values into the Listing struct - listing.Sort, listing.Order = b.handleSortOrder(w, r, bc.PathScope) + var limit int + listing.Sort, listing.Order, limit, err = b.handleSortOrder(w, r, bc.PathScope) + if err != nil { + return http.StatusBadRequest, err + } listing.applySort() + if limit > 0 && limit <= len(listing.Items) { + listing.Items = listing.Items[:limit] + listing.ItemsLimitedTo = limit + } + var buf *bytes.Buffer acceptHeader := strings.ToLower(strings.Join(r.Header["Accept"], ",")) switch { case strings.Contains(acceptHeader, "application/json"): - var limit int - if limitQuery := r.URL.Query().Get("limit"); limitQuery != "" { - limit, err = strconv.Atoi(limitQuery) - if err != nil { // if the 'limit' query can't be interpreted as a number, return err - return http.StatusBadRequest, err - } - } - - if buf, err = b.formatAsJSON(listing, bc, limit); err != nil { + if buf, err = b.formatAsJSON(listing, bc); err != nil { return http.StatusInternalServerError, err } w.Header().Set("Content-Type", "application/json; charset=utf-8") @@ -384,32 +398,14 @@ func (b Browse) ServeListing(w http.ResponseWriter, r *http.Request, requestedFi return http.StatusOK, nil } -func (b Browse) formatAsJSON(listing *Listing, bc *Config, limit int) (*bytes.Buffer, error) { - buf := new(bytes.Buffer) - var marsh []byte - var err error - - // Check if we are limited - if limit > 0 { - // if `limit` is equal or less than len(listing.Items) and bigger than 0, list them - if limit <= len(listing.Items) { - marsh, err = json.Marshal(listing.Items[:limit]) - } else { // if the 'limit' query is empty, or has the wrong value, list everything - marsh, err = json.Marshal(listing.Items) - } - if err != nil { - return nil, err - } - } else { // There's no 'limit' query; list them all - marsh, err = json.Marshal(listing.Items) - if err != nil { - return nil, err - } +func (b Browse) formatAsJSON(listing *Listing, bc *Config) (*bytes.Buffer, error) { + marsh, err := json.Marshal(listing.Items) + if err != nil { + return nil, err } - // Write the marshaled json to buf + buf := new(bytes.Buffer) _, err = buf.Write(marsh) - return buf, err } From 239f6825f76d3046198ab556b1851e40422c9558 Mon Sep 17 00:00:00 2001 From: W-Mark Kubacki Date: Mon, 18 Apr 2016 23:05:08 +0200 Subject: [PATCH 12/28] browse: When sorting by size, offset directories Assigns negative sizes to directories in order to have them listed reliably before any zero-sized files. That order is what most users expect when sorting by size. As side effect directories will appear before files on all filesystem implementations. To give an example: before this change directories had a size of 4 KiB when using Linux with ext4 or tmpfs, and with ZFS a size resembling an estimation of the number of leaves within said directory. --- middleware/browse/browse.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go index e350c07a..a21b2304 100644 --- a/middleware/browse/browse.go +++ b/middleware/browse/browse.go @@ -135,9 +135,20 @@ func (l byName) Less(i, j int) bool { } // By Size -func (l bySize) Len() int { return len(l.Items) } -func (l bySize) Swap(i, j int) { l.Items[i], l.Items[j] = l.Items[j], l.Items[i] } -func (l bySize) Less(i, j int) bool { return l.Items[i].Size < l.Items[j].Size } +func (l bySize) Len() int { return len(l.Items) } +func (l bySize) Swap(i, j int) { l.Items[i], l.Items[j] = l.Items[j], l.Items[i] } + +const directoryOffset = -1 << 31 // = math.MinInt32 +func (l bySize) Less(i, j int) bool { + iSize, jSize := l.Items[i].Size, l.Items[j].Size + if l.Items[i].IsDir { + iSize = directoryOffset + iSize + } + if l.Items[j].IsDir { + jSize = directoryOffset + jSize + } + return iSize < jSize +} // By Time func (l byTime) Len() int { return len(l.Items) } From cc6aa6b54bd560527c18524b803a280c56cb88e7 Mon Sep 17 00:00:00 2001 From: W-Mark Kubacki Date: Mon, 18 Apr 2016 22:47:53 +0200 Subject: [PATCH 13/28] browse: Remove whitespace from template's output, annotate output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a surplus — next to "go up". Identifies the preamble as the table's summary. Emits filesizes in bytes, which can be consumed by any browser-side scripts or utilized in sorting when the table is copy-and-pasted into a spreadsheet software. Uses