Commit 4a6121f9 authored by W-Mark Kubacki's avatar W-Mark Kubacki

Tell usage of 'path' from 'filepath' and fix *path checking

parent b75016e6
......@@ -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())
}
......
......@@ -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
......
......@@ -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,21 +229,36 @@ 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
}
// Do not reply to anything else because it might be nonsensical
switch r.Method {
case http.MethodGet, http.MethodHead:
default:
......@@ -249,25 +267,28 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
// 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] != '/' {
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)
file, err := os.Open(requestedFilepath)
if err != nil {
if os.IsPermission(err) {
return http.StatusForbidden, err
}
return http.StatusNotFound, err
return http.StatusInternalServerError, err
}
defer file.Close()
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
......@@ -281,47 +302,40 @@ 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
continue
return http.StatusInternalServerError, err
}
// Get the query vales and store them in the Listing struct
// 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, check the cookies
// If the query 'sort' or 'order' is empty, use defaults or any values previously saved in 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
if sortCookie, sortErr := r.Cookie("sort"); sortErr == nil {
listing.Sort = sortCookie.Value
}
} else { // save the query value of 'sort' and 'order' as cookies
} 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
if orderCookie, orderErr := r.Cookie("order"); orderErr == nil {
listing.Order = orderCookie.Value
}
} else { // save the query value of 'sort' and 'order' as cookies
} else {
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
// 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
// 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
......@@ -336,20 +350,20 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
if err != nil {
return http.StatusInternalServerError, err
}
} else { // there's no 'limit' query, list them all
} 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
// 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
} 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
......@@ -361,8 +375,4 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
buf.WriteTo(w)
return http.StatusOK, nil
}
// Didn't qualify; pass-thru
return b.Next.ServeHTTP(w, r)
}
......@@ -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
}
......
......@@ -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": "<h1>file1.html</h1>",
filepath.Join("dirwithindex", "index.html"): "<h1>dirwithindex/index.html</h1>",
filepath.Join("dir", "file2.html"): "<h1>dir/file2.html</h1>",
filepath.Join("dir", "hidden.html"): "<h1>dir/hidden.html</h1>",
"unreachable.html": "<h1>must not leak</h1>",
filepath.Join("webroot", "file1.html"): "<h1>file1.html</h1>",
filepath.Join("webroot", "dirwithindex", "index.html"): "<h1>dirwithindex/index.html</h1>",
filepath.Join("webroot", "dir", "file2.html"): "<h1>dir/file2.html</h1>",
filepath.Join("webroot", "dir", "hidden.html"): "<h1>dir/hidden.html</h1>",
}
// 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)
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment