diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go index fc8bddb67..750eb143e 100644 --- a/modules/caddyhttp/fileserver/browse.go +++ b/modules/caddyhttp/fileserver/browse.go @@ -22,6 +22,7 @@ import ( "os" "path" "strings" + "sync" "text/template" "github.com/caddyserver/caddy/v2" @@ -34,8 +35,6 @@ import ( type Browse struct { // Use this template file instead of the default browse template. TemplateFile string `json:"template_file,omitempty"` - - template *template.Template } func (fsrv *FileServer) serveBrowse(root, dirPath string, w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error { @@ -75,11 +74,14 @@ func (fsrv *FileServer) serveBrowse(root, dirPath string, w http.ResponseWriter, fsrv.browseApplyQueryParams(w, r, &listing) - // write response as either JSON or HTML - var buf *bytes.Buffer + buf := bufPool.Get().(*bytes.Buffer) + defer bufPool.Put(buf) + acceptHeader := strings.ToLower(strings.Join(r.Header["Accept"], ",")) + + // write response as either JSON or HTML if strings.Contains(acceptHeader, "application/json") { - if buf, err = fsrv.browseWriteJSON(listing); err != nil { + if err := json.NewEncoder(buf).Encode(listing.Items); err != nil { return caddyhttp.Error(http.StatusInternalServerError, err) } w.Header().Set("Content-Type", "application/json; charset=utf-8") @@ -98,12 +100,11 @@ func (fsrv *FileServer) serveBrowse(root, dirPath string, w http.ResponseWriter, browseTemplateContext: listing, } - err = fsrv.makeBrowseTemplate(tplCtx) + tpl, err := fsrv.makeBrowseTemplate(tplCtx) if err != nil { return fmt.Errorf("parsing browse template: %v", err) } - - if buf, err = fsrv.browseWriteHTML(tplCtx); err != nil { + if err := tpl.Execute(buf, tplCtx); err != nil { return caddyhttp.Error(http.StatusInternalServerError, err) } w.Header().Set("Content-Type", "text/html; charset=utf-8") @@ -161,7 +162,7 @@ func (fsrv *FileServer) browseApplyQueryParams(w http.ResponseWriter, r *http.Re } // makeBrowseTemplate creates the template to be used for directory listings. -func (fsrv *FileServer) makeBrowseTemplate(tplCtx *templateContext) error { +func (fsrv *FileServer) makeBrowseTemplate(tplCtx *templateContext) (*template.Template, error) { var tpl *template.Template var err error @@ -169,33 +170,17 @@ func (fsrv *FileServer) makeBrowseTemplate(tplCtx *templateContext) error { tpl = tplCtx.NewTemplate(path.Base(fsrv.Browse.TemplateFile)) tpl, err = tpl.ParseFiles(fsrv.Browse.TemplateFile) if err != nil { - return fmt.Errorf("parsing browse template file: %v", err) + return nil, fmt.Errorf("parsing browse template file: %v", err) } } else { tpl = tplCtx.NewTemplate("default_listing") tpl, err = tpl.Parse(defaultBrowseTemplate) if err != nil { - return fmt.Errorf("parsing default browse template: %v", err) + return nil, fmt.Errorf("parsing default browse template: %v", err) } } - fsrv.Browse.template = tpl - - return nil -} - -func (fsrv *FileServer) browseWriteJSON(listing browseTemplateContext) (*bytes.Buffer, error) { - buf := bufPool.Get().(*bytes.Buffer) - defer bufPool.Put(buf) - err := json.NewEncoder(buf).Encode(listing.Items) - return buf, err -} - -func (fsrv *FileServer) browseWriteHTML(tplCtx *templateContext) (*bytes.Buffer, error) { - buf := bufPool.Get().(*bytes.Buffer) - defer bufPool.Put(buf) - err := fsrv.Browse.template.Execute(buf, tplCtx) - return buf, err + return tpl, nil } // isSymlink return true if f is a symbolic link @@ -224,3 +209,10 @@ type templateContext struct { templates.TemplateContext browseTemplateContext } + +// bufPool is used to increase the efficiency of file listings. +var bufPool = sync.Pool{ + New: func() interface{} { + return new(bytes.Buffer) + }, +} diff --git a/modules/caddyhttp/fileserver/browse_test.go b/modules/caddyhttp/fileserver/browse_test.go deleted file mode 100644 index 30862fa44..000000000 --- a/modules/caddyhttp/fileserver/browse_test.go +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2015 Matthew Holt and The Caddy Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package fileserver - -import ( - "testing" - "text/template" -) - -func BenchmarkBrowseWriteJSON(b *testing.B) { - fsrv := new(FileServer) - listing := browseTemplateContext{ - Name: "test", - Path: "test", - CanGoUp: false, - Items: make([]fileInfo, 100), - NumDirs: 42, - NumFiles: 420, - Sort: "", - Order: "", - Limit: 42, - } - b.ResetTimer() - - for n := 0; n < b.N; n++ { - fsrv.browseWriteJSON(listing) - } -} - -func BenchmarkBrowseWriteHTML(b *testing.B) { - fsrv := new(FileServer) - fsrv.Browse = &Browse{ - TemplateFile: "", - template: template.New("test"), - } - listing := browseTemplateContext{ - Name: "test", - Path: "test", - CanGoUp: false, - Items: make([]fileInfo, 100), - NumDirs: 42, - NumFiles: 420, - Sort: "", - Order: "", - Limit: 42, - } - tplCtx := &templateContext{ - browseTemplateContext: listing, - } - fsrv.makeBrowseTemplate(tplCtx) - b.ResetTimer() - - for n := 0; n < b.N; n++ { - fsrv.browseWriteHTML(tplCtx) - } -} diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index f2320aab4..f151e323e 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -15,7 +15,6 @@ package fileserver import ( - "bytes" "fmt" weakrand "math/rand" "mime" @@ -24,7 +23,6 @@ import ( "path/filepath" "strconv" "strings" - "sync" "time" "github.com/caddyserver/caddy/v2" @@ -178,7 +176,6 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c } else if os.IsPermission(err) { return caddyhttp.Error(http.StatusForbidden, err) } - // TODO: treat this as resource exhaustion like with os.Open? Or unnecessary here? return caddyhttp.Error(http.StatusInternalServerError, err) } @@ -555,12 +552,6 @@ func (wr statusOverrideResponseWriter) WriteHeader(int) { var defaultIndexNames = []string{"index.html", "index.txt"} -var bufPool = sync.Pool{ - New: func() interface{} { - return new(bytes.Buffer) - }, -} - const ( minBackoff, maxBackoff = 2, 5 separator = string(filepath.Separator)