From 8a0ad8e5ca47590c00bcec818d868fef7a20a478 Mon Sep 17 00:00:00 2001
From: David Christofas <dchristofas@owncloud.com>
Date: Thu, 10 Mar 2022 14:45:15 +0100
Subject: [PATCH 1/5] implement propfinds for spaces trash-bin

---
 internal/http/services/owncloud/ocdav/dav.go  |  15 +-
 .../http/services/owncloud/ocdav/spaces.go    |  40 ++++-
 .../http/services/owncloud/ocdav/trashbin.go  | 142 ++++++++----------
 3 files changed, 107 insertions(+), 90 deletions(-)

diff --git a/internal/http/services/owncloud/ocdav/dav.go b/internal/http/services/owncloud/ocdav/dav.go
index babb166ca6..86f2f80a4d 100644
--- a/internal/http/services/owncloud/ocdav/dav.go
+++ b/internal/http/services/owncloud/ocdav/dav.go
@@ -27,7 +27,6 @@ import (
 	gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
-	rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
@@ -39,6 +38,10 @@ import (
 	"google.golang.org/grpc/metadata"
 )
 
+const (
+	_trashbinPath = "trash-bin"
+)
+
 type tokenStatInfoKey struct{}
 
 // DavHandler routes to the different sub handlers
@@ -172,7 +175,7 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
 			base := path.Join(ctx.Value(net.CtxKeyBaseURI).(string), "spaces")
 			ctx := context.WithValue(ctx, net.CtxKeyBaseURI, base)
 			r = r.WithContext(ctx)
-			h.SpacesHandler.Handler(s).ServeHTTP(w, r)
+			h.SpacesHandler.Handler(s, h.TrashbinHandler).ServeHTTP(w, r)
 		case "public-files":
 			base := path.Join(ctx.Value(net.CtxKeyBaseURI).(string), "public-files")
 			ctx = context.WithValue(ctx, net.CtxKeyBaseURI, base)
@@ -203,9 +206,9 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
 			case err != nil:
 				w.WriteHeader(http.StatusInternalServerError)
 				return
-			case res.Status.Code == rpcv1beta1.Code_CODE_PERMISSION_DENIED:
+			case res.Status.Code == rpc.Code_CODE_PERMISSION_DENIED:
 				fallthrough
-			case res.Status.Code == rpcv1beta1.Code_CODE_UNAUTHENTICATED:
+			case res.Status.Code == rpc.Code_CODE_UNAUTHENTICATED:
 				w.WriteHeader(http.StatusUnauthorized)
 				if hasValidBasicAuthHeader {
 					b, err := errors.Marshal(http.StatusUnauthorized, "Username or password was incorrect", "")
@@ -215,10 +218,10 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
 				b, err := errors.Marshal(http.StatusUnauthorized, "No 'Authorization: Basic' header found", "")
 				errors.HandleWebdavError(log, w, b, err)
 				return
-			case res.Status.Code == rpcv1beta1.Code_CODE_NOT_FOUND:
+			case res.Status.Code == rpc.Code_CODE_NOT_FOUND:
 				w.WriteHeader(http.StatusNotFound)
 				return
-			case res.Status.Code != rpcv1beta1.Code_CODE_OK:
+			case res.Status.Code != rpc.Code_CODE_OK:
 				w.WriteHeader(http.StatusInternalServerError)
 				return
 			}
diff --git a/internal/http/services/owncloud/ocdav/spaces.go b/internal/http/services/owncloud/ocdav/spaces.go
index 901083e452..d8055316d8 100644
--- a/internal/http/services/owncloud/ocdav/spaces.go
+++ b/internal/http/services/owncloud/ocdav/spaces.go
@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"path"
 
+	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind"
 	"github.com/cs3org/reva/v2/pkg/appctx"
@@ -44,7 +45,7 @@ func (h *SpacesHandler) init(c *Config) error {
 }
 
 // Handler handles requests
-func (h *SpacesHandler) Handler(s *svc) http.Handler {
+func (h *SpacesHandler) Handler(s *svc, trashbinHandler *TrashbinHandler) http.Handler {
 	config := s.Config()
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// ctx := r.Context()
@@ -55,8 +56,15 @@ func (h *SpacesHandler) Handler(s *svc) http.Handler {
 			return
 		}
 
+		var p string
+		p, r.URL.Path = router.ShiftPath(r.URL.Path)
+
 		var spaceID string
-		spaceID, r.URL.Path = router.ShiftPath(r.URL.Path)
+		if p == _trashbinPath {
+			h.handleSpacesTrashbin(w, r, s, trashbinHandler)
+		} else {
+			spaceID = p
+		}
 
 		if spaceID == "" {
 			// listing is disabled, no auth will change that
@@ -135,3 +143,31 @@ func (h *SpacesHandler) Handler(s *svc) http.Handler {
 		}
 	})
 }
+
+func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Request, s *svc, trashbinHandler *TrashbinHandler) {
+	var spaceID string
+	spaceID, r.URL.Path = router.ShiftPath(r.URL.Path)
+	if spaceID == "" {
+		// listing is disabled, no auth will change that
+		w.WriteHeader(http.StatusMethodNotAllowed)
+		return
+	}
+
+	ref := &provider.Reference{
+		ResourceId: &provider.ResourceId{
+			StorageId: spaceID,
+			OpaqueId:  spaceID,
+		},
+	}
+
+	var key string
+	key, r.URL.Path = router.ShiftPath(r.URL.Path)
+
+	switch r.Method {
+	case MethodPropfind:
+		trashbinHandler.listTrashbin(w, r, s, ref, path.Join(_trashbinPath, spaceID), key, r.URL.Path)
+	case http.MethodDelete:
+		// trashbinHandler.delete(w, r, s, )
+	}
+
+}
diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go
index 38cc799097..247763d858 100644
--- a/internal/http/services/owncloud/ocdav/trashbin.go
+++ b/internal/http/services/owncloud/ocdav/trashbin.go
@@ -43,7 +43,6 @@ import (
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
-	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/v2/pkg/rhttp/router"
 	"github.com/cs3org/reva/v2/pkg/utils"
 )
