Avoid copying proto messages

protobuf messages are not supposed to be copied. This changes
the code to use pointers instead. Or proto.Clone() if a copy is
really needed.

Addresss ("copylocks: xxxx passes lock by value ...") issues raised by go-vet

internal/grpc/services/gateway/authprovider.go

@@ -36,6 +36,7 @@ import (
 	"github.com/cs3org/reva/v2/pkg/sharedconf"
 	"github.com/pkg/errors"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/protobuf/proto"
 )
 
 func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest) (*gateway.AuthenticateResponse, error) {
@@ -92,12 +93,12 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest
 		}, nil
 	}
 
-	u := *res.User
+	u := proto.Clone(res.User).(*userpb.User)
 	if sharedconf.SkipUserGroupsInToken() {
 		u.Groups = []string{}
 	}
 
-	token, err := s.tokenmgr.MintToken(ctx, &u, res.TokenScope)
+	token, err := s.tokenmgr.MintToken(ctx, u, res.TokenScope)
 	if err != nil {
 		err = errors.Wrap(err, "authsvc: error in MintToken")
 		res := &gateway.AuthenticateResponse{

internal/http/services/owncloud/ocdav/spaces.go

@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"path"
 
+	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/config"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
@@ -30,6 +31,7 @@ import (
 	"github.com/cs3org/reva/v2/pkg/rhttp/router"
 	"github.com/cs3org/reva/v2/pkg/storagespace"
 	"github.com/cs3org/reva/v2/pkg/utils"
+	"google.golang.org/protobuf/proto"
 )
 
 // SpacesHandler handles trashbin requests
@@ -167,10 +169,10 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
 		}
 		log.Debug().Str("key", key).Str("path", r.URL.Path).Str("dst", dst).Msg("spaces restore")
 
-		dstRef := ref
+		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, r.URL.Path)
 	case http.MethodDelete:
 		trashbinHandler.delete(w, r, s, &ref, key, r.URL.Path)
 	default:

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

@@ -179,13 +179,14 @@ func (h *Handler) startCacheWarmup(c sharecache.Warmup) {
 	}
 }
 
