incorporate review

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
cs3org · Aug 11, 2021 · 3ee5e6f · 3ee5e6f
1 parent 83975c2
commit 3ee5e6f
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 27 deletions.
diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go
@@ -34,6 +34,7 @@ import (
 	"github.com/cs3org/reva/pkg/rgrpc"
 	"github.com/cs3org/reva/pkg/rgrpc/status"
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/pkg/utils"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 	"go.opencensus.io/trace"
@@ -577,7 +578,7 @@ func (s *service) unwrap(ctx context.Context, ref *provider.Reference) (token st
 		return "", "", errtypes.BadRequest("need absolute path ref: got " + ref.String())
 	}
 
-	if !strings.HasPrefix(ref.GetPath(), "/") {
+	if !utils.IsAbsolutePathReference(ref) {
 		// abort, no valid id nor path
 		return "", "", errtypes.BadRequest("invalid ref: " + ref.String())
 	}

diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go
@@ -500,7 +500,6 @@ func (s *service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteSt
 }
 
 func (s *service) CreateContainer(ctx context.Context, req *provider.CreateContainerRequest) (*provider.CreateContainerResponse, error) {
-	var err error
 	var parentRef *provider.Reference
 	var name string
 	switch {
@@ -521,20 +520,6 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta
 			Status: status.NewInvalidArg(ctx, "invalid reference, name required"),
 		}, nil
 	}
-	var st *rpc.Status
-	if err != nil {
-		switch err.(type) {
-		case errtypes.IsNotFound:
-			st = status.NewNotFound(ctx, "path not found when unwrapping")
-		case errtypes.PermissionDenied:
-			st = status.NewPermissionDenied(ctx, err, "permission denied")
-		default:
-			st = status.NewInternal(ctx, err, "error unwrapping: "+req.String())
-		}
-		return &provider.CreateContainerResponse{
-			Status: st,
-		}, nil
-	}
 	if err := s.storage.CreateDir(ctx, parentRef, name); err != nil {
 		var st *rpc.Status
 		switch err.(type) {
@@ -605,19 +590,14 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide
 	sourceRef, err := s.unwrap(ctx, req.Source)
 	if err != nil {
 		return &provider.MoveResponse{
-			Status: status.NewInternal(ctx, err, "error unwrapping source path"),
+			Status: status.NewInvalid(ctx, err.Error()),
 		}, nil
 	}
 	targetRef, err := s.unwrap(ctx, req.Destination)
 	if err != nil {
-		switch err.(type) {
-		case errtypes.IsNotFound:
-			targetRef = req.Destination
-		default:
-			return &provider.MoveResponse{
-				Status: status.NewInternal(ctx, err, "error unwrapping destination path"),
-			}, nil
-		}
+		return &provider.MoveResponse{
+			Status: status.NewInvalid(ctx, err.Error()),
+		}, nil
 	}
 
 	if err := s.storage.Move(ctx, sourceRef, targetRef); err != nil {

diff --git a/pkg/auth/scope/publicshare.go b/pkg/auth/scope/publicshare.go
@@ -71,7 +71,6 @@ func publicshareScope(scope *authpb.Scope, resource interface{}) (bool, error) {
 }
 
 func checkStorageRef(s *link.PublicShare, r *provider.Reference) bool {
-	// TODO @ishank011 con you explain how this is used?
 	// r: <resource_id:<storage_id:$storageID opaque_id:$opaqueID> path:$path > >
 	if r.ResourceId != nil && r.Path == "" { // path must be empty
 		return utils.ResourceIDEqual(s.ResourceId, r.GetResourceId())

diff --git a/pkg/auth/scope/resourceinfo.go b/pkg/auth/scope/resourceinfo.go
@@ -68,7 +68,6 @@ func resourceinfoScope(scope *authpb.Scope, resource interface{}) (bool, error)
 }
 
 func checkResourceInfo(inf *provider.ResourceInfo, ref *provider.Reference) bool {
-	// TODO @ishank011 con you explain how this is used?
 	// ref: <resource_id:<storage_id:$storageID opaque_id:$opaqueID path:$path> >
 	if ref.ResourceId != nil { // path can be empty or a relative path
 		if inf.Id.StorageId == ref.ResourceId.StorageId && inf.Id.OpaqueId == ref.ResourceId.OpaqueId {