Minor cleanup and fixes

- Do not allow '.' as path component, because it undermines depth
  checks, and add tests
- Fix GiB reporting
- Fix metrics label
- Helper function for http errors

handlers.go

@@ -32,30 +32,41 @@ type Server struct {
 	quotaManager *quota.Manager
 }
 
+// MaxFolderDepth is the maxDepth param passed to splitURLPath.
+// A max depth of 2 mean that we accept folders like: '/', '/foo' and '/foo/bar'
+// TODO: Move to a Server option
+const MaxFolderDepth = 2
+
+// httpDefaultError write a HTTP error with the default description
+func httpDefaultError(w http.ResponseWriter, code int) {
+	http.Error(w, http.StatusText(code), code)
+}
+
 // ServeHTTP makes this server an http.Handler. It handlers the administrative
 // part of the request (figuring out the filesystem location, performing
 // authentication, etc) and then passes it on to repo.Handler for actual
 // REST API processing.
 func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	// First of all, check auth
+	// First of all, check auth (will always pass if NoAuth is set)
 	username, ok := s.checkAuth(r)
 	if !ok {
-		http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+		httpDefaultError(w, http.StatusUnauthorized)
+		return
 	}
 
 	// Perform the path parsing to determine the repo folder and remainder for the
-	// repo handler
+	// repo handler.
-	folderPath, remainder := splitURLPath(r.URL.Path, 2) // FIXME: configurable
+	folderPath, remainder := splitURLPath(r.URL.Path, MaxFolderDepth)
 	if !folderPathValid(folderPath) {
 		log.Printf("Invalid request path: %s", r.URL.Path)
-		http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
+		httpDefaultError(w, http.StatusNotFound)
 		return
 	}
 
 	// Check if the current user is allowed to access this path
 	if !s.NoAuth && s.PrivateRepos {
 		if len(folderPath) == 0 || folderPath[0] != username {
-			http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+			httpDefaultError(w, http.StatusUnauthorized)
 			return
 		}
 	}
@@ -65,7 +76,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if err != nil {
 		// We did not expect an error at this stage, because we just checked the path
 		log.Printf("Unexpected join error for path %q", r.URL.Path)
-		http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
+		httpDefaultError(w, http.StatusNotFound)
 		return
 	}
 
@@ -81,7 +92,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	repoHandler, err := repo.New(fsPath, opt)
 	if err != nil {
 		log.Printf("repo.New error: %v", err)
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+		httpDefaultError(w, http.StatusInternalServerError)
 		return
 	}
 	r.URL.Path = remainder // strip folderPath for next handler
@@ -170,7 +181,7 @@ func splitURLPath(urlPath string, maxDepth int) (folderPath []string, remainder
 // safe.
 func folderPathValid(folderPath []string) bool {
 	for _, name := range folderPath {
-		if name == "" || name == ".." || !valid(name) {
+		if name == "" || name == ".." || name == "." || !valid(name) {
 			return false
 		}
 	}

handlers_test.go

@@ -248,10 +248,18 @@ func TestSplitURLPath(t *testing.T) {
 		{"/foo/bar/zzz/locks/0123", 2, []string{"foo", "bar"}, "/zzz/locks/0123"},
 		{"/foo/bar/zzz/locks/0123", 3, []string{"foo", "bar", "zzz"}, "/locks/0123"},
 		{"/foo/bar/locks/", 2, []string{"foo", "bar"}, "/locks/"},
+		{"/foo/locks/", 2, []string{"foo"}, "/locks/"},
+		{"/foo/data/", 2, []string{"foo"}, "/data/"},
+		{"/foo/index/", 2, []string{"foo"}, "/index/"},
+		{"/foo/keys/", 2, []string{"foo"}, "/keys/"},
+		{"/foo/snapshots/", 2, []string{"foo"}, "/snapshots/"},
+		{"/foo/config", 2, []string{"foo"}, "/config"},
+		{"/foo/", 2, []string{"foo"}, "/"},
 		{"/foo/bar/", 2, []string{"foo", "bar"}, "/"},
 		{"/foo/bar", 2, []string{"foo"}, "/bar"},
 		{"/locks/", 2, nil, "/locks/"},
 		// This function only splits, it does not check the path components!
+		{"/././locks/", 2, []string{"..", ".."}, "/locks/"},
 		{"/../../locks/", 2, []string{"..", ".."}, "/locks/"},
 		{"///locks/", 2, []string{"", ""}, "/locks/"},
 		{"////locks/", 2, []string{"", ""}, "//locks/"},

metrics.go

@@ -63,7 +63,7 @@ func makeBlobMetricFunc(username string, folderPath []string) repo.BlobMetricFun
 	var f repo.BlobMetricFunc = func(objectType string, operation repo.BlobOperation, nBytes uint64) {
 		labels := prometheus.Labels{
 			"user": username,
-			"repo": strings.Join(folderPath, ""),
+			"repo": strings.Join(folderPath, "/"),
 			"type": objectType,
 		}
 		switch operation {

mux.go

@@ -12,6 +12,10 @@ import (
 	"github.com/restic/rest-server/quota"
 )
 
+const (
+	GiB = 1024 * 1024 * 1024
+)
+
 func (s *Server) debugHandler(next http.Handler) http.Handler {
 	return http.HandlerFunc(
 		func(w http.ResponseWriter, r *http.Request) {
@@ -58,7 +62,7 @@ func NewHandler(server *Server) (http.Handler, error) {
 			return nil, err
 		}
 		server.quotaManager = qm
-		log.Printf("Quota initialized, currenly using %.2f GiB", float64(qm.SpaceUsed()/1024/1024))
+		log.Printf("Quota initialized, currenly using %.2f GiB", float64(qm.SpaceUsed())/GiB)
 	}
 
 	mux := http.NewServeMux()

repo/repo.go

@@ -165,14 +165,14 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 // getObject parses the URL path and returns the objectType and objectID,
 // if any. The objectID is optional.
 func (h *Handler) getObject(urlPath string) (objectType, objectID string) {
-	if m := BlobPathRE.FindStringSubmatch(urlPath); len(m) > 0 {
+	m := BlobPathRE.FindStringSubmatch(urlPath)
-		if len(m) == 2 || m[2] == "" {
+	if len(m) == 0 {
-			return m[1], ""
+		return "", "" // no match
-		}
-		return m[1], m[2]
-	} else {
-		return "", ""
 	}
+	if len(m) == 2 || m[2] == "" {
+		return m[1], "" // no objectID
+	}
+	return m[1], m[2]
 }
 
 // getSubPath returns the path for a file or subdir in the root of the repo.