Skip to content

Commit

Permalink
allow listing directory trash items by key
Browse files Browse the repository at this point in the history 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
  • Loading branch information
@butonic
butonic committed Aug 22, 2024
1 parent 774910f commit 6100655
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 134 deletions.
9 changes: 9 additions & 0 deletions changelog/unreleased/trash-listing-bykey-fixes.md
View file
Original file line number Diff line number Diff line change
@@ -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
31 changes: 23 additions & 8 deletions internal/grpc/services/storageprovider/storageprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"path"
"sort"
"strconv"
"strings"
"time"

rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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),
Expand All @@ -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().
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion internal/http/services/owncloud/ocdav/dav.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
http.Redirect(w, r, rUrl, http.StatusTemporaryRedirect)
return
}
log.Debug().Str("token", token).Interface("status", res.Status).Msg("resource id not found")
log.Debug().Str("token", token).Interface("status", psRes.Status).Msg("resource id not found")
w.WriteHeader(http.StatusNotFound)
return
}
Expand Down
25 changes: 16 additions & 9 deletions internal/http/services/owncloud/ocdav/spaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
}
98 changes: 41 additions & 57 deletions internal/http/services/owncloud/ocdav/trashbin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
"github.com/cs3org/reva/v2/pkg/appctx"
ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
rstatus "github.com/cs3org/reva/v2/pkg/rgrpc/status"
"github.com/cs3org/reva/v2/pkg/rhttp/router"
"github.com/cs3org/reva/v2/pkg/utils"
semconv "go.opentelemetry.io/otel/semconv/v1.20.0"
)
Expand Down Expand Up @@ -74,7 +73,7 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
}

var username string
username, r.URL.Path = router.ShiftPath(r.URL.Path)
username, r.URL.Path = splitSpaceAndKey(r.URL.Path)
if username == "" {
// listing is disabled, no auth will change that
w.WriteHeader(http.StatusMethodNotAllowed)
Expand Down Expand Up @@ -131,13 +130,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)
Expand Down Expand Up @@ -172,16 +170,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()

Expand Down Expand Up @@ -213,23 +211,9 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
return
}

if depth == net.DepthZero {
rootHref := path.Join(refBase, key, itemPath)
propRes, err := h.formatTrashPropfind(ctx, s, ref.ResourceId.SpaceId, refBase, rootHref, nil, nil)
if err != nil {
sublog.Error().Err(err).Msg("error formatting propfind")
w.WriteHeader(http.StatusInternalServerError)
return
}
w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol")
w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8")
w.WriteHeader(http.StatusMultiStatus)
_, err = w.Write(propRes)
if err != nil {
sublog.Error().Err(err).Msg("error writing body")
return
}
return
if key != "" && depth == net.DepthOne && !strings.HasSuffix(key, "/") {
// when a key is provided and the depth is 1 we need to append a / to the key to list the children
key += "/"
}

pf, status, err := propfind.ReadPropfind(r.Body)
Expand All @@ -246,7 +230,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)
Expand Down Expand Up @@ -304,8 +288,8 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
}
}

rootHref := path.Join(refBase, key, itemPath)
propRes, err := h.formatTrashPropfind(ctx, s, ref.ResourceId.SpaceId, refBase, rootHref, &pf, items)
rootHref := path.Join(refBase, key)
propRes, err := h.formatTrashPropfind(ctx, s, ref.ResourceId.SpaceId, refBase, rootHref, &pf, items, depth != net.DepthZero)
if err != nil {
sublog.Error().Err(err).Msg("error formatting propfind")
w.WriteHeader(http.StatusInternalServerError)
Expand All @@ -321,29 +305,30 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
}
}

func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, spaceID, refBase, rootHref string, pf *propfind.XML, items []*provider.RecycleItem) ([]byte, error) {
func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, spaceID, refBase, rootHref string, pf *propfind.XML, items []*provider.RecycleItem, fakeRoot bool) ([]byte, error) {
responses := make([]*propfind.ResponseXML, 0, len(items)+1)
// add trashbin dir . entry
responses = append(responses, &propfind.ResponseXML{
Href: net.EncodePath(path.Join(ctx.Value(net.CtxKeyBaseURI).(string), rootHref) + "/"), // url encode response.Href TODO
Propstat: []propfind.PropstatXML{
{
Status: "HTTP/1.1 200 OK",
Prop: []prop.PropertyXML{
prop.Raw("d:resourcetype", "<d:collection/>"),
if fakeRoot {
responses = append(responses, &propfind.ResponseXML{
Href: net.EncodePath(path.Join(ctx.Value(net.CtxKeyBaseURI).(string), rootHref) + "/"), // url encode response.Href TODO
Propstat: []propfind.PropstatXML{
{
Status: "HTTP/1.1 200 OK",
Prop: []prop.PropertyXML{
prop.Raw("d:resourcetype", "<d:collection/>"),
},
},
},
{
Status: "HTTP/1.1 404 Not Found",
Prop: []prop.PropertyXML{
prop.NotFound("oc:trashbin-original-filename"),
prop.NotFound("oc:trashbin-original-location"),
prop.NotFound("oc:trashbin-delete-datetime"),
prop.NotFound("d:getcontentlength"),
{
Status: "HTTP/1.1 404 Not Found",
Prop: []prop.PropertyXML{
prop.NotFound("oc:trashbin-original-filename"),
prop.NotFound("oc:trashbin-original-location"),
prop.NotFound("oc:trashbin-delete-datetime"),
prop.NotFound("d:getcontentlength"),
},
},
},
},
})
})
}

for i := range items {
res, err := h.itemToPropResponse(ctx, s, spaceID, refBase, pf, items[i])
Expand Down Expand Up @@ -401,7 +386,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, spaceI
propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:trashbin-delete-datetime", dTime))
if item.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER {
propstatOK.Prop = append(propstatOK.Prop, prop.Raw("d:resourcetype", "<d:collection/>"))
// TODO(jfd): decide if we can and want to list oc:size for folders
propstatOK.Prop = append(propstatOK.Prop, prop.Raw("oc:size", size))
} else {
propstatOK.Prop = append(propstatOK.Prop,
prop.Escaped("d:resourcetype", ""),
Expand All @@ -426,7 +411,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, spaceI
switch pf.Prop[i].Local {
case "oc:size":
if item.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER {
propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("d:getcontentlength", size))
propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:size", size))
} else {
propstatNotFound.Prop = append(propstatNotFound.Prop, prop.NotFound("oc:size"))
}
Expand Down Expand Up @@ -480,7 +465,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()

Expand Down Expand Up @@ -566,7 +551,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,
}

Expand Down Expand Up @@ -608,16 +593,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()
Expand All @@ -638,7 +622,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:
Expand Down
Loading

0 comments on commit 6100655

Please sign in to comment.