From 8b56cf460ba599972ad1660b4a33651400525582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 11 Jul 2023 10:07:11 +0200 Subject: [PATCH 1/2] Return proper RPC status when the public share wasn't found --- .../publicshareprovider.go | 31 ++++++++++++++----- pkg/publicshare/manager/json/json.go | 6 ++-- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider.go b/internal/grpc/services/publicshareprovider/publicshareprovider.go index 7f7e301662..e8f18aea6c 100644 --- a/internal/grpc/services/publicshareprovider/publicshareprovider.go +++ b/internal/grpc/services/publicshareprovider/publicshareprovider.go @@ -22,6 +22,7 @@ import ( "context" "regexp" + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" @@ -218,15 +219,29 @@ func (s *service) GetPublicShare(ctx context.Context, req *link.GetPublicShareRe log.Error().Msg("error getting user from context") } - found, err := s.sm.GetPublicShare(ctx, u, req.Ref, req.GetSign()) - if err != nil { - return nil, err + ps, err := s.sm.GetPublicShare(ctx, u, req.Ref, req.GetSign()) + switch { + case err != nil: + var st *rpc.Status + switch err.(type) { + case errtypes.IsNotFound: + st = status.NewNotFound(ctx, err.Error()) + default: + st = status.NewInternal(ctx, err.Error()) + } + return &link.GetPublicShareResponse{ + Status: st, + }, nil + case ps == nil: + return &link.GetPublicShareResponse{ + Status: status.NewNotFound(ctx, "not found"), + }, nil + default: + return &link.GetPublicShareResponse{ + Status: status.NewOK(ctx), + Share: ps, + }, nil } - - return &link.GetPublicShareResponse{ - Status: status.NewOK(ctx), - Share: found, - }, nil } func (s *service) ListPublicShares(ctx context.Context, req *link.ListPublicSharesRequest) (*link.ListPublicSharesResponse, error) { diff --git a/pkg/publicshare/manager/json/json.go b/pkg/publicshare/manager/json/json.go index bad4767ec3..5d7673b824 100644 --- a/pkg/publicshare/manager/json/json.go +++ b/pkg/publicshare/manager/json/json.go @@ -430,7 +430,7 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu if ref.GetToken() != "" { ps, pw, err := m.getByToken(ctx, ref.GetToken()) if err != nil { - return nil, errors.New("no shares found by token") + return nil, errtypes.NotFound("no shares found by token") } if ps.PasswordProtected && sign { err := publicshare.AddSignature(ps, pw) @@ -463,7 +463,7 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu if err := m.revokeExpiredPublicShare(ctx, &ps, u); err != nil { return nil, err } - return nil, errors.New("no shares found by id:" + ref.GetId().String()) + return nil, errtypes.NotFound("no shares found by id:" + ref.GetId().String()) } if ps.PasswordProtected && sign { err := publicshare.AddSignature(&ps, passDB) @@ -475,7 +475,7 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu } } - return nil, errors.New("no shares found by id:" + ref.GetId().String()) + return nil, errtypes.NotFound("no shares found by id:" + ref.GetId().String()) } // ListPublicShares retrieves all the shares on the manager that are valid. From 83142bee9e4d48d0d31c2a3469427e1cdd950858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 11 Jul 2023 10:20:27 +0200 Subject: [PATCH 2/2] Add changelog --- .../unreleased/fix-error-handling-in-publicshareprovider.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/fix-error-handling-in-publicshareprovider.md diff --git a/changelog/unreleased/fix-error-handling-in-publicshareprovider.md b/changelog/unreleased/fix-error-handling-in-publicshareprovider.md new file mode 100644 index 0000000000..59a3ac74e5 --- /dev/null +++ b/changelog/unreleased/fix-error-handling-in-publicshareprovider.md @@ -0,0 +1,5 @@ +Bugfix: Properly handle not-found errors when getting a public share + +We fixed a problem where not-found errors caused a hard error instead of a proper RPC error state. + +https://github.com/cs3org/reva/pull/4057