feat(reva): next try to fix copy

Signed-off-by: jkoberg <jkoberg@owncloud.com>
kobergj · Aug 5, 2024 · c64252b · c64252b
1 parent 0944995
commit c64252b
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 12 deletions.
diff --git a/changelog/unreleased/block-overwriting-mountpoints.md b/changelog/unreleased/block-overwriting-mountpoints.md
@@ -2,5 +2,6 @@ Bugfix: Block overwriting mountpoints
 
 This blocks overwriting mountpoints through the webdav COPY api. It is now returning a bad request when attempting to overwrite a mountpoint.
 
+https://github.com/cs3org/reva/pull/4796
 https://github.com/cs3org/reva/pull/4786
 https://github.com/cs3org/reva/pull/4785
diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go
@@ -159,7 +159,7 @@ func (s *svc) executePathCopy(ctx context.Context, selector pool.Selectable[gate
 			log.Error().Err(err).Msg("error performing create container grpc request")
 			return err
 		}
-		if createRes.Status.Code != rpc.Code_CODE_OK {
+		if createRes.Status.Code != rpc.Code_CODE_OK && createRes.Status.Code != rpc.Code_CODE_INVALID_ARGUMENT {
 			if createRes.Status.Code == rpc.Code_CODE_PERMISSION_DENIED {
 				w.WriteHeader(http.StatusForbidden)
 				m := fmt.Sprintf("Permission denied to create %v", createReq.Ref.Path)
@@ -395,7 +395,7 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, sele
 			log.Error().Err(err).Msg("error performing create container grpc request")
 			return err
 		}
-		if createRes.Status.Code != rpc.Code_CODE_OK {
+		if createRes.Status.Code != rpc.Code_CODE_OK && createRes.Status.Code != rpc.Code_CODE_INVALID_ARGUMENT {
 			if createRes.Status.Code == rpc.Code_CODE_PERMISSION_DENIED {
 				w.WriteHeader(http.StatusForbidden)
 				// TODO path could be empty or relative...
@@ -690,21 +690,39 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
 			return nil
 		}
 
-		// we must not allow to override mountpoints - so we check if we have access to the parent. If not this is a mountpoint
-		if destInShareJail {
-			dir, file := filepath.Split(dstRef.GetPath())
-			if dir == "/" || dir == "" || file == "" {
-				log.Error().Msg("must not overwrite mount points")
-				w.WriteHeader(http.StatusBadRequest)
-				_, _ = w.Write([]byte("must not overwrite mount points"))
+		if dstStatRes.GetInfo().GetType() == provider.ResourceType_RESOURCE_TYPE_CONTAINER {
+			res, err := client.ListContainer(ctx, &provider.ListContainerRequest{Ref: dstRef})
+			if err != nil || res.GetStatus().GetCode() != rpc.Code_CODE_OK {
+				log.Error().Err(err).Msg("error listing container")
+				w.WriteHeader(http.StatusInternalServerError)
 				return nil
 			}
+
+			for _, info := range res.GetInfos() {
+				res, err := client.Delete(ctx, &provider.DeleteRequest{Ref: &provider.Reference{ResourceId: info.Id}})
+				if err != nil || res.GetStatus().GetCode() != rpc.Code_CODE_OK {
+					log.Error().Err(err).Msg("error deleting container")
+					w.WriteHeader(http.StatusInternalServerError)
+					return nil
+				}
+			}
 		}
 
 		// delete existing tree when overwriting a directory or replacing a file with a directory
-		if dstStatRes.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER ||
-			(dstStatRes.Info.Type == provider.ResourceType_RESOURCE_TYPE_FILE &&
-				srcStatRes.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER) {
+		if dstStatRes.Info.Type == provider.ResourceType_RESOURCE_TYPE_FILE &&
+			srcStatRes.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER {
+
+			// we must not allow to override mountpoints - so we check if we have access to the parent. If not this is a mountpoint
+			if destInShareJail {
+				dir, file := filepath.Split(dstRef.GetPath())
+				if dir == "/" || dir == "" || file == "" {
+					log.Error().Msg("must not overwrite mount points")
+					w.WriteHeader(http.StatusBadRequest)
+					_, _ = w.Write([]byte("must not overwrite mount points"))
+					return nil
+				}
+			}
+
 			delReq := &provider.DeleteRequest{Ref: dstRef}
 			delRes, err := client.Delete(ctx, delReq)
 			if err != nil {