From 2f2d357fb6f5ce0510adae10340bab3b88927223 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?W=2E=E2=80=89Mark=20Kubacki?= <wmark@hurrikane.de>
Date: Mon, 18 Apr 2016 19:42:36 +0200
Subject: [PATCH] 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 8caed54c8..32e42ddab 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 a9983bd23..56af8454b 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 {