-func (h *Handler) extractReference(r *http.Request) (provider.Reference, error) {
-	var ref provider.Reference
+func (h *Handler) extractReference(r *http.Request) (*provider.Reference, error) {
+	ref := &provider.Reference{}
 
 	// NOTE: space_ref is deprecated and will be removed in ~2 weeks (1.6.22)
 	sr := r.FormValue("space_ref")
 	if sr != "" {
-		return storagespace.ParseReference(sr)
+		ref, err := storagespace.ParseReference(sr)
+		return &ref, err
 	}
 
 	p, id := r.FormValue("path"), r.FormValue("space")
@@ -243,7 +244,7 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	statReq := provider.StatRequest{Ref: &ref, FieldMask: &fieldmaskpb.FieldMask{Paths: []string{"space"}}}
+	statReq := provider.StatRequest{Ref: ref, FieldMask: &fieldmaskpb.FieldMask{Paths: []string{"space"}}}
 	statRes, err := client.Stat(ctx, &statReq)
 	if err != nil {
 		sublog.Debug().Err(err).Msg("CreateShare: error on stat call")
@@ -976,7 +977,7 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) {
 		}
 
 		var status *rpc.Status
-		pinfo, status, err = h.getResourceInfoByReference(ctx, client, &ref)
+		pinfo, status, err = h.getResourceInfoByReference(ctx, client, ref)
 		if err != nil {
 			response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error sending a grpc stat request", err)
 			return
@@ -1157,7 +1158,7 @@ func (h *Handler) listSharesWithOthers(w http.ResponseWriter, r *http.Request) {
 			response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, errParsingSpaceReference.Error(), errParsingSpaceReference)
 			return
 		}
-		filters, linkFilters, e = h.addFilters(w, r, &ref)
+		filters, linkFilters, e = h.addFilters(w, r, ref)
 		if e != nil {
 			// result has been written as part of addFilters
 			return

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go

@@ -150,14 +150,14 @@ func (h *Handler) addSpaceMember(w http.ResponseWriter, r *http.Request, info *p
 		return
 	}
 
-	if !isSpaceManagerRemaining(lgRes.Grants, grantee) {
+	if !isSpaceManagerRemaining(lgRes.Grants, &grantee) {
 		response.WriteOCSError(w, r, http.StatusForbidden, "the space must have at least one manager", nil)
 		return
 	}
 
 	// we have to send the update request to the gateway to give it a chance to invalidate its cache
 	// TODO the gateway no longer should cache stuff because invalidation is to expensive. The decomposedfs already has a better cache.
-	if granteeExists(lgRes.Grants, grantee) {
+	if granteeExists(lgRes.Grants, &grantee) {
 		if permissions != nil {
 			fieldmask = append(fieldmask, "permissions")
 		}
@@ -257,7 +257,7 @@ func (h *Handler) removeSpaceMember(w http.ResponseWriter, r *http.Request, spac
 		return
 	}
 
-	if len(lgRes.Grants) == 1 || !isSpaceManagerRemaining(lgRes.Grants, grantee) {
+	if len(lgRes.Grants) == 1 || !isSpaceManagerRemaining(lgRes.Grants, &grantee) {
 		response.WriteOCSError(w, r, http.StatusForbidden, "can't remove the last manager", nil)
 		return
 	}
@@ -349,28 +349,28 @@ func (h *Handler) findProvider(ctx context.Context, ref *provider.Reference) (*r
 	return res.Providers[0], nil
 }
 
-func isSpaceManagerRemaining(grants []*provider.Grant, grantee provider.Grantee) bool {
+func isSpaceManagerRemaining(grants []*provider.Grant, grantee *provider.Grantee) bool {
 	for _, g := range grants {
 		// RemoveGrant is currently the way to check for the manager role
 		// If it is not set than the current grant is not for a manager and
 		// we can just continue with the next one.
-		if g.Permissions.RemoveGrant && !isEqualGrantee(*g.Grantee, grantee) {
+		if g.Permissions.RemoveGrant && !isEqualGrantee(g.Grantee, grantee) {
 			return true
 		}
 	}
 	return false
 }
 
-func granteeExists(grants []*provider.Grant, grantee provider.Grantee) bool {
+func granteeExists(grants []*provider.Grant, grantee *provider.Grantee) bool {
 	for _, g := range grants {
-		if isEqualGrantee(*g.Grantee, grantee) {
+		if isEqualGrantee(g.Grantee, grantee) {
 			return true
 		}
 	}
 	return false
 }
 
-func isEqualGrantee(a, b provider.Grantee) bool {
+func isEqualGrantee(a, b *provider.Grantee) bool {
 	// Ideally we would want to use utils.GranteeEqual()
 	// but the grants stored in the decomposedfs aren't complete (missing usertype and idp)
 	// because of that the check would fail so we can only check the ... for now.

pkg/publicshare/manager/cs3/cs3.go

@@ -421,11 +421,11 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters []
 			return nil, err
 		}
 
-		if !publicshare.MatchesFilters(ps.PublicShare, filters) {
+		if !publicshare.MatchesFilters(&ps.PublicShare, filters) {
 			continue
 		}
 
-		if publicshare.IsExpired(ps.PublicShare) {
+		if publicshare.IsExpired(&ps.PublicShare) {
 			ref := &link.PublicShareReference{
 				Spec: &link.PublicShareReference_Id{
 					Id: ps.PublicShare.Id,
@@ -505,7 +505,7 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters []
 			statMem[resourceIDToIndex(s.PublicShare.GetResourceId())] = struct{}{}
 		}
 
-		if publicshare.MatchesFilters(s.PublicShare, filters) {
+		if publicshare.MatchesFilters(&s.PublicShare, filters) {
 			result = append(result, &s.PublicShare)
 			shareMem[s.PublicShare.Token] = struct{}{}
 		}
@@ -545,7 +545,7 @@ func (m *Manager) GetPublicShareByToken(ctx context.Context, token string, auth
 		return nil, err
 	}
 
-	if publicshare.IsExpired(ps.PublicShare) {
+	if publicshare.IsExpired(&ps.PublicShare) {
 		return nil, errtypes.NotFound("public share has expired")
 	}
 

pkg/publicshare/manager/json/json.go

@@ -461,7 +461,7 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu
 		}
 
 		if ref.GetId().GetOpaqueId() == ps.Id.OpaqueId {
-			if publicshare.IsExpired(ps) {
+			if publicshare.IsExpired(&ps) {
 				if err := m.revokeExpiredPublicShare(ctx, &ps); err != nil {
 					return nil, err
 				}
@@ -509,7 +509,7 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters []
 			return nil, err
 		}
 
-		if publicshare.IsExpired(local.PublicShare) {
+		if publicshare.IsExpired(&local.PublicShare) {
 			if err := m.revokeExpiredPublicShare(ctx, &local.PublicShare); err != nil {
 				log.Error().Err(err).
 					Str("share_token", local.Token).
@@ -518,7 +518,7 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters []
 			continue
 		}
 
-		if !publicshare.MatchesFilters(local.PublicShare, filters) {
+		if !publicshare.MatchesFilters(&local.PublicShare, filters) {
 			continue
 		}
 
@@ -583,7 +583,7 @@ func (m *manager) cleanupExpiredShares() {
 		var ps link.PublicShare
 		_ = utils.UnmarshalJSONToProtoV1([]byte(d.(string)), &ps)
 
-		if publicshare.IsExpired(ps) {
+		if publicshare.IsExpired(&ps) {
 			_ = m.revokeExpiredPublicShare(context.Background(), &ps)
 		}
 	}
@@ -693,7 +693,7 @@ func (m *manager) GetPublicShareByToken(ctx context.Context, token string, auth
 		}
 
 		if local.Token == token {
-			if publicshare.IsExpired(local) {
+			if publicshare.IsExpired(&local) {
 				if err := m.revokeExpiredPublicShare(ctx, &local); err != nil {
 					return nil, err
 				}

pkg/publicshare/manager/owncloudsql/owncloudsql.go

@@ -268,7 +268,7 @@ func (m *mgr) GetPublicShare(ctx context.Context, u *user.User, ref *link.Public
 		return nil, err
 	}
 
-	if publicshare.IsExpired(ps.PublicShare) {
+	if publicshare.IsExpired(&ps.PublicShare) {
 		if err := m.cleanupExpiredShares(); err != nil {
 			return nil, err
 		}
@@ -436,7 +436,7 @@ func (m *mgr) ListPublicShares(ctx context.Context, u *user.User, filters []*lin
 		if cs3Share, err = m.ConvertToCS3PublicShare(ctx, s); err != nil {
 			return nil, err
 		}
-		if publicshare.IsExpired(*cs3Share) {
+		if publicshare.IsExpired(cs3Share) {
 			_ = m.cleanupExpiredShares()
 		} else {
 			if cs3Share.PasswordProtected && sign {
@@ -495,7 +495,7 @@ func (m *mgr) GetPublicShareByToken(ctx context.Context, token string, auth *lin
 		return nil, err
 	}
 
-	if publicshare.IsExpired(ps.PublicShare) {
+	if publicshare.IsExpired(&ps.PublicShare) {
 		if err := m.cleanupExpiredShares(); err != nil {
 			return nil, err
 		}

pkg/publicshare/publicshare.go

@@ -131,7 +131,7 @@ func StorageIDFilter(id string) *link.ListPublicSharesRequest_Filter {
 }
 
 // MatchesFilter tests if the share passes the filter.
-func MatchesFilter(share link.PublicShare, filter *link.ListPublicSharesRequest_Filter) bool {
+func MatchesFilter(share *link.PublicShare, filter *link.ListPublicSharesRequest_Filter) bool {
 	switch filter.Type {
 	case link.ListPublicSharesRequest_Filter_TYPE_RESOURCE_ID:
 		return utils.ResourceIDEqual(share.ResourceId, filter.GetResourceId())
@@ -143,7 +143,7 @@ func MatchesFilter(share link.PublicShare, filter *link.ListPublicSharesRequest_
 }
 
 // MatchesAnyFilter checks if the share passes at least one of the given filters.
-func MatchesAnyFilter(share link.PublicShare, filters []*link.ListPublicSharesRequest_Filter) bool {
+func MatchesAnyFilter(share *link.PublicShare, filters []*link.ListPublicSharesRequest_Filter) bool {
 	for _, f := range filters {
 		if MatchesFilter(share, f) {
 			return true
@@ -156,7 +156,7 @@ func MatchesAnyFilter(share link.PublicShare, filters []*link.ListPublicSharesRe
 // Filters of the same type form a disjuntion, a logical OR. Filters of separate type form a conjunction, a logical AND.
 // Here is an example:
 // (resource_id=1 OR resource_id=2) AND (grantee_type=USER OR grantee_type=GROUP)
-func MatchesFilters(share link.PublicShare, filters []*link.ListPublicSharesRequest_Filter) bool {
+func MatchesFilters(share *link.PublicShare, filters []*link.ListPublicSharesRequest_Filter) bool {
 	if len(filters) == 0 {
 		return true
 	}
@@ -179,7 +179,7 @@ func GroupFiltersByType(filters []*link.ListPublicSharesRequest_Filter) map[link
 }
 
 // IsExpired tests whether a public share is expired
-func IsExpired(s link.PublicShare) bool {
+func IsExpired(s *link.PublicShare) bool {
 	expiration := time.Unix(int64(s.Expiration.GetSeconds()), int64(s.Expiration.GetNanos()))
 	return s.Expiration != nil && expiration.Before(time.Now())
 }

pkg/storage/fs/posix/testhelpers/helpers.go

@@ -285,7 +285,7 @@ func (t *TestEnv) CreateTestStorageSpace(typ string, quota *providerv1beta1.Quot
 		Status: &v1beta11.Status{Code: v1beta11.Code_CODE_OK},
 	}, nil)
 	// Permissions required for setup below
-	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(providerv1beta1.ResourcePermissions{
+	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
 		Stat:     true,
 		AddGrant: true,
 	}, nil).Times(1) //
@@ -319,7 +319,7 @@ func (t *TestEnv) CreateTestStorageSpace(typ string, quota *providerv1beta1.Quot
 	}
 
 	// Create dir1
-	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(providerv1beta1.ResourcePermissions{
+	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
 		Stat:            true,
 		CreateContainer: true,
 	}, nil).Times(1) // Permissions required for setup below
@@ -335,7 +335,7 @@ func (t *TestEnv) CreateTestStorageSpace(typ string, quota *providerv1beta1.Quot
 	}
 
 	// Create subdir1 in dir1
-	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(providerv1beta1.ResourcePermissions{
+	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
 		Stat:            true,
 		CreateContainer: true,
 	}, nil).Times(1) // Permissions required for setup below
@@ -351,7 +351,7 @@ func (t *TestEnv) CreateTestStorageSpace(typ string, quota *providerv1beta1.Quot
 	}
 
 	// Create emptydir
-	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(providerv1beta1.ResourcePermissions{
+	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
 		Stat:            true,
 		CreateContainer: true,
 	}, nil).Times(1) // Permissions required for setup below

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -567,7 +567,7 @@ func (fs *Decomposedfs) GetQuota(ctx context.Context, ref *provider.Reference) (
 	}
 
 	// FIXME move treesize & quota to fieldmask
-	ri, err := n.AsResourceInfo(ctx, &rp, []string{"treesize", "quota"}, []string{}, true)
+	ri, err := n.AsResourceInfo(ctx, rp, []string{"treesize", "quota"}, []string{}, true)
 	if err != nil {
 		return 0, 0, 0, err
 	}
@@ -881,7 +881,7 @@ func (fs *Decomposedfs) GetMD(ctx context.Context, ref *provider.Reference, mdKe
 		return nil, errtypes.NotFound(f)
 	}
 
-	md, err := node.AsResourceInfo(ctx, &rp, mdKeys, fieldMask, utils.IsRelativeReference(ref))
+	md, err := node.AsResourceInfo(ctx, rp, mdKeys, fieldMask, utils.IsRelativeReference(ref))
 	if err != nil {
 		return nil, err
 	}
@@ -961,8 +961,8 @@ func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference,
 				np := rp
 				// add this childs permissions
 				pset, _ := child.PermissionSet(ctx)
-				node.AddPermissions(&np, &pset)
-				ri, err := child.AsResourceInfo(ctx, &np, mdKeys, fieldMask, utils.IsRelativeReference(ref))
+				node.AddPermissions(np, pset)
+				ri, err := child.AsResourceInfo(ctx, np, mdKeys, fieldMask, utils.IsRelativeReference(ref))
 				if err != nil {
 					return errtypes.InternalError(err.Error())
 				}

pkg/storage/utils/decomposedfs/decomposedfs_concurrency_test.go

@@ -178,7 +178,7 @@ var _ = Describe("Decomposed", func() {
 
 		Describe("CreateDir", func() {
 			JustBeforeEach(func() {
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(provider.ResourcePermissions{
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
 					Stat:            true,
 					CreateContainer: true,
 				}, nil)