From 9a6ad3fdd2728b714fd19c3954117d05e5549e09 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 31 May 2022 17:13:37 +0200 Subject: [PATCH 1/4] fix share jail listing --- .../owncloud/ocdav/propfind/propfind.go | 46 ++++++++++++++++--- .../handlers/apps/sharing/shares/pending.go | 12 ++--- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index d21ff071d0..ef06f76b5a 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -144,6 +144,11 @@ type PropstatUnmarshalXML struct { ResponseDescription string `xml:"responsedescription,omitempty"` } +type SpaceData struct { + Ref *provider.Reference + Space *provider.StorageSpace +} + // NewMultiStatusResponseXML returns a preconfigured instance of MultiStatusResponseXML func NewMultiStatusResponseXML() *MultiStatusResponseXML { return &MultiStatusResponseXML{ @@ -209,12 +214,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 +326,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 +394,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 +418,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 @@ -483,9 +504,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 +540,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 +1341,14 @@ func (pn *Props) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { } } } + +func isVirtualRoot(ref *provider.Reference) bool { + return isVirtualRootResourceID(ref.ResourceId) && (ref.Path == "" || ref.Path == "." || ref.Path == "./") +} + +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..8cebb8d82b 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 @@ -24,6 +24,7 @@ import ( "net/http" "path" "sort" + "strconv" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -94,13 +95,12 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) { // 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, mp := range mountPoints { + if mp == mount { + mount = fmt.Sprintf("%s (%s)", base, strconv.Itoa(i+1)) } - */ + } for id := range sharesToAccept { data := h.updateReceivedShare(w, r, id, false, mount) From ab1b4f535171a5c9e8c080a332b73ded80c681e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 31 May 2022 19:16:21 +0000 Subject: [PATCH 2/4] always aggregate size for child entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/http/services/owncloud/ocdav/propfind/propfind.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index ef06f76b5a..98a4e80f12 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -482,11 +482,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 == "/" { From f3482b4087a1b58a7f2eb1aef00b20e244f6aaf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 1 Jun 2022 07:59:26 +0000 Subject: [PATCH 3/4] add changelog, lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/fix-shares-jail-child-size.md | 5 +++++ .../http/services/owncloud/ocdav/propfind/propfind.go | 11 ++++------- 2 files changed, 9 insertions(+), 7 deletions(-) create mode 100644 changelog/unreleased/fix-shares-jail-child-size.md 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 98a4e80f12..8faa863c07 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -144,7 +144,8 @@ type PropstatUnmarshalXML struct { ResponseDescription string `xml:"responsedescription,omitempty"` } -type SpaceData struct { +// spaceData is used to remember the space for a resource info +type spaceData struct { Ref *provider.Reference Space *provider.StorageSpace } @@ -394,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]SpaceData{} + 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 @@ -418,7 +419,7 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r info.Path = filepath.Join(spacePath, spaceRef.Path) } - spaceMap[info] = SpaceData{Ref: spaceRef, Space: space} + spaceMap[info] = spaceData{Ref: spaceRef, Space: space} spaceInfos = append(spaceInfos, info) if rootInfo == nil && (requestPath == info.Path || (spacesPropfind && requestPath == path.Join("/", info.Path))) { @@ -1343,10 +1344,6 @@ func (pn *Props) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { } } -func isVirtualRoot(ref *provider.Reference) bool { - return isVirtualRootResourceID(ref.ResourceId) && (ref.Path == "" || ref.Path == "." || ref.Path == "./") -} - func isVirtualRootResourceID(id *provider.ResourceId) bool { return utils.ResourceIDEqual(id, &provider.ResourceId{ StorageId: utils.ShareStorageProviderID, From b7ec546472ae16e6a25ede0afb3b2fbdfab78527 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 1 Jun 2022 10:05:04 +0000 Subject: [PATCH 4/4] rewrite mount point naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../handlers/apps/sharing/shares/pending.go | 37 +++++++++++++++---- 1 file changed, 29 insertions(+), 8 deletions(-) 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 8cebb8d82b..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,8 +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" @@ -75,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()) { @@ -86,19 +88,38 @@ 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 } }