diff --git a/changelog/unreleased/fix-shares-jail-child-size.md b/changelog/unreleased/fix-shares-jail-child-size.md new file mode 100644 index 0000000000..b21ccf9b57 --- /dev/null +++ b/changelog/unreleased/fix-shares-jail-child-size.md @@ -0,0 +1,5 @@ +Bugfix: correct share jail child aggregation + +We now add up the size of all mount points when aggregating the size for a child with the same name. Furthermore, the listing should no longer contain duplicate entries. + +https://github.com/cs3org/reva/pull/2907 \ No newline at end of file diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index d21ff071d0..8faa863c07 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -144,6 +144,12 @@ type PropstatUnmarshalXML struct { ResponseDescription string `xml:"responsedescription,omitempty"` } +// spaceData is used to remember the space for a resource info +type spaceData struct { + Ref *provider.Reference + Space *provider.StorageSpace +} + // NewMultiStatusResponseXML returns a preconfigured instance of MultiStatusResponseXML func NewMultiStatusResponseXML() *MultiStatusResponseXML { return &MultiStatusResponseXML{ @@ -209,12 +215,28 @@ func (p *Handler) HandlePathPropfind(w http.ResponseWriter, r *http.Request, ns return } + var root *provider.StorageSpace + + switch { + case len(spaces) == 1: + root = spaces[0] + case len(spaces) > 1: + for _, space := range spaces { + if isVirtualRootResourceID(space.Root) { + root = space + } + } + if root == nil { + root = spaces[0] + } + } + resourceInfos, sendTusHeaders, ok := p.getResourceInfos(ctx, w, r, pf, spaces, fn, false, sublog) if !ok { // getResourceInfos handles responses in case of an error so we can just return here. return } - p.propfindResponse(ctx, w, r, ns, spaces[0].SpaceType, pf, sendTusHeaders, resourceInfos, sublog) + p.propfindResponse(ctx, w, r, ns, root.SpaceType, pf, sendTusHeaders, resourceInfos, sublog) } // HandleSpacesPropfind handles a spaces based propfind request @@ -305,7 +327,6 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r } w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") - if sendTusHeaders { w.Header().Add(net.HeaderAccessControlExposeHeaders, strings.Join([]string{net.HeaderTusResumable, net.HeaderTusVersion, net.HeaderTusExtension}, ", ")) w.Header().Set(net.HeaderTusResumable, "1.0.0") @@ -374,7 +395,7 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r var mostRecentChildInfo *provider.ResourceInfo var aggregatedChildSize uint64 spaceInfos := make([]*provider.ResourceInfo, 0, len(spaces)) - spaceMap := map[*provider.ResourceInfo]*provider.Reference{} + spaceMap := map[*provider.ResourceInfo]spaceData{} for _, space := range spaces { if space.Opaque == nil || space.Opaque.Map == nil || space.Opaque.Map["path"] == nil || space.Opaque.Map["path"].Decoder != "plain" { continue // not mounted @@ -398,8 +419,9 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r info.Path = filepath.Join(spacePath, spaceRef.Path) } - spaceMap[info] = spaceRef + spaceMap[info] = spaceData{Ref: spaceRef, Space: space} spaceInfos = append(spaceInfos, info) + if rootInfo == nil && (requestPath == info.Path || (spacesPropfind && requestPath == path.Join("/", info.Path))) { rootInfo = info } else if requestPath != spacePath && strings.HasPrefix(spacePath, requestPath) { // Check if the space is a child of the requested path @@ -461,11 +483,12 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r } spaceInfo.Path = path.Join(requestPath, childName) if existingChild, ok := childInfos[childName]; ok { + // aggregate size + childInfos[childName].Size += spaceInfo.Size // use most recent child if existingChild.Mtime == nil || (spaceInfo.Mtime != nil && utils.TSToUnixNano(spaceInfo.Mtime) > utils.TSToUnixNano(existingChild.Mtime)) { childInfos[childName].Mtime = spaceInfo.Mtime childInfos[childName].Etag = spaceInfo.Etag - childInfos[childName].Size += spaceInfo.Size } // only update fileid if the resource is a direct child if tail == "/" { @@ -483,9 +506,9 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r case spaceInfo.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == net.DepthOne: switch { - case strings.HasPrefix(requestPath, spaceInfo.Path): + case strings.HasPrefix(requestPath, spaceInfo.Path) && spaceMap[spaceInfo].Space.SpaceType != "virtual": req := &provider.ListContainerRequest{ - Ref: spaceMap[spaceInfo], + Ref: spaceMap[spaceInfo].Ref, ArbitraryMetadataKeys: metadataKeys, } res, err := client.ListContainer(ctx, req) @@ -519,7 +542,7 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r info := stack[0] stack = stack[1:] - if info.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER { + if info.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER || spaceMap[spaceInfo].Space.SpaceType == "virtual" { continue } req := &provider.ListContainerRequest{ @@ -1320,3 +1343,10 @@ func (pn *Props) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { } } } + +func isVirtualRootResourceID(id *provider.ResourceId) bool { + return utils.ResourceIDEqual(id, &provider.ResourceId{ + StorageId: utils.ShareStorageProviderID, + OpaqueId: utils.ShareStorageProviderID, + }) +} diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go index 2e6e452106..2b66953ab5 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go @@ -23,7 +23,10 @@ import ( "fmt" "net/http" "path" + "path/filepath" "sort" + "strconv" + "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -74,7 +77,7 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) { // we need to sort the received shares by mount point in order to make things easier to evaluate. base := path.Base(sharedResource.GetInfo().GetPath()) mount := base - var mountPoints []string + var mountedShares []*collaboration.ReceivedShare sharesToAccept := map[string]bool{shareID: true} for _, s := range lrs.Shares { if utils.ResourceIDEqual(s.Share.ResourceId, rs.Share.Share.GetResourceId()) { @@ -85,22 +88,40 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) { } } else { if s.State == collaboration.ShareState_SHARE_STATE_ACCEPTED { - mountPoints = append(mountPoints, s.MountPoint.Path) + mountedShares = append(mountedShares, s) } } } - sort.Strings(mountPoints) + compareMountPoint := func(i, j int) bool { + return mountedShares[i].MountPoint.Path > mountedShares[j].MountPoint.Path + } + sort.Slice(mountedShares, compareMountPoint) // now we have a list of shares, we want to iterate over all of them and check for name collisions - // FIXME: adjust logic - /* - for i, mp := range mountPoints { - if mp == mount { - mount = fmt.Sprintf("%s (%s)", base, strconv.Itoa(i+1)) + for i, ms := range mountedShares { + if ms.MountPoint.Path == mount { + // does the shared resource still exist? + res, err := client.Stat(ctx, &provider.StatRequest{ + Ref: &provider.Reference{ + ResourceId: ms.Share.ResourceId, + }, + }) + if err == nil && res.Status.Code == rpc.Code_CODE_OK { + // The mount point really already exists, we need to insert a number into the filename + ext := filepath.Ext(base) + name := strings.TrimSuffix(base, ext) + // be smart about .tar.(gz|bz) files + if strings.HasSuffix(name, ".tar") { + name = strings.TrimSuffix(name, ".tar") + ext = ".tar" + ext + } + + mount = fmt.Sprintf("%s (%s)%s", name, strconv.Itoa(i+1), ext) } + // TODO we could delete shares here if the stat returns code NOT FOUND ... but listening for file deletes would be better } - */ + } for id := range sharesToAccept { data := h.updateReceivedShare(w, r, id, false, mount)