Prevent panic on /remote.php/dav/files/

changelog/unreleased/handle-invalid-webdav-listing.md

@@ -0,0 +1,5 @@
+Bugfix: Do not panic on remote.php/dav/files/
+
+Currently requests to /remote.php/dav/files/ result in panics since we cannot longer strip the user + destination from the url. This fixes the server response code and adds an error body to the response.
+
+https://github.com/cs3org/reva/pull/1320

internal/http/services/owncloud/ocdav/dav.go

@@ -93,6 +93,29 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
 		ctx := r.Context()
 		log := appctx.GetLogger(ctx)
 
+		// if there is no file in the request url we assume the request url is: "/remote.php/dav/files"
+		// https://github.com/owncloud/core/blob/18475dac812064b21dabcc50f25ef3ffe55691a5/tests/acceptance/features/apiWebdavOperations/propfind.feature
+		if r.URL.Path == "/files" {
+			log.Debug().Str("path", r.URL.Path).Msg("method not allowed")
+			w.WriteHeader(http.StatusMethodNotAllowed)
+			b, err := Marshal(exception{
+				code:    SabredavMethodNotAllowed,
+				message: "Listing members of this collection is disabled",
+			})
+			if err != nil {
+				log.Error().Msgf("error marshaling xml response: %s", b)
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			_, err = w.Write(b)
+			if err != nil {
+				log.Error().Msgf("error writing xml response: %s", b)
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			return
+		}
+
 		var head string
 		head, r.URL.Path = router.ShiftPath(r.URL.Path)
 

internal/http/services/owncloud/ocdav/error.go

@@ -0,0 +1,51 @@
+// Copyright 2018-2020 CERN
+//
+// 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.
+//
+// In applying this license, CERN does not waive the privileges and immunities
+// granted to it by virtue of its status as an Intergovernmental Organization
+// or submit itself to any jurisdiction.
+
+package ocdav
+
+import (
+	"encoding/xml"
+)
+
+type code int
+
+const (
+	// SabredavMethodNotAllowed maps to HTTP 405
+	SabredavMethodNotAllowed code = iota
+)
+
+var (
+	codesEnum = []string{
+		"Sabre\\DAV\\Exception\\MethodNotAllowed",
+	}
+)
+
+type exception struct {
+	code    code
+	message string
+}
+
+// Marshal just calls the xml marshaller for a given exception.
+func Marshal(e exception) ([]byte, error) {
+	return xml.Marshal(&errorXML{
+		Xmlnsd:    "DAV",
+		Xmlnss:    "http://sabredav.org/ns",
+		Exception: codesEnum[e.code],
+		Message:   e.message,
+	})
+}

internal/http/services/owncloud/ocdav/propfind.go

@@ -55,6 +55,7 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string)
 	if depth == "" {
 		depth = "1"
 	}
+
 	// see https://tools.ietf.org/html/rfc4918#section-10.2
 	if depth != "0" && depth != "1" && depth != "infinity" {
 		log.Error().Msgf("invalid Depth header value %s", depth)
@@ -639,8 +640,12 @@ type propertyXML struct {
 
 // http://www.webdav.org/specs/rfc4918.html#ELEMENT_error
 type errorXML struct {
-	XMLName  xml.Name `xml:"d:error"`
-	InnerXML []byte   `xml:",innerxml"`
+	XMLName   xml.Name `xml:"d:error"`
+	Xmlnsd    string   `xml:"xmlns:d,attr"`
+	Xmlnss    string   `xml:"xmlns:s,attr"`
+	Exception string   `xml:"s:exception"`
+	Message   string   `xml:"s:message"`
+	InnerXML  []byte   `xml:",innerxml"`
 }
 
 var errInvalidPropfind = errors.New("webdav: invalid propfind")

tests/acceptance/expected-failures-on-OCIS-storage.txt

@@ -1067,10 +1067,6 @@ apiWebdavOperations/refuseAccess.feature:22
 apiWebdavOperations/refuseAccess.feature:33
 apiWebdavOperations/refuseAccess.feature:34
 #
-# https://github.com/owncloud/core/pull/38035 PROPFIND to https://localhost:9200/remote.php/dav/files gets an error 500 response
-#
-apiWebdavOperations/propfind.feature:5
-#
 # https://github.com/owncloud/ocis-reva/issues/39  REPORT request not implemented
 #
 apiWebdavOperations/search.feature:42

tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt

@@ -1038,10 +1038,6 @@ apiWebdavOperations/refuseAccess.feature:22
 apiWebdavOperations/refuseAccess.feature:33
 apiWebdavOperations/refuseAccess.feature:34
 #
-# https://github.com/owncloud/core/pull/38035 PROPFIND to https://localhost:9200/remote.php/dav/files gets an error 500 response
-#
-apiWebdavOperations/propfind.feature:5
-#
 # https://github.com/owncloud/ocis-reva/issues/39  REPORT request not implemented
 #
 apiWebdavOperations/search.feature:42

tests/acceptance/features/apiOcisSpecific/apiShareWebdavOperations-propfind.feature

@@ -6,4 +6,4 @@ Feature: PROPFIND
   Scenario: PROPFIND to "/remote.php/dav/files"
     Given user "Alice" has been created with default attributes and without skeleton files
     When user "Alice" requests "/remote.php/dav/files" with "PROPFIND" using basic auth
-    Then the HTTP status code should be "500"
+    Then the HTTP status code should be "405"