@@ -51,10 +50,12 @@ import (
 // TrashbinHandler handles trashbin requests
 type TrashbinHandler struct {
 	gatewaySvc string
+	namespace  string
 }
 
 func (h *TrashbinHandler) init(c *Config) error {
 	h.gatewaySvc = c.GatewaySvc
+	h.namespace = path.Join("/", c.FilesNamespace)
 	return nil
 }
 
@@ -71,20 +72,19 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 
 		var username string
 		username, r.URL.Path = router.ShiftPath(r.URL.Path)
-
 		if username == "" {
 			// listing is disabled, no auth will change that
 			w.WriteHeader(http.StatusMethodNotAllowed)
 			return
 		}
 
-		u, ok := ctxpkg.ContextGetUser(ctx)
+		user, ok := ctxpkg.ContextGetUser(ctx)
 		if !ok {
 			w.WriteHeader(http.StatusBadRequest)
 			return
 		}
-		if u.Username != username {
-			log.Debug().Str("username", username).Interface("user", u).Msg("trying to read another users trash")
+		if user.Username != username {
+			log.Debug().Str("username", username).Interface("user", user).Msg("trying to read another users trash")
 			// listing other users trash is forbidden, no auth will change that
 			w.WriteHeader(http.StatusUnauthorized)
 			b, err := errors.Marshal(http.StatusUnauthorized, "", "")
@@ -102,42 +102,47 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 			return
 		}
 
-		// key will be a base64 encoded cs3 path, it uniquely identifies a trash item & storage
-		var key string
-		key, r.URL.Path = router.ShiftPath(r.URL.Path)
-
-		// If the recycle bin corresponding to a speicific path is requested, use that.
-		// If not, we user the user home to route the request
-		basePath := r.URL.Query().Get("base_path")
-		if basePath == "" {
-			gc, err := pool.GetGatewayServiceClient(s.c.GatewaySvc)
-			if err != nil {
-				// TODO(jfd) how do we make the user aware that some storages are not available?
-				// opaque response property? Or a list of errors?
-				// add a recycle entry with the path to the storage that produced the error?
-				log.Error().Err(err).Msg("error getting gateway client")
-				w.WriteHeader(http.StatusInternalServerError)
-				return
-			}
+		useLoggedInUser := true
+		ns, newPath, err := s.ApplyLayout(ctx, h.namespace, useLoggedInUser, r.URL.Path)
+		if err != nil {
+			w.WriteHeader(http.StatusNotFound)
+			b, err := errors.Marshal(http.StatusNotFound, fmt.Sprintf("could not get storage for %s", r.URL.Path), "")
+			errors.HandleWebdavError(appctx.GetLogger(r.Context()), w, b, err)
+		}
+		r.URL.Path = newPath
 
-			getHomeRes, err := gc.GetHome(ctx, &provider.GetHomeRequest{})
-			if err != nil {
-				log.Error().Err(err).Msg("error calling GetHome")
-				w.WriteHeader(http.StatusInternalServerError)
-				return
-			}
-			if getHomeRes.Status.Code != rpc.Code_CODE_OK {
-				errors.HandleErrorStatus(log, w, getHomeRes.Status)
-				return
-			}
-			basePath = getHomeRes.Path
+		client, err := s.getClient()
+		if err != nil {
+			log.Error().Err(err).Msg("error getting grpc client")
+			w.WriteHeader(http.StatusInternalServerError)
+			return
 		}
 
-		if r.Method == MethodPropfind {
-			h.listTrashbin(w, r, s, u, basePath, key, r.URL.Path)
+		basePath := path.Join(ns, newPath)
+		space, rpcstatus, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, basePath)
+		if err != nil {
+			log.Error().Err(err).Str("path", basePath).Msg("failed to look up storage space")
+			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
-		if key != "" && r.Method == MethodMove {
+		if rpcstatus.Code != rpc.Code_CODE_OK {
+			errors.HandleErrorStatus(log, w, rpcstatus)
+			return
+		}
+		ref := spacelookup.MakeRelativeReference(space, ".", false)
+
+		// key will be a base64 encoded cs3 path, it uniquely identifies a trash item & storage
+		var key string
+		key, r.URL.Path = router.ShiftPath(r.URL.Path)
+
+		switch r.Method {
+		case MethodPropfind:
+			h.listTrashbin(w, r, s, ref, user.Username, key, r.URL.Path)
+		case MethodMove:
+			if key == "" {
+				http.Error(w, "501 Not implemented", http.StatusNotImplemented)
+				break
+			}
 			// find path in url relative to trash base
 			trashBase := ctx.Value(net.CtxKeyBaseURI).(string)
 			baseURI := path.Join(path.Dir(trashBase), "files", username)
@@ -154,20 +159,16 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 
 			log.Debug().Str("key", key).Str("dst", dst).Msg("restore")
 
-			h.restore(w, r, s, u, basePath, dst, key, r.URL.Path)
-			return
+			h.restore(w, r, s, user, basePath, dst, key, r.URL.Path)
+		case http.MethodDelete:
+			h.delete(w, r, s, user, basePath, key, r.URL.Path)
+		default:
+			http.Error(w, "501 Not implemented", http.StatusNotImplemented)
 		}
-
-		if r.Method == http.MethodDelete {
-			h.delete(w, r, s, u, basePath, key, r.URL.Path)
-			return
-		}
-
-		http.Error(w, "501 Not implemented", http.StatusNotImplemented)
 	})
 }
 
-func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, basePath, key, itemPath string) {
+func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, refBase, key, itemPath string) {
 	ctx, span := rtrace.Provider.Tracer("trash-bin").Start(r.Context(), "list_trashbin")
 	defer span.End()
 
@@ -181,15 +182,8 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 		return
 	}
 
-	client, err := s.getClient()
-	if err != nil {
-		sublog.Error().Err(err).Msg("error getting grpc client")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
-
 	if depth == net.DepthZero {
-		propRes, err := h.formatTrashPropfind(ctx, s, u, nil, nil)
+		propRes, err := h.formatTrashPropfind(ctx, s, refBase, nil, nil)
 		if err != nil {
 			sublog.Error().Err(err).Msg("error formatting propfind")
 			w.WriteHeader(http.StatusInternalServerError)
@@ -213,31 +207,15 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 		return
 	}
 
-	gc, err := pool.GetGatewayServiceClient(s.c.GatewaySvc)
-	if err != nil {
-		// TODO(jfd) how do we make the user aware that some storages are not available?
-		// opaque response property? Or a list of errors?
-		// add a recycle entry with the path to the storage that produced the error?
-		sublog.Error().Err(err).Msg("error getting gateway client")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
-
-	space, rpcstatus, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, basePath)
+	client, err := s.getClient()
 	if err != nil {
-		sublog.Error().Err(err).Str("path", basePath).Msg("failed to look up storage space")
+		sublog.Error().Err(err).Msg("error getting grpc client")
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
-	if rpcstatus.Code != rpc.Code_CODE_OK {
-		errors.HandleErrorStatus(&sublog, w, rpcstatus)
-		return
-	}
-	ref := spacelookup.MakeRelativeReference(space, basePath, false)
 
 	// ask gateway for recycle items
-	getRecycleRes, err := gc.ListRecycle(ctx, &provider.ListRecycleRequest{Ref: ref, Key: path.Join(key, itemPath)})
-
+	getRecycleRes, err := client.ListRecycle(ctx, &provider.ListRecycleRequest{Ref: ref, Key: path.Join(key, itemPath)})
 	if err != nil {
 		sublog.Error().Err(err).Msg("error calling ListRecycle")
 		w.WriteHeader(http.StatusInternalServerError)
@@ -264,7 +242,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 
 		for len(stack) > 0 {
 			key := stack[len(stack)-1]
-			getRecycleRes, err := gc.ListRecycle(ctx, &provider.ListRecycleRequest{Ref: ref, Key: key})
+			getRecycleRes, err := client.ListRecycle(ctx, &provider.ListRecycleRequest{Ref: ref, Key: key})
 			if err != nil {
 				sublog.Error().Err(err).Msg("error calling ListRecycle")
 				w.WriteHeader(http.StatusInternalServerError)
@@ -290,11 +268,11 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 	}
 
 	// TODO when using space based requests we should be able to get rid of this path unprefixing
-	for i := range items {
-		items[i].Ref.Path = strings.TrimPrefix(items[i].Ref.Path, basePath)
-	}
+	// for i := range items {
+	// 	items[i].Ref.Path = strings.TrimPrefix(items[i].Ref.Path, basePath)
+	// }
 
-	propRes, err := h.formatTrashPropfind(ctx, s, u, &pf, items)
+	propRes, err := h.formatTrashPropfind(ctx, s, refBase, &pf, items)
 	if err != nil {
 		sublog.Error().Err(err).Msg("error formatting propfind")
 		w.WriteHeader(http.StatusInternalServerError)
@@ -310,7 +288,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 	}
 }
 
-func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *userpb.User, pf *propfind.XML, items []*provider.RecycleItem) ([]byte, error) {
+func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, refBase string, pf *propfind.XML, items []*provider.RecycleItem) ([]byte, error) {
 	responses := make([]*propfind.ResponseXML, 0, len(items)+1)
 	// add trashbin dir . entry
 	responses = append(responses, &propfind.ResponseXML{
@@ -335,7 +313,7 @@ func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *us
 	})
 
 	for i := range items {
-		res, err := h.itemToPropResponse(ctx, s, u, pf, items[i])
+		res, err := h.itemToPropResponse(ctx, s, refBase, pf, items[i])
 		if err != nil {
 			return nil, err
 		}
@@ -357,10 +335,10 @@ func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *us
 // itemToPropResponse needs to create a listing that contains a key and destination
 // the key is the name of an entry in the trash listing
 // for now we need to limit trash to the users home, so we can expect all trash keys to have the home storage as the opaque id
-func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, u *userpb.User, pf *propfind.XML, item *provider.RecycleItem) (*propfind.ResponseXML, error) {
+func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, refBase string, pf *propfind.XML, item *provider.RecycleItem) (*propfind.ResponseXML, error) {
 
 	baseURI := ctx.Value(net.CtxKeyBaseURI).(string)
-	ref := path.Join(baseURI, u.Username, item.Key)
+	ref := path.Join(baseURI, refBase, item.Key)
 	if item.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER {
 		ref += "/"
 	}

From 50fb28933fe646cd29b84d71af1de1140ccc351f Mon Sep 17 00:00:00 2001
From: David Christofas <dchristofas@owncloud.com>
Date: Thu, 10 Mar 2022 15:36:55 +0100
Subject: [PATCH 2/5] implement webdav delete for spaces trash-bin

---
 .../http/services/owncloud/ocdav/spaces.go    |  4 +--
 .../http/services/owncloud/ocdav/trashbin.go  | 30 ++++++-------------
 pkg/storage/utils/decomposedfs/recycle.go     |  3 ++
 3 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/internal/http/services/owncloud/ocdav/spaces.go b/internal/http/services/owncloud/ocdav/spaces.go
index d8055316d8..db0333e482 100644
--- a/internal/http/services/owncloud/ocdav/spaces.go
+++ b/internal/http/services/owncloud/ocdav/spaces.go
@@ -62,6 +62,7 @@ func (h *SpacesHandler) Handler(s *svc, trashbinHandler *TrashbinHandler) http.H
 		var spaceID string
 		if p == _trashbinPath {
 			h.handleSpacesTrashbin(w, r, s, trashbinHandler)
+			return
 		} else {
 			spaceID = p
 		}
@@ -167,7 +168,6 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
 	case MethodPropfind:
 		trashbinHandler.listTrashbin(w, r, s, ref, path.Join(_trashbinPath, spaceID), key, r.URL.Path)
 	case http.MethodDelete:
-		// trashbinHandler.delete(w, r, s, )
+		trashbinHandler.delete(w, r, s, ref, key, r.URL.Path)
 	}
-
 }
diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go
index 247763d858..7c6d5de3b1 100644
--- a/internal/http/services/owncloud/ocdav/trashbin.go
+++ b/internal/http/services/owncloud/ocdav/trashbin.go
@@ -161,7 +161,7 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 
 			h.restore(w, r, s, user, basePath, dst, key, r.URL.Path)
 		case http.MethodDelete:
-			h.delete(w, r, s, user, basePath, key, r.URL.Path)
+			h.delete(w, r, s, ref, key, r.URL.Path)
 		default:
 			http.Error(w, "501 Not implemented", http.StatusNotImplemented)
 		}
@@ -385,7 +385,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, refBas
 			Status: "HTTP/1.1 404 Not Found",
 			Prop:   []*props.PropertyXML{},
 		}
-		size := fmt.Sprintf("%d", item.Size)
+		size := strconv.FormatUint(item.Size, 10)
 		for i := range pf.Prop {
 			switch pf.Prop[i].Space {
 			case net.NsOwncloud:
@@ -605,11 +605,11 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 }
 
 // delete has only a key
-func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, basePath, key, itemPath string) {
+func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, key, itemPath string) {
 	ctx, span := rtrace.Provider.Tracer("trash-bin").Start(r.Context(), "erase")
 	defer span.End()
 
-	sublog := appctx.GetLogger(ctx).With().Str("key", key).Logger()
+	sublog := appctx.GetLogger(ctx).With().Interface("reference", ref).Str("key", key).Str("item_path", itemPath).Logger()
 
 	client, err := s.getClient()
 	if err != nil {
@@ -618,22 +618,10 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc,
 		return
 	}
 
-	// set key as opaque id, the storageprovider will use it as the key for the
-	// storage drives  PurgeRecycleItem key call
-	space, status, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, basePath)
-	if err != nil {
-		sublog.Error().Err(err).Str("path", basePath).Msg("failed to look up storage space")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
-	if status.Code != rpc.Code_CODE_OK {
-		errors.HandleErrorStatus(&sublog, w, status)
-		return
-	}
-
+	trashPath := path.Join(key, itemPath)
 	req := &provider.PurgeRecycleRequest{
-		Ref: spacelookup.MakeRelativeReference(space, basePath, false),
-		Key: path.Join(key, itemPath),
+		Ref: ref,
+		Key: trashPath,
 	}
 
 	res, err := client.PurgeRecycle(ctx, req)
@@ -646,9 +634,9 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc,
 	case rpc.Code_CODE_OK:
 		w.WriteHeader(http.StatusNoContent)
 	case rpc.Code_CODE_NOT_FOUND:
-		sublog.Debug().Str("path", basePath).Str("key", key).Interface("status", res.Status).Msg("resource not found")
+		sublog.Debug().Interface("status", res.Status).Msg("resource not found")
 		w.WriteHeader(http.StatusConflict)
-		m := fmt.Sprintf("path %s not found", basePath)
+		m := fmt.Sprintf("path %s not found", trashPath)
 		b, err := errors.Marshal(http.StatusConflict, m, "")
 		errors.HandleWebdavError(&sublog, w, b, err)
 	case rpc.Code_CODE_PERMISSION_DENIED:
diff --git a/pkg/storage/utils/decomposedfs/recycle.go b/pkg/storage/utils/decomposedfs/recycle.go
index e1442e3c73..36e3fea073 100644
--- a/pkg/storage/utils/decomposedfs/recycle.go
+++ b/pkg/storage/utils/decomposedfs/recycle.go
@@ -295,6 +295,9 @@ func (fs *Decomposedfs) PurgeRecycleItem(ctx context.Context, ref *provider.Refe
 	}
 	rn, purgeFunc, err := fs.tp.PurgeRecycleItemFunc(ctx, ref.ResourceId.OpaqueId, key, relativePath)
 	if err != nil {
+		if errors.Is(err, iofs.ErrNotExist) {
+			return errtypes.NotFound(key)
+		}
 		return err
 	}
 

From 9d357cbb83c7f0b6e657b03f7bbcf089a57305a8 Mon Sep 17 00:00:00 2001
From: David Christofas <dchristofas@owncloud.com>
Date: Thu, 10 Mar 2022 15:58:02 +0100
Subject: [PATCH 3/5] refactor overwrite header parsing

---
 internal/http/services/owncloud/ocdav/copy.go | 24 ++++---------------
 internal/http/services/owncloud/ocdav/move.go | 17 +++++--------
 .../http/services/owncloud/ocdav/net/net.go   | 13 ++++++++++
 .../services/owncloud/ocdav/net/net_test.go   | 13 ++++++++++
 .../http/services/owncloud/ocdav/trashbin.go  | 14 ++++-------
 5 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go
index 351070ba77..9363c2d3bc 100644
--- a/internal/http/services/owncloud/ocdav/copy.go
+++ b/internal/http/services/owncloud/ocdav/copy.go
@@ -25,7 +25,6 @@ import (
 	"path"
 	"path/filepath"
 	"strconv"
-	"strings"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
@@ -480,7 +479,8 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, clie
 }
 
 func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, srcRef, dstRef *provider.Reference, log *zerolog.Logger) *copy {
-	overwrite, err := extractOverwrite(w, r)
+	oh := r.Header.Get(net.HeaderOverwrite)
+	overwrite, err := net.ParseOverwrite(oh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		m := fmt.Sprintf("Overwrite header is set to incorrect value %v", overwrite)
@@ -504,7 +504,7 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
 		depth = net.DepthInfinity
 	}
 
-	log.Debug().Str("overwrite", overwrite).Str("depth", depth.String()).Msg("copy")
+	log.Debug().Bool("overwrite", overwrite).Str("depth", depth.String()).Msg("copy")
 
 	client, err := s.getClient()
 	if err != nil {
@@ -548,8 +548,8 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
 	if dstStatRes.Status.Code == rpc.Code_CODE_OK {
 		successCode = http.StatusNoContent // 204 if target already existed, see https://tools.ietf.org/html/rfc4918#section-9.8.5
 
-		if overwrite == "F" {
-			log.Warn().Str("overwrite", overwrite).Msg("dst already exists")
+		if !overwrite {
+			log.Warn().Bool("overwrite", overwrite).Msg("dst already exists")
 			w.WriteHeader(http.StatusPreconditionFailed)
 			m := fmt.Sprintf("Could not overwrite Resource %v", dstRef.Path)
 			b, err := errors.Marshal(http.StatusPreconditionFailed, m, "")
@@ -598,17 +598,3 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
 
 	return &copy{source: srcRef, sourceInfo: srcStatRes.Info, depth: depth, successCode: successCode, destination: dstRef}
 }
-
-func extractOverwrite(w http.ResponseWriter, r *http.Request) (string, error) {
-	overwrite := r.Header.Get(net.HeaderOverwrite)
-	overwrite = strings.ToUpper(overwrite)
-	if overwrite == "" {
-		overwrite = "T"
-	}
-
-	if overwrite != "T" && overwrite != "F" {
-		return "", errInvalidValue
-	}
-
-	return overwrite, nil
-}
diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go
index b682acfbc2..7b0bb71eae 100644
--- a/internal/http/services/owncloud/ocdav/move.go
+++ b/internal/http/services/owncloud/ocdav/move.go
@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"net/http"
 	"path"
-	"strings"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
@@ -145,15 +144,11 @@ func (s *svc) handleSpacesMove(w http.ResponseWriter, r *http.Request, srcSpaceI
 }
 
 func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Request, src, dst *provider.Reference, log zerolog.Logger) {
-	overwrite := r.Header.Get(net.HeaderOverwrite)
-	log.Debug().Str("overwrite", overwrite).Msg("move")
+	oh := r.Header.Get(net.HeaderOverwrite)
+	log.Debug().Str("overwrite", oh).Msg("move")
 
-	overwrite = strings.ToUpper(overwrite)
-	if overwrite == "" {
-		overwrite = "T"
-	}
-
-	if overwrite != "T" && overwrite != "F" {
+	overwrite, err := net.ParseOverwrite(oh)
+	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
 	}
@@ -201,8 +196,8 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req
 	if dstStatRes.Status.Code == rpc.Code_CODE_OK {
 		successCode = http.StatusNoContent // 204 if target already existed, see https://tools.ietf.org/html/rfc4918#section-9.9.4
 
-		if overwrite == "F" {
-			log.Warn().Str("overwrite", overwrite).Msg("dst already exists")
+		if !overwrite {
+			log.Warn().Bool("overwrite", overwrite).Msg("dst already exists")
 			w.WriteHeader(http.StatusPreconditionFailed) // 412, see https://tools.ietf.org/html/rfc4918#section-9.9.4
 			return
 		}
diff --git a/internal/http/services/owncloud/ocdav/net/net.go b/internal/http/services/owncloud/ocdav/net/net.go
index 06472d607d..6c4af82287 100644
--- a/internal/http/services/owncloud/ocdav/net/net.go
+++ b/internal/http/services/owncloud/ocdav/net/net.go
@@ -113,3 +113,16 @@ func ParseDepth(s string) (Depth, error) {
 		return "", fmt.Errorf("invalid depth: %s", s)
 	}
 }
+
+// ParseOverwrite parses the overwrite header value defined in https://datatracker.ietf.org/doc/html/rfc4918#section-10.6
+// Valid values are "T" and "F". An empty string will be parse to true.
+func ParseOverwrite(s string) (bool, error) {
+	s = strings.ToUpper(s)
+	if s == "" {
+		s = "T"
+	}
+	if s != "T" && s != "F" {
+		return false, fmt.Errorf("invalid overwrite: %s", s)
+	}
+	return s == "T", nil
+}
diff --git a/internal/http/services/owncloud/ocdav/net/net_test.go b/internal/http/services/owncloud/ocdav/net/net_test.go
index ed40f19263..2d289fa84a 100644
--- a/internal/http/services/owncloud/ocdav/net/net_test.go
+++ b/internal/http/services/owncloud/ocdav/net/net_test.go
@@ -90,4 +90,17 @@ var _ = Describe("Net", func() {
 			Expect(medianDuration).To(BeNumerically("<", 10*time.Millisecond))
 		})
 	})
+
+	DescribeTable("TestParseOverwrite",
+		func(v string, expectSuccess bool, expectedValue bool) {
+			parsed, err := net.ParseOverwrite(v)
+			Expect(err == nil).To(Equal(expectSuccess))
+			Expect(parsed).To(Equal(expectedValue))
+		},
+		Entry("default", "", true, true),
+		Entry("T", "T", true, true),
+		Entry("t", "t", true, true),
+		Entry("F", "F", true, false),
+		Entry("f", "f", true, false),
+		Entry("invalid", "invalid", false, false))
 })
diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go
index 7c6d5de3b1..98557a48de 100644
--- a/internal/http/services/owncloud/ocdav/trashbin.go
+++ b/internal/http/services/owncloud/ocdav/trashbin.go
@@ -450,14 +450,10 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 
 	sublog := appctx.GetLogger(ctx).With().Logger()
 
-	overwrite := r.Header.Get(net.HeaderOverwrite)
+	oh := r.Header.Get(net.HeaderOverwrite)
 
-	overwrite = strings.ToUpper(overwrite)
-	if overwrite == "" {
-		overwrite = "T"
-	}
-
-	if overwrite != "T" && overwrite != "F" {
+	overwrite, err := net.ParseOverwrite(oh)
+	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
 	}
@@ -521,8 +517,8 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 	if dstStatRes.Status.Code == rpc.Code_CODE_OK {
 		successCode = http.StatusNoContent // 204 if target already existed, see https://tools.ietf.org/html/rfc4918#section-9.9.4
 
-		if overwrite != "T" {
-			sublog.Warn().Str("overwrite", overwrite).Msg("dst already exists")
+		if !overwrite {
+			sublog.Warn().Bool("overwrite", overwrite).Msg("dst already exists")
 			w.WriteHeader(http.StatusPreconditionFailed) // 412, see https://tools.ietf.org/html/rfc4918#section-9.9.4
 			b, err := errors.Marshal(
 				http.StatusPreconditionFailed,

From d192df5f48005f368d081963739d3b2aac8454ff Mon Sep 17 00:00:00 2001
From: David Christofas <dchristofas@owncloud.com>
Date: Thu, 10 Mar 2022 16:45:25 +0100
Subject: [PATCH 4/5] implement restore for spaces trashbin

---
 changelog/unreleased/spaces-trashbin.md       |  5 ++
 internal/http/services/owncloud/ocdav/copy.go |  9 ++-
 internal/http/services/owncloud/ocdav/move.go |  8 ++-
 .../http/services/owncloud/ocdav/net/net.go   | 34 +++++++++-
 .../services/owncloud/ocdav/net/net_test.go   | 12 ++++
 .../http/services/owncloud/ocdav/ocdav.go     | 25 --------
 .../services/owncloud/ocdav/ocdav_test.go     | 62 -------------------
 .../http/services/owncloud/ocdav/spaces.go    | 44 +++++++++----
 .../http/services/owncloud/ocdav/trashbin.go  | 53 ++++------------
 9 files changed, 106 insertions(+), 146 deletions(-)
 create mode 100644 changelog/unreleased/spaces-trashbin.md

diff --git a/changelog/unreleased/spaces-trashbin.md b/changelog/unreleased/spaces-trashbin.md
new file mode 100644
index 0000000000..1518f72edc
--- /dev/null
+++ b/changelog/unreleased/spaces-trashbin.md
@@ -0,0 +1,5 @@
+Enhancement: Add spaces aware trash-bin API
+
+Added the webdav trash-bin endpoint for spaces.
+
+https://github.com/cs3org/reva/pull/2628
diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go
index 9363c2d3bc..51ea05b438 100644
--- a/internal/http/services/owncloud/ocdav/copy.go
+++ b/internal/http/services/owncloud/ocdav/copy.go
@@ -55,7 +55,10 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string)
 	defer span.End()
 
 	src := path.Join(ns, r.URL.Path)
-	dst, err := extractDestination(r)
+
+	dh := r.Header.Get(net.HeaderDestination)
+	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
+	dst, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
@@ -274,7 +277,9 @@ func (s *svc) handleSpacesCopy(w http.ResponseWriter, r *http.Request, spaceID s
 	ctx, span := rtrace.Provider.Tracer("reva").Start(r.Context(), "spaces_copy")
 	defer span.End()
 
-	dst, err := extractDestination(r)
+	dh := r.Header.Get(net.HeaderDestination)
+	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
+	dst, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go
index 7b0bb71eae..0146cd5fa8 100644
--- a/internal/http/services/owncloud/ocdav/move.go
+++ b/internal/http/services/owncloud/ocdav/move.go
@@ -42,7 +42,9 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string)
 	defer span.End()
 
 	srcPath := path.Join(ns, r.URL.Path)
-	dstPath, err := extractDestination(r)
+	dh := r.Header.Get(net.HeaderDestination)
+	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
+	dstPath, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
@@ -98,7 +100,9 @@ func (s *svc) handleSpacesMove(w http.ResponseWriter, r *http.Request, srcSpaceI
 	ctx, span := rtrace.Provider.Tracer("ocdav").Start(r.Context(), "spaces_move")
 	defer span.End()
 
-	dst, err := extractDestination(r)
+	dh := r.Header.Get(net.HeaderDestination)
+	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
+	dst, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
diff --git a/internal/http/services/owncloud/ocdav/net/net.go b/internal/http/services/owncloud/ocdav/net/net.go
index 6c4af82287..95a772a604 100644
--- a/internal/http/services/owncloud/ocdav/net/net.go
+++ b/internal/http/services/owncloud/ocdav/net/net.go
@@ -20,8 +20,16 @@ package net
 
 import (
 	"fmt"
+	"net/url"
 	"regexp"
 	"strings"
+
+	"github.com/pkg/errors"
+)
+
+var (
+	// ErrInvalidHeaderValue defines an error which can occure when trying to parse a header value.
+	ErrInvalidHeaderValue = errors.New("invalid value")
 )
 
 type ctxKey int
@@ -110,7 +118,7 @@ func ParseDepth(s string) (Depth, error) {
 	case DepthInfinity.String():
 		return DepthInfinity, nil
 	default:
-		return "", fmt.Errorf("invalid depth: %s", s)
+		return "", errors.Wrapf(ErrInvalidHeaderValue, "invalid depth: %s", s)
 	}
 }
 
@@ -122,7 +130,29 @@ func ParseOverwrite(s string) (bool, error) {
 		s = "T"
 	}
 	if s != "T" && s != "F" {
-		return false, fmt.Errorf("invalid overwrite: %s", s)
+		return false, errors.Wrapf(ErrInvalidHeaderValue, "invalid overwrite: %s", s)
 	}
 	return s == "T", nil
 }
+
+// ParseDestination parses the destination header value defined in https://datatracker.ietf.org/doc/html/rfc4918#section-10.3
+// The returned path will be relative to the given baseURI.
+func ParseDestination(baseURI, s string) (string, error) {
+	if s == "" {
+		return "", errors.Wrap(ErrInvalidHeaderValue, "destination header is empty")
+	}
+	dstURL, err := url.ParseRequestURI(s)
+	if err != nil {
+		return "", errors.Wrap(ErrInvalidHeaderValue, err.Error())
+	}
+
+	// TODO check if path is on same storage, return 502 on problems, see https://tools.ietf.org/html/rfc4918#section-9.9.4
+	// TODO make request.php optional in destination header
+	// Strip the base URI from the destination. The destination might contain redirection prefixes which need to be handled
+	urlSplit := strings.Split(dstURL.Path, baseURI)
+	if len(urlSplit) != 2 {
+		return "", errors.Wrap(ErrInvalidHeaderValue, "destination path does not contain base URI")
+	}
+
+	return urlSplit[1], nil
+}
diff --git a/internal/http/services/owncloud/ocdav/net/net_test.go b/internal/http/services/owncloud/ocdav/net/net_test.go
index 2d289fa84a..779cf7e2f4 100644
--- a/internal/http/services/owncloud/ocdav/net/net_test.go
+++ b/internal/http/services/owncloud/ocdav/net/net_test.go
@@ -103,4 +103,16 @@ var _ = Describe("Net", func() {
 		Entry("F", "F", true, false),
 		Entry("f", "f", true, false),
 		Entry("invalid", "invalid", false, false))
+
+	DescribeTable("TestParseDestination",
+		func(baseURI, v string, expectSuccess bool, expectedValue string) {
+			parsed, err := net.ParseDestination(baseURI, v)
+			Expect(err == nil).To(Equal(expectSuccess))
+			Expect(parsed).To(Equal(expectedValue))
+		},
+		Entry("invalid1", "", "", false, ""),
+		Entry("invalid2", "baseURI", "", false, ""),
+		Entry("invalid3", "", "/dest/path", false, ""),
+		Entry("invalid4", "/foo", "/dest/path", false, ""),
+		Entry("valid", "/foo", "https://example.com/foo/dest/path", true, "/dest/path"))
 })
diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go
index 1b6594990e..b5e32daf9f 100644
--- a/internal/http/services/owncloud/ocdav/ocdav.go
+++ b/internal/http/services/owncloud/ocdav/ocdav.go
@@ -21,7 +21,6 @@ package ocdav
 import (
 	"context"
 	"net/http"
-	"net/url"
 	"path"
 	"strings"
 	"time"
@@ -42,13 +41,10 @@ import (
 	"github.com/cs3org/reva/v2/pkg/storage/favorite/registry"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/templates"
 	"github.com/mitchellh/mapstructure"
-	"github.com/pkg/errors"
 	"github.com/rs/zerolog"
 )
 
 var (
-	errInvalidValue = errors.New("invalid value")
-
 	nameRules = [...]nameRule{
 		nameNotEmpty{},
 		nameDoesNotContain{chars: "\f\r\n\\"},
@@ -339,24 +335,3 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) {
 		headers.Set("Strict-Transport-Security", "max-age=63072000")
 	}
 }
-
-func extractDestination(r *http.Request) (string, error) {
-	dstHeader := r.Header.Get(net.HeaderDestination)
-	if dstHeader == "" {
-		return "", errors.Wrap(errInvalidValue, "destination header is empty")
-	}
-	dstURL, err := url.ParseRequestURI(dstHeader)
-	if err != nil {
-		return "", errors.Wrap(errInvalidValue, err.Error())
-	}
-
-	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
-	// TODO check if path is on same storage, return 502 on problems, see https://tools.ietf.org/html/rfc4918#section-9.9.4
-	// Strip the base URI from the destination. The destination might contain redirection prefixes which need to be handled
-	urlSplit := strings.Split(dstURL.Path, baseURI)
-	if len(urlSplit) != 2 {
-		return "", errors.Wrap(errInvalidValue, "destination path does not contain base URI")
-	}
-
-	return urlSplit[1], nil
-}
diff --git a/internal/http/services/owncloud/ocdav/ocdav_test.go b/internal/http/services/owncloud/ocdav/ocdav_test.go
index d4b3f11014..13f47f4cc3 100644
--- a/internal/http/services/owncloud/ocdav/ocdav_test.go
+++ b/internal/http/services/owncloud/ocdav/ocdav_test.go
@@ -18,14 +18,9 @@
 package ocdav
 
 import (
-	"context"
-	"errors"
-	"net/http"
-	"net/http/httptest"
 	"testing"
 
 	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
-	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
 	"github.com/cs3org/reva/v2/pkg/utils/resourceid"
 )
 
@@ -38,63 +33,6 @@ func TestWrapResourceID(t *testing.T) {
 	}
 }
 
-func TestExtractDestination(t *testing.T) {
-	expected := "/dst"
-	request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil)
-	request.Header.Set(net.HeaderDestination, "https://example.org/remote.php/dav/dst")
-
-	ctx := context.WithValue(context.Background(), net.CtxKeyBaseURI, "remote.php/dav")
-	destination, err := extractDestination(request.WithContext(ctx))
-	if err != nil {
-		t.Errorf("Expected err to be nil got %s", err)
-	}
-
-	if destination != expected {
-		t.Errorf("Extracted destination is not expected, got %s want %s", destination, expected)
-	}
-}
-
-func TestExtractDestinationWithoutHeader(t *testing.T) {
-	request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil)
-
-	_, err := extractDestination(request)
-	if err == nil {
-		t.Errorf("Expected err to be nil got %s", err)
-	}
-
-	if !errors.Is(err, errInvalidValue) {
-		t.Errorf("Expected error invalid value, got %s", err)
-	}
-}
-
-func TestExtractDestinationWithInvalidDestination(t *testing.T) {
-	request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil)
-	request.Header.Set(net.HeaderDestination, "://example.org/remote.php/dav/dst")
-	_, err := extractDestination(request)
-	if err == nil {
-		t.Errorf("Expected err to be nil got %s", err)
-	}
-
-	if !errors.Is(err, errInvalidValue) {
-		t.Errorf("Expected error invalid value, got %s", err)
-	}
-}
-
-func TestExtractDestinationWithDestinationWrongBaseURI(t *testing.T) {
-	request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil)
-	request.Header.Set(net.HeaderDestination, "https://example.org/remote.php/dav/dst")
-
-	ctx := context.WithValue(context.Background(), net.CtxKeyBaseURI, "remote.php/webdav")
-	_, err := extractDestination(request.WithContext(ctx))
-	if err == nil {
-		t.Errorf("Expected err to be nil got %s", err)
-	}
-
-	if !errors.Is(err, errInvalidValue) {
-		t.Errorf("Expected error invalid value, got %s", err)
-	}
-}
-
 func TestNameNotEmptyRule(t *testing.T) {
 	tests := map[string]bool{
 		"":      false,
diff --git a/internal/http/services/owncloud/ocdav/spaces.go b/internal/http/services/owncloud/ocdav/spaces.go
index db0333e482..e803431e8d 100644
--- a/internal/http/services/owncloud/ocdav/spaces.go
+++ b/internal/http/services/owncloud/ocdav/spaces.go
@@ -24,6 +24,7 @@ import (
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
+	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
@@ -56,23 +57,21 @@ func (h *SpacesHandler) Handler(s *svc, trashbinHandler *TrashbinHandler) http.H
 			return
 		}
 
-		var p string
-		p, r.URL.Path = router.ShiftPath(r.URL.Path)
-
-		var spaceID string
-		if p == _trashbinPath {
-			h.handleSpacesTrashbin(w, r, s, trashbinHandler)
+		var segment string
+		segment, r.URL.Path = router.ShiftPath(r.URL.Path)
+		if segment == "" {
+			// listing is disabled, no auth will change that
+			w.WriteHeader(http.StatusMethodNotAllowed)
 			return
-		} else {
-			spaceID = p
 		}
 
-		if spaceID == "" {
-			// listing is disabled, no auth will change that
-			w.WriteHeader(http.StatusMethodNotAllowed)
+		if segment == _trashbinPath {
+			h.handleSpacesTrashbin(w, r, s, trashbinHandler)
 			return
 		}
 
+		spaceID := segment
+
 		switch r.Method {
 		case MethodPropfind:
 			p := propfind.NewHandler(config.PublicURL, func() (propfind.GatewayClient, error) {
@@ -146,6 +145,9 @@ func (h *SpacesHandler) Handler(s *svc, trashbinHandler *TrashbinHandler) http.H
 }
 
 func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Request, s *svc, trashbinHandler *TrashbinHandler) {
+	ctx := r.Context()
+	log := appctx.GetLogger(ctx)
+
 	var spaceID string
 	spaceID, r.URL.Path = router.ShiftPath(r.URL.Path)
 	if spaceID == "" {
@@ -167,7 +169,27 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
 	switch r.Method {
 	case MethodPropfind:
 		trashbinHandler.listTrashbin(w, r, s, ref, path.Join(_trashbinPath, spaceID), key, r.URL.Path)
+	case MethodMove:
+		if key == "" {
+			http.Error(w, "501 Not implemented", http.StatusNotImplemented)
+			break
+		}
+		// find path in url relative to trash base
+		baseURI := ctx.Value(net.CtxKeyBaseURI).(string)
+		baseURI = path.Join(baseURI, spaceID)
+
+		dh := r.Header.Get(net.HeaderDestination)
+		dst, err := net.ParseDestination(baseURI, dh)
+		if err != nil {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		log.Debug().Str("key", key).Str("path", r.URL.Path).Str("dst", dst).Msg("spaces restore")
+		trashbinHandler.restore(w, r, s, ref, dst, key, r.URL.Path)
 	case http.MethodDelete:
 		trashbinHandler.delete(w, r, s, ref, key, r.URL.Path)
+	default:
+		http.Error(w, "501 Not implemented", http.StatusNotImplemented)
 	}
 }
diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go
index 98557a48de..2aab1c7440 100644
--- a/internal/http/services/owncloud/ocdav/trashbin.go
+++ b/internal/http/services/owncloud/ocdav/trashbin.go
@@ -25,7 +25,6 @@ import (
 	"fmt"
 	"net/http"
 	"path"
-	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
@@ -38,7 +37,6 @@ import (
 	rtrace "github.com/cs3org/reva/v2/pkg/trace"
 	"github.com/cs3org/reva/v2/pkg/utils/resourceid"
 
-	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/appctx"
@@ -146,20 +144,16 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 			// find path in url relative to trash base
 			trashBase := ctx.Value(net.CtxKeyBaseURI).(string)
 			baseURI := path.Join(path.Dir(trashBase), "files", username)
-			ctx = context.WithValue(ctx, net.CtxKeyBaseURI, baseURI)
-			r = r.WithContext(ctx)
 
-			// TODO make request.php optional in destination header
-			dst, err := extractDestination(r)
+			dh := r.Header.Get(net.HeaderDestination)
+			dst, err := net.ParseDestination(baseURI, dh)
 			if err != nil {
 				w.WriteHeader(http.StatusBadRequest)
 				return
 			}
-			dst = path.Join(basePath, dst)
 
 			log.Debug().Str("key", key).Str("dst", dst).Msg("restore")
-
-			h.restore(w, r, s, user, basePath, dst, key, r.URL.Path)
+			h.restore(w, r, s, ref, dst, key, r.URL.Path)
 		case http.MethodDelete:
 			h.delete(w, r, s, ref, key, r.URL.Path)
 		default:
@@ -444,7 +438,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, refBas
 	return &response, nil
 }
 
-func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, basePath, dst, key, itemPath string) {
+func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, dst, key, itemPath string) {
 	ctx, span := rtrace.Provider.Tracer("trash-bin").Start(r.Context(), "restore")
 	defer span.End()
 
@@ -464,22 +458,11 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
-	space, rpcstatus, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, dst)
-	if err != nil {
-		sublog.Error().Err(err).Str("path", basePath).Msg("failed to look up storage space")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
-	if rpcstatus.Code != rpc.Code_CODE_OK {
-		errors.HandleErrorStatus(&sublog, w, rpcstatus)
-		return
-	}
-	dstRef := spacelookup.MakeRelativeReference(space, dst, false)
 
-	dstStatReq := &provider.StatRequest{
-		Ref: dstRef,
-	}
+	dstRef := ref
+	dstRef.Path = utils.MakeRelativePath(dst)
 
+	dstStatReq := &provider.StatRequest{Ref: dstRef}
 	dstStatRes, err := client.Stat(ctx, dstStatReq)
 	if err != nil {
 		sublog.Error().Err(err).Msg("error sending grpc stat request")
@@ -495,9 +478,9 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 	// Restoring to a non-existent location is not supported by the WebDAV spec. The following block ensures the target
 	// restore location exists, and if it doesn't returns a conflict error code.
 	if dstStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND && isNested(dst) {
-		parentStatReq := &provider.StatRequest{
-			Ref: spacelookup.MakeRelativeReference(space, filepath.Dir(dst), false),
-		}
+		parentRef := ref
+		parentRef.Path = utils.MakeRelativePath(path.Dir(dst))
+		parentStatReq := &provider.StatRequest{Ref: parentRef}
 
 		parentStatResponse, err := client.Stat(ctx, parentStatReq)
 		if err != nil {
@@ -543,22 +526,8 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 		}
 	}
 
-	sourceSpace, rpcstatus, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, basePath)
-	if err != nil {
-		sublog.Error().Err(err).Str("path", basePath).Msg("failed to look up storage space")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
-	if rpcstatus.Code != rpc.Code_CODE_OK {
-		errors.HandleErrorStatus(&sublog, w, rpcstatus)
-		return
-	}
 	req := &provider.RestoreRecycleItemRequest{
-		// use the target path to find the storage provider
-		// this means we can only undelete on the same storage, not to a different folder
-		// use the key which is prefixed with the StoragePath to lookup the correct storage ...
-		// TODO currently limited to the home storage
-		Ref:        spacelookup.MakeRelativeReference(sourceSpace, basePath, false),
+		Ref:        ref,
 		Key:        path.Join(key, itemPath),
 		RestoreRef: dstRef,
 	}

From 73b2aa082b3d4bf2f3ad9accb52bc259797d26ac Mon Sep 17 00:00:00 2001
From: David Christofas <dchristofas@owncloud.com>
Date: Fri, 11 Mar 2022 12:18:22 +0100
Subject: [PATCH 5/5] fix restore for nested spaces

There can be nested spaces e.g. the 'Shares Jail' inside a users space
---
 .../http/services/owncloud/ocdav/net/net.go   |  1 -
 .../services/owncloud/ocdav/net/net_test.go   |  2 -
 .../http/services/owncloud/ocdav/spaces.go    |  8 +++-
 .../http/services/owncloud/ocdav/trashbin.go  | 39 +++++++++++--------
 4 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/internal/http/services/owncloud/ocdav/net/net.go b/internal/http/services/owncloud/ocdav/net/net.go
index 95a772a604..586c8c7cc4 100644
--- a/internal/http/services/owncloud/ocdav/net/net.go
+++ b/internal/http/services/owncloud/ocdav/net/net.go
@@ -125,7 +125,6 @@ func ParseDepth(s string) (Depth, error) {
 // ParseOverwrite parses the overwrite header value defined in https://datatracker.ietf.org/doc/html/rfc4918#section-10.6
 // Valid values are "T" and "F". An empty string will be parse to true.
 func ParseOverwrite(s string) (bool, error) {
-	s = strings.ToUpper(s)
 	if s == "" {
 		s = "T"
 	}
diff --git a/internal/http/services/owncloud/ocdav/net/net_test.go b/internal/http/services/owncloud/ocdav/net/net_test.go
index 779cf7e2f4..24ee8da430 100644
--- a/internal/http/services/owncloud/ocdav/net/net_test.go
+++ b/internal/http/services/owncloud/ocdav/net/net_test.go
@@ -99,9 +99,7 @@ var _ = Describe("Net", func() {
 		},
 		Entry("default", "", true, true),
 		Entry("T", "T", true, true),
-		Entry("t", "t", true, true),
 		Entry("F", "F", true, false),
-		Entry("f", "f", true, false),
 		Entry("invalid", "invalid", false, false))
 
 	DescribeTable("TestParseDestination",
diff --git a/internal/http/services/owncloud/ocdav/spaces.go b/internal/http/services/owncloud/ocdav/spaces.go
index e803431e8d..44b53dff44 100644
--- a/internal/http/services/owncloud/ocdav/spaces.go
+++ b/internal/http/services/owncloud/ocdav/spaces.go
@@ -29,6 +29,7 @@ import (
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/v2/pkg/rhttp/router"
+	"github.com/cs3org/reva/v2/pkg/utils"
 )
 
 // SpacesHandler handles trashbin requests
@@ -184,9 +185,12 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
 			w.WriteHeader(http.StatusBadRequest)
 			return
 		}
-
 		log.Debug().Str("key", key).Str("path", r.URL.Path).Str("dst", dst).Msg("spaces restore")
-		trashbinHandler.restore(w, r, s, ref, dst, key, r.URL.Path)
+
+		dstRef := ref
+		dstRef.Path = utils.MakeRelativePath(dst)
+
+		trashbinHandler.restore(w, r, s, ref, dstRef, key, r.URL.Path)
 	case http.MethodDelete:
 		trashbinHandler.delete(w, r, s, ref, key, r.URL.Path)
 	default:
diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go
index 2aab1c7440..bf1e970907 100644
--- a/internal/http/services/owncloud/ocdav/trashbin.go
+++ b/internal/http/services/owncloud/ocdav/trashbin.go
@@ -152,8 +152,22 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 				return
 			}
 
+			p := path.Join(ns, dst)
+			// The destination can be in another space. E.g. the 'Shares Jail'.
+			space, rpcstatus, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, p)
+			if err != nil {
+				log.Error().Err(err).Str("path", p).Msg("failed to look up destination storage space")
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			if rpcstatus.Code != rpc.Code_CODE_OK {
+				errors.HandleErrorStatus(log, w, rpcstatus)
+				return
+			}
+			dstRef := spacelookup.MakeRelativeReference(space, p, false)
+
 			log.Debug().Str("key", key).Str("dst", dst).Msg("restore")
-			h.restore(w, r, s, ref, dst, key, r.URL.Path)
+			h.restore(w, r, s, ref, dstRef, key, r.URL.Path)
 		case http.MethodDelete:
 			h.delete(w, r, s, ref, key, r.URL.Path)
 		default:
@@ -261,11 +275,6 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 		}
 	}
 
-	// TODO when using space based requests we should be able to get rid of this path unprefixing
-	// for i := range items {
-	// 	items[i].Ref.Path = strings.TrimPrefix(items[i].Ref.Path, basePath)
-	// }
-
 	propRes, err := h.formatTrashPropfind(ctx, s, refBase, &pf, items)
 	if err != nil {
 		sublog.Error().Err(err).Msg("error formatting propfind")
@@ -438,7 +447,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, refBas
 	return &response, nil
 }
 
-func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, dst, key, itemPath string) {
+func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, ref, dst *provider.Reference, key, itemPath string) {
 	ctx, span := rtrace.Provider.Tracer("trash-bin").Start(r.Context(), "restore")
 	defer span.End()
 
@@ -459,10 +468,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 		return
 	}
 
-	dstRef := ref
-	dstRef.Path = utils.MakeRelativePath(dst)
-
-	dstStatReq := &provider.StatRequest{Ref: dstRef}
+	dstStatReq := &provider.StatRequest{Ref: dst}
 	dstStatRes, err := client.Stat(ctx, dstStatReq)
 	if err != nil {
 		sublog.Error().Err(err).Msg("error sending grpc stat request")
@@ -477,9 +483,8 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 
 	// Restoring to a non-existent location is not supported by the WebDAV spec. The following block ensures the target
 	// restore location exists, and if it doesn't returns a conflict error code.
-	if dstStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND && isNested(dst) {
-		parentRef := ref
-		parentRef.Path = utils.MakeRelativePath(path.Dir(dst))
+	if dstStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND && isNested(dst.Path) {
+		parentRef := &provider.Reference{ResourceId: dst.ResourceId, Path: utils.MakeRelativePath(path.Dir(dst.Path))}
 		parentStatReq := &provider.StatRequest{Ref: parentRef}
 
 		parentStatResponse, err := client.Stat(ctx, parentStatReq)
@@ -512,7 +517,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 			return
 		}
 		// delete existing tree
-		delReq := &provider.DeleteRequest{Ref: dstRef}
+		delReq := &provider.DeleteRequest{Ref: dst}
 		delRes, err := client.Delete(ctx, delReq)
 		if err != nil {
 			sublog.Error().Err(err).Msg("error sending grpc delete request")
@@ -529,7 +534,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 	req := &provider.RestoreRecycleItemRequest{
 		Ref:        ref,
 		Key:        path.Join(key, itemPath),
-		RestoreRef: dstRef,
+		RestoreRef: dst,
 	}
 
 	res, err := client.RestoreRecycleItem(ctx, req)
@@ -621,5 +626,5 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc,
 
 func isNested(p string) bool {
 	dir, _ := path.Split(p)
-	return dir != "/"
+	return dir != "/" && dir != "./"
 }