fix: Prevent conflict when copying a live photo inside a folder with …

…a mov file with the same name

Signed-off-by: Louis Chemineau <[email protected]>

apps/dav/lib/Connector/Sabre/Directory.php

@@ -447,7 +447,10 @@ public function copyInto($targetName, $sourcePath, INode $sourceNode) {
 					throw new InvalidPath($ex->getMessage());
 				}
 
-				return $this->fileView->copy($sourcePath, $destinationPath);
+				$copyOkay = $this->fileView->copy($sourcePath, $destinationPath);
+				if (!$copyOkay) {
+					throw new \Sabre\DAV\Exception\Forbidden('');
+				}
 			} catch (StorageNotAvailableException $e) {
 				throw new ServiceUnavailable($e->getMessage(), $e->getCode(), $e);
 			} catch (ForbiddenException $ex) {
@@ -457,7 +460,7 @@ public function copyInto($targetName, $sourcePath, INode $sourceNode) {
 			}
 		}
 
-		return false;
+		return true;
 	}
 
 	public function getNode(): Folder {

apps/files/lib/Listener/SyncLivePhotosListener.php

@@ -78,6 +78,7 @@ public function handle(Event $event): void {
 			}
 
 			if ($event instanceof BeforeNodeRenamedEvent) {
+				$this->runMoveOrCopyChecks($event->getSource(), $event->getTarget(), $peerFile);
 				$this->handleMove($event->getSource(), $event->getTarget(), $peerFile, false);
 			} elseif ($event instanceof BeforeNodeDeletedEvent) {
 				$this->handleDeletion($event, $peerFile);
@@ -87,18 +88,12 @@ public function handle(Event $event): void {
 		}
 	}
 
-	/**
-	 * During rename events, which also include move operations,
-	 * we rename the peer file using the same name.
-	 * The event listener being singleton, we can store the current state
-	 * of pending renames inside the 'pendingRenames' property,
-	 * to prevent infinite recursive.
-	 */
-	private function handleMove(Node $sourceFile, Node $targetFile, Node $peerFile, bool $prepForCopyOnly = false): void {
+	private function runMoveOrCopyChecks(Node $sourceFile, Node $targetFile, Node $peerFile): void {
 		$targetParent = $targetFile->getParent();
 		$sourceExtension = $sourceFile->getExtension();
 		$peerFileExtension = $peerFile->getExtension();
 		$targetName = $targetFile->getName();
+		$peerTargetName = substr($targetName, 0, -strlen($sourceExtension)) . $peerFileExtension;
 
 		if (!str_ends_with($targetName, '.' . $sourceExtension)) {
 			throw new AbortedEventException('Cannot change the extension of a Live Photo');
@@ -110,17 +105,37 @@ private function handleMove(Node $sourceFile, Node $targetFile, Node $peerFile,
 		} catch (NotFoundException) {
 		}
 
-		$peerTargetName = substr($targetName, 0, -strlen($sourceExtension)) . $peerFileExtension;
+		try {
+			$targetParent->get($peerTargetName);
+			throw new AbortedEventException('A file already exist at destination path of the Live Photo');
+		} catch (NotFoundException) {
+		}
+
 		if (!($targetParent instanceof NonExistingFolder)) {
 			try {
 				$targetParent->get($peerTargetName);
 				throw new AbortedEventException('A file already exist at destination path of the Live Photo');
 			} catch (NotFoundException) {
 			}
 		}
+	}
+
+	/**
+	 * During rename events, which also include move operations,
+	 * we rename the peer file using the same name.
+	 * The event listener being singleton, we can store the current state
+	 * of pending renames inside the 'pendingRenames' property,
+	 * to prevent infinite recursive.
+	 */
+	private function handleMove(Node $sourceFile, Node $targetFile, Node $peerFile): void {
+		$targetParent = $targetFile->getParent();
+		$sourceExtension = $sourceFile->getExtension();
+		$peerFileExtension = $peerFile->getExtension();
+		$targetName = $targetFile->getName();
+		$peerTargetName = substr($targetName, 0, -strlen($sourceExtension)) . $peerFileExtension;
 
 		// in case the rename was initiated from this listener, we stop right now
-		if ($prepForCopyOnly || in_array($peerFile->getId(), $this->pendingRenames)) {
+		if (in_array($peerFile->getId(), $this->pendingRenames)) {
 			return;
 		}
 
@@ -131,7 +146,7 @@ private function handleMove(Node $sourceFile, Node $targetFile, Node $peerFile,
 			throw new AbortedEventException($ex->getMessage());
 		}
 
-		array_diff($this->pendingRenames, [$sourceFile->getId()]);
+		$this->pendingRenames = array_diff($this->pendingRenames, [$sourceFile->getId()]);
 	}
 
 
@@ -227,7 +242,7 @@ private function handleCopyRecursive(Event $event, Node $sourceNode, Node $targe
 			}
 
 			if ($event instanceof BeforeNodeCopiedEvent) {
-				$this->handleMove($sourceNode, $targetNode, $peerFile, true);
+				$this->runMoveOrCopyChecks($event->getSource(), $event->getTarget(), $peerFile);
 			} elseif ($event instanceof NodeCopiedEvent) {
 				$this->handleCopy($sourceNode, $targetNode, $peerFile);
 			}

cypress/e2e/files/FilesUtils.ts

@@ -159,7 +159,7 @@ export const createFolder = (folderName: string) => {
 
 	// TODO: replace by proper data-cy selectors
 	cy.get('[data-cy-upload-picker] .action-item__menutoggle').first().click()
-	cy.contains('.upload-picker__menu-entry button', 'New folder').click()
+	cy.get('[data-cy-upload-picker-menu-entry="newFolder"] button').click()
 	cy.get('[data-cy-files-new-node-dialog]').should('be.visible')
 	cy.get('[data-cy-files-new-node-dialog-input]').type(`{selectall}${folderName}`)
 	cy.get('[data-cy-files-new-node-dialog-submit]').click()

cypress/e2e/files/live_photos.cy.ts

@@ -68,20 +68,30 @@ describe('Files: Live photos', { testIsolation: true }, () => {
 			getRowForFile(`${randomFileName} (copy).mov`).should('have.length', 1)
 		})
 
-		it.only('Keeps live photo link when copying folder', () => {
-			setShowHiddenFiles(false)
-
+		it('Keeps live photo link when copying folder', () => {
 			createFolder('folder')
 			moveFile(`${randomFileName}.jpg`, 'folder')
 			copyFile('folder', '.')
 			navigateToFolder('folder (copy)')
 
 			getRowForFile(`${randomFileName}.jpg`).should('have.length', 1)
-			getRowForFile(`${randomFileName}.mov`).should('have.length', 0)
+			getRowForFile(`${randomFileName}.mov`).should('have.length', 1)
 
-			setShowHiddenFiles(true)
+			setShowHiddenFiles(false)
 
 			getRowForFile(`${randomFileName}.jpg`).should('have.length', 1)
+			getRowForFile(`${randomFileName}.mov`).should('have.length', 0)
+		})
+
+		it.only('Block copying live photo in a folder containing a mov file with the same name', () => {
+			createFolder('folder')
+			cy.uploadContent(user, new Blob(['mov file'], { type: 'video/mov' }), 'video/mov', `/folder/${randomFileName}.mov`)
+			cy.login(user)
+			cy.visit('/apps/files')
+			copyFile(`${randomFileName}.jpg`, 'folder')
+			navigateToFolder('folder')
+
+			cy.get('[data-cy-files-list-row-fileid]').should('have.length', 1)
 			getRowForFile(`${randomFileName}.mov`).should('have.length', 1)
 		})