Merge pull request #32360 from owncloud/verify_checksum_fixes_stable10

[stable10] Verify checksum fixes
owncloud · Aug 21, 2018 · 13efb13 · 13efb13
2 parents 2fb0f21 + 948c517
commit 13efb13
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 8 deletions.
diff --git a/apps/files/lib/Command/VerifyChecksums.php b/apps/files/lib/Command/VerifyChecksums.php
@@ -22,7 +22,9 @@
 namespace OCA\Files\Command;
 
 use OC\Files\FileInfo;
+use OC\Files\Storage\FailedStorage;
 use OC\Files\Storage\Wrapper\Checksum;
+use OCA\Files_Sharing\ISharedStorage;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
 use OCP\Files\NotFoundException;
@@ -49,7 +51,6 @@ class VerifyChecksums extends Command {
 	 * @var IRootFolder
 	 */
 	private $rootFolder;
-
 	/**
 	 * @var IUserManager
 	 */
@@ -94,19 +95,34 @@ public function execute(InputInterface $input, OutputInterface $output) {
 		$walkFunction = function (Node $node) use ($input, $output) {
 			$path = $node->getInternalPath();
 			$currentChecksums = $node->getChecksum();
+			$owner = $node->getOwner()->getUID();
+			$storage = $node->getStorage();
+
+			if ($storage->instanceOfStorage(ISharedStorage::class) || $storage->instanceOfStorage(FailedStorage::class)) {
+				return;
+			}
+
+			if (!self::fileExistsOnDisk($node)) {
+				$output->writeln("Skipping $owner/$path => File is in file-cache but doesn't exist on storage/disk", OutputInterface::VERBOSITY_VERBOSE);
+				return;
+			}
+
+			if (!$node->isReadable()) {
+				$output->writeln("Skipping $owner/$path => File not readable", OutputInterface::VERBOSITY_VERBOSE);
+				return;
+			}
 
 			// Files without calculated checksum can't cause checksum errors
 			if (empty($currentChecksums)) {
-				$output->writeln("Skipping $path => No Checksum", OutputInterface::VERBOSITY_VERBOSE);
+				$output->writeln("Skipping $owner/$path => No Checksum", OutputInterface::VERBOSITY_VERBOSE);
 				return;
 			}
 
-			$output->writeln("Checking $path => $currentChecksums", OutputInterface::VERBOSITY_VERBOSE);
+			$output->writeln("Checking $owner/$path => $currentChecksums", OutputInterface::VERBOSITY_VERBOSE);
 			$actualChecksums = self::calculateActualChecksums($path, $node->getStorage());
-
 			if ($actualChecksums !== $currentChecksums) {
 				$output->writeln(
-					"<info>Mismatch for $path:\n Filecache:\t$currentChecksums\n Actual:\t$actualChecksums</info>"
+					"<info>Mismatch for $owner/$path:\n Filecache:\t$currentChecksums\n Actual:\t$actualChecksums</info>"
 				);
 
 				$this->exitStatus = self::EXIT_CHECKSUM_ERRORS;
@@ -178,6 +194,16 @@ private function updateChecksumsForNode(Node $node, $correctChecksum) {
 		);
 	}
 
+	/**
+	 *
+	 * @param Node $node
+	 * @return bool
+	 */
+	private static function fileExistsOnDisk(Node $node) {
+		$statResult = @$node->stat();
+		return \is_array($statResult) && isset($statResult['size']) && $statResult['size'] !== false;
+	}
+
 	/**
 	 * @param $path
 	 * @param IStorage $storage

diff --git a/apps/files/tests/Command/VerifyChecksumsTest.php b/apps/files/tests/Command/VerifyChecksumsTest.php
@@ -6,6 +6,7 @@
 use OC\Files\Storage\Wrapper\Checksum;
 use OCA\Files\Command\VerifyChecksums;
 use OCP\IUser;
+use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Tester\CommandTester;
 use Test\TestCase;
 
@@ -90,7 +91,7 @@ private function createFileForUser($uid, $path, $content) {
 			if (!empty($subDir)) {
 				$currentDir = "$currentDir/$subDir";
 				if (!$userFolder->nodeExists($currentDir)) {
-					$userFolder->newFolder("$currentDir");
+					$userFolder->newFolder($currentDir);
 				}
 			}
 		}
@@ -189,8 +190,8 @@ public function testFilesWithBrokenChecksumsAreDisplayed() {
 
 		$output = $this->cmd->getDisplay();
 
-		$this->assertContains("Mismatch for {$file1->getInternalPath()}", $output);
-		$this->assertContains("Mismatch for {$file2->getInternalPath()}", $output);
+		$this->assertContains($file1->getInternalPath(), $output);
+		$this->assertContains($file2->getInternalPath(), $output);
 		$this->assertContains(self::BROKEN_CHECKSUM_STRING, $output);
 		$this->assertContains($this->testFiles[4]['expectedChecksums'](), $output);
 		$this->assertContains($this->testFiles[7]['expectedChecksums'](), $output);
@@ -270,6 +271,7 @@ public function testOnlyFilesInGivenPathArgumentAreRepaired() {
 	}
 
 	public function testOnlyFilesOfAGivenUserAreRepaired() {
+
 		/** @var File $file1 */
 		$file1 = $this->testFiles[0]['file'];
 		/** @var File $file2 */
@@ -299,4 +301,62 @@ public function testAllFilesCanBeRepaired() {
 
 		$this->assertChecksumsAreCorrect($this->testFiles);
 	}
+
+	public function testFileWithoutChecksumIsIgnored() {
+		/** @var File $file */
+		$file = $this->testFiles[0]['file'];
+		$file->getStorage()->getCache()->update(
+			$file->getId(),
+			['checksum' => '']
+		);
+
+		$this->cmd->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
+		$output = $this->cmd->getDisplay();
+
+		$this->assertContains('somefile.txt => No Checksum', $output);
+		$this->assertContains('Skipping', $output);
+	}
+
+	public function testFileInCacheButNotOnDiskIsIgnored() {
+		/** @var File $file */
+		$file = $this->testFiles[0]['file'];
+		$file->getStorage()->getCache()->update(
+			$file->getId(),
+			['path' => '/does/not/exist', 'name' => 'x-file.txt']
+		);
+
+		$this->cmd->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
+		$output = $this->cmd->getDisplay();
+
+		$this->assertContains('x-file.txt => File is in file-cache but doesn\'t exist on storage/disk', $output);
+	}
+
+	public function testInvalidArgs() {
+		$this->cmd->execute(['-p' => '/does/not/exist']);
+
+		$this->assertEquals(
+			VerifyChecksums::EXIT_INVALID_ARGS,
+			$this->cmd->getStatusCode(),
+			'Not existing path must return invalid args status code'
+		);
+
+		$this->cmd->execute(['-u' => 'doesnotexist@example.com']);
+
+		$this->assertEquals(
+			VerifyChecksums::EXIT_INVALID_ARGS,
+			$this->cmd->getStatusCode(),
+			'Not existing user must return invalid args status code'
+		);
+
+		$this->cmd->execute([
+			'-u' => 'foo@example.com',
+			'-p' => '/some/path'
+		]);
+
+		$this->assertEquals(
+			VerifyChecksums::EXIT_INVALID_ARGS,
+			$this->cmd->getStatusCode(),
+			'User and path must return invalid args status code when combined'
+		);
+	}
 }