allow listing directory trash items by key

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

Author: butonic

changelog/unreleased/trash-listing-bykey-fixes.md

@@ -0,0 +1,9 @@
+Bugfix: Allow listing directory trash items by key
+
+The storageprovider now passes on the key without inventing a `/` as the relative path when it was not present at the end of the key. This allows differentiating requests that want to get the trash item of a folder itself (where the relative path is empty) or listing the children of a folder in the trash (where the relative path at least starts with a `/`).
+
+We also fixed the `/dav/spaces` endpoint to not invent a `/` at the end of URLs to allow clients to actually make these different requests.
+
+As a byproduct we now return the size of trashed items.
+
+https://github.com/cs3org/reva/pull/4818

internal/grpc/services/storageprovider/storageprovider.go

@@ -27,6 +27,7 @@ import (
 	"path"
 	"sort"
 	"strconv"
+	"strings"
 	"time"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
@@ -880,8 +881,11 @@ func (s *Service) ListRecycleStream(req *provider.ListRecycleStreamRequest, ss p
 	ctx := ss.Context()
 	log := appctx.GetLogger(ctx)
 
-	key, itemPath := router.ShiftPath(req.Key)
-	items, err := s.Storage.ListRecycle(ctx, req.Ref, key, itemPath)
+	// if no slash is present in the key, do not pass a relative path to the storage
+	// when a path is passed to the storage, it will list the contents of the directory
+	key, relativePath := splitKeyAndPath(req.GetKey())
+	items, err := s.Storage.ListRecycle(ctx, req.Ref, key, relativePath)
+
 	if err != nil {
 		var st *rpc.Status
 		switch err.(type) {
@@ -924,8 +928,10 @@ func (s *Service) ListRecycleStream(req *provider.ListRecycleStreamRequest, ss p
 }
 
 func (s *Service) ListRecycle(ctx context.Context, req *provider.ListRecycleRequest) (*provider.ListRecycleResponse, error) {
-	key, itemPath := router.ShiftPath(req.Key)
-	items, err := s.Storage.ListRecycle(ctx, req.Ref, key, itemPath)
+	// if no slash is present in the key, do not pass a relative path to the storage
+	// when a path is passed to the storage, it will list the contents of the directory
+	key, relativePath := splitKeyAndPath(req.GetKey())
+	items, err := s.Storage.ListRecycle(ctx, req.Ref, key, relativePath)
 	if err != nil {
 		var st *rpc.Status
 		switch err.(type) {
@@ -962,8 +968,8 @@ func (s *Service) RestoreRecycleItem(ctx context.Context, req *provider.RestoreR
 	ctx = ctxpkg.ContextSetLockID(ctx, req.LockId)
 
 	// TODO(labkode): CRITICAL: fill recycle info with storage provider.
-	key, itemPath := router.ShiftPath(req.Key)
-	err := s.Storage.RestoreRecycleItem(ctx, req.Ref, key, itemPath, req.RestoreRef)
+	key, relativePath := splitKeyAndPath(req.GetKey())
+	err := s.Storage.RestoreRecycleItem(ctx, req.Ref, key, relativePath, req.RestoreRef)
 
 	res := &provider.RestoreRecycleItemResponse{
 		Status: status.NewStatusFromErrType(ctx, "restore recycle item", err),
@@ -980,9 +986,9 @@ func (s *Service) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleRe
 	}
 
 	// if a key was sent as opaque id purge only that item
-	key, itemPath := router.ShiftPath(req.Key)
+	key, relativePath := splitKeyAndPath(req.GetKey())
 	if key != "" {
-		if err := s.Storage.PurgeRecycleItem(ctx, req.Ref, key, itemPath); err != nil {
+		if err := s.Storage.PurgeRecycleItem(ctx, req.Ref, key, relativePath); err != nil {
 			st := status.NewStatusFromErrType(ctx, "error purging recycle item", err)
 			appctx.GetLogger(ctx).
 				Error().
@@ -1313,3 +1319,12 @@ func canLockPublicShare(ctx context.Context) bool {
 	psr := utils.ReadPlainFromOpaque(u.Opaque, "public-share-role")
 	return psr == "" || psr == conversions.RoleEditor
 }
+
+// splitKeyAndPath splits a key into a root and a relative path
+func splitKeyAndPath(key string) (string, string) {
+	root, relativePath := router.ShiftPath(key)
+	if relativePath == "/" && !strings.HasSuffix(key, "/") {
+		relativePath = ""
+	}
+	return root, relativePath
+}

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

@@ -21,6 +21,7 @@ package ocdav
 import (
 	"net/http"
 	"path"
+	"strings"
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/config"
@@ -132,8 +133,7 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
 	ctx := r.Context()
 	log := appctx.GetLogger(ctx)
 
-	var spaceID string
-	spaceID, r.URL.Path = router.ShiftPath(r.URL.Path)
+	spaceID, key := splitSpaceAndKey(r.URL.Path)
 	if spaceID == "" {
 		// listing is disabled, no auth will change that
 		w.WriteHeader(http.StatusMethodNotAllowed)
@@ -146,12 +146,9 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	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)
+		trashbinHandler.listTrashbin(w, r, s, &ref, path.Join(_trashbinPath, spaceID), key)
 	case MethodMove:
 		if key == "" {
 			http.Error(w, "501 Not implemented", http.StatusNotImplemented)
@@ -167,15 +164,25 @@ 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")
+		log.Debug().Str("key", key).Str("dst", dst).Msg("spaces restore")
 
 		dstRef := proto.Clone(&ref).(*provider.Reference)
 		dstRef.Path = utils.MakeRelativePath(dst)
 
-		trashbinHandler.restore(w, r, s, &ref, dstRef, key, r.URL.Path)
+		trashbinHandler.restore(w, r, s, &ref, dstRef, key)
 	case http.MethodDelete:
-		trashbinHandler.delete(w, r, s, &ref, key, r.URL.Path)
+		trashbinHandler.delete(w, r, s, &ref, key)
 	default:
 		http.Error(w, "501 Not implemented", http.StatusNotImplemented)
 	}
 }
+
+func splitSpaceAndKey(p string) (space, key string) {
+	p = strings.TrimPrefix(p, "/")
+	parts := strings.SplitN(p, "/", 2)
+	space = parts[0]
+	if len(parts) > 1 {
+		key = parts[1]
+	}
+	return
+}

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

@@ -131,13 +131,12 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 		}
 		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)
+		// key will be a base64 encoded cs3 path, it uniquely identifies a trash item with an opaque id and an optional path
+		key := r.URL.Path
 
 		switch r.Method {
 		case MethodPropfind:
-			h.listTrashbin(w, r, s, ref, user.Username, key, r.URL.Path)
+			h.listTrashbin(w, r, s, ref, user.Username, key)
 		case MethodMove:
 			if key == "" {
 				http.Error(w, "501 Not implemented", http.StatusNotImplemented)
@@ -172,16 +171,16 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 			dstRef := spacelookup.MakeRelativeReference(space, p, false)
 
 			log.Debug().Str("key", key).Str("dst", dst).Msg("restore")
-			h.restore(w, r, s, ref, dstRef, key, r.URL.Path)
+			h.restore(w, r, s, ref, dstRef, key)
 		case http.MethodDelete:
-			h.delete(w, r, s, ref, key, r.URL.Path)
+			h.delete(w, r, s, ref, key)
 		default:
 			http.Error(w, "501 Not implemented", http.StatusNotImplemented)
 		}
 	})
 }
 
-func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, refBase, key, itemPath string) {
+func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, refBase, key string) {
 	ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "list_trashbin")
 	defer span.End()
 
@@ -214,7 +213,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 	}
 
 	if depth == net.DepthZero {
-		rootHref := path.Join(refBase, key, itemPath)
+		rootHref := path.Join(refBase, key)
 		propRes, err := h.formatTrashPropfind(ctx, s, ref.ResourceId.SpaceId, refBase, rootHref, nil, nil)
 		if err != nil {
 			sublog.Error().Err(err).Msg("error formatting propfind")
@@ -246,7 +245,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 		return
 	}
 	// ask gateway for recycle items
-	getRecycleRes, err := client.ListRecycle(ctx, &provider.ListRecycleRequest{Ref: ref, Key: path.Join(key, itemPath)})
+	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)
@@ -304,7 +303,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
 		}
 	}
 
-	rootHref := path.Join(refBase, key, itemPath)
+	rootHref := path.Join(refBase, key)
 	propRes, err := h.formatTrashPropfind(ctx, s, ref.ResourceId.SpaceId, refBase, rootHref, &pf, items)
 	if err != nil {
 		sublog.Error().Err(err).Msg("error formatting propfind")
@@ -480,7 +479,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, spaceI
 	return &response, nil
 }
 
-func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, ref, dst *provider.Reference, key, itemPath string) {
+func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, ref, dst *provider.Reference, key string) {
 	ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "restore")
 	defer span.End()
 
@@ -566,7 +565,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 
 	req := &provider.RestoreRecycleItemRequest{
 		Ref:        ref,
-		Key:        path.Join(key, itemPath),
+		Key:        key,
 		RestoreRef: dst,
 	}
 
@@ -608,16 +607,15 @@ 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, ref *provider.Reference, key, itemPath string) {
+func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, key string) {
 	ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "erase")
 	defer span.End()
 
-	sublog := appctx.GetLogger(ctx).With().Interface("reference", ref).Str("key", key).Str("item_path", itemPath).Logger()
+	sublog := appctx.GetLogger(ctx).With().Interface("reference", ref).Str("key", key).Logger()
 
-	trashPath := path.Join(key, itemPath)
 	req := &provider.PurgeRecycleRequest{
 		Ref: ref,
-		Key: trashPath,
+		Key: key,
 	}
 
 	client, err := s.gatewaySelector.Next()
@@ -638,7 +636,7 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc,
 	case rpc.Code_CODE_NOT_FOUND:
 		sublog.Debug().Interface("status", res.Status).Msg("resource not found")
 		w.WriteHeader(http.StatusConflict)
-		m := fmt.Sprintf("path %s not found", trashPath)
+		m := fmt.Sprintf("key %s not found", key)
 		b, err := errors.Marshal(http.StatusConflict, m, "", "")
 		errors.HandleWebdavError(&sublog, w, b, err)
 	case rpc.Code_CODE_PERMISSION_DENIED: