From 4960cafea5c1e762e96e2e5600855b8124de933f Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Mon, 5 Feb 2024 18:03:42 +0100 Subject: [PATCH 1/4] Removed stat to all storage providers in case of a Depth:0 PROPFIND to "/" --- .../grpc/services/gateway/storageprovider.go | 36 ++----------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 924666b7e1..086f3f95df 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -1419,12 +1419,8 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St return rsp, nil } - return s.statAcrossProviders(ctx, req, providers) -} - -func (s *svc) statAcrossProviders(ctx context.Context, req *provider.StatRequest, providers []*registry.ProviderInfo) (*provider.StatResponse, error) { - // TODO(ishank011): aggregrate properties such as etag, checksum, etc. - log := appctx.GetLogger(ctx) + // otherwise, this is a Stat for "/", which corresponds to a 0-Depth PROPFIND from web to just get the fileid: + // we respond with an hardcoded value, no need to poke all storage providers as we did before info := &provider.ResourceInfo{ Id: &provider.ResourceId{ StorageId: "/", @@ -1437,34 +1433,6 @@ func (s *svc) statAcrossProviders(ctx context.Context, req *provider.StatRequest Mtime: &types.Timestamp{}, } - for _, p := range providers { - c, err := s.getStorageProviderClient(ctx, p) - if err != nil { - log.Err(err).Msg("error connecting to storage provider=" + p.Address) - continue - } - resp, err := c.Stat(ctx, req) - if err != nil { - log.Err(err).Msgf("gateway: error calling Stat %s: %+v", req.Ref.String(), p) - continue - } - if resp.Status.Code != rpc.Code_CODE_OK { - log.Err(status.NewErrorFromCode(rpc.Code_CODE_OK, "gateway")).Send() - continue - } - if resp.Info != nil { - info.Size += resp.Info.Size - if utils.TSToUnixNano(resp.Info.Mtime) > utils.TSToUnixNano(info.Mtime) { - info.Mtime = resp.Info.Mtime - info.Etag = resp.Info.Etag - info.Checksum = resp.Info.Checksum - } - if info.Etag == "" && info.Etag != resp.Info.Etag { - info.Etag = resp.Info.Etag - } - } - } - return &provider.StatResponse{ Status: status.NewOK(ctx), Info: info, From 6766520b75f52c154b877102fada54b741aba635 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Mon, 5 Feb 2024 18:06:13 +0100 Subject: [PATCH 2/4] Added changelog --- changelog/unreleased/zerodepth-propfind-fix.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/zerodepth-propfind-fix.md diff --git a/changelog/unreleased/zerodepth-propfind-fix.md b/changelog/unreleased/zerodepth-propfind-fix.md new file mode 100644 index 0000000000..a56f29c58c --- /dev/null +++ b/changelog/unreleased/zerodepth-propfind-fix.md @@ -0,0 +1,6 @@ +Bugfix: removed stat to all storage providers on Depth:0 PROPFIND to "/" + +This PR removes an unnecessary and potentially problematic call, which would +fail if any of the configured storage providers has an issue. + +https://github.com/cs3org/reva/pull/4497 From 3888b1cff2fdfa9eae624165b47c960b19759bec Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Tue, 6 Feb 2024 11:53:03 +0100 Subject: [PATCH 3/4] debug --- internal/grpc/services/gateway/storageprovider.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 086f3f95df..76dbc4a002 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -1396,6 +1396,7 @@ func (s *svc) statSharesFolder(ctx context.Context) (*provider.StatResponse, err } func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) { + log := appctx.GetLogger(ctx) providers, err := s.findProviders(ctx, req.Ref) if err != nil { return &provider.StatResponse{ @@ -1412,6 +1413,7 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St Status: status.NewInternal(ctx, err, "error connecting to storage provider="+providers[0].Address), }, nil } + log.Debug().Interface("ref", req.Ref).Msg("calling Stat") rsp, err := c.Stat(ctx, req) if err != nil || rsp.Status.Code != rpc.Code_CODE_OK { return rsp, err @@ -1421,6 +1423,7 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St // otherwise, this is a Stat for "/", which corresponds to a 0-Depth PROPFIND from web to just get the fileid: // we respond with an hardcoded value, no need to poke all storage providers as we did before + log.Debug().Interface("ref", req.Ref).Msg("sending back fake file-id") info := &provider.ResourceInfo{ Id: &provider.ResourceId{ StorageId: "/", From 0b0bbaa0ba02c0e7c02bfedfcf73e308118dc4d2 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Tue, 6 Feb 2024 12:05:26 +0100 Subject: [PATCH 4/4] Revert "debug" This reverts commit 3888b1cff2fdfa9eae624165b47c960b19759bec. --- internal/grpc/services/gateway/storageprovider.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 76dbc4a002..086f3f95df 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -1396,7 +1396,6 @@ func (s *svc) statSharesFolder(ctx context.Context) (*provider.StatResponse, err } func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) { - log := appctx.GetLogger(ctx) providers, err := s.findProviders(ctx, req.Ref) if err != nil { return &provider.StatResponse{ @@ -1413,7 +1412,6 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St Status: status.NewInternal(ctx, err, "error connecting to storage provider="+providers[0].Address), }, nil } - log.Debug().Interface("ref", req.Ref).Msg("calling Stat") rsp, err := c.Stat(ctx, req) if err != nil || rsp.Status.Code != rpc.Code_CODE_OK { return rsp, err @@ -1423,7 +1421,6 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St // otherwise, this is a Stat for "/", which corresponds to a 0-Depth PROPFIND from web to just get the fileid: // we respond with an hardcoded value, no need to poke all storage providers as we did before - log.Debug().Interface("ref", req.Ref).Msg("sending back fake file-id") info := &provider.ResourceInfo{ Id: &provider.ResourceId{ StorageId: "/",