From 8cec65908b810637a11e695ba72c418a4c0eb4d9 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 7 Feb 2017 11:06:25 +0100 Subject: [PATCH 1/4] Server side checksum verification #26655 adding file_put_contents implementation to checksum wrapper #26655 fixing bug where stream was not defined #26655 fixing bug where wrapper could not write to memory stream#26655 fixing priority #26655 fixing broken indents adding class description #26655 Integration tests for checksuming #26655 - adding memory capped cache for paths - do not register checksum wrapper for #26655 no mimetype handling code #26655 Fixing failing testExpireOldFilesShared by puting it before testExpireOldFiles. If testExpireOldFiles runs before testExpireOldFilesShared, the second test fails randomly. #26655 Checksum only on write #26655 strong comparison Reverting accidentally commited testcode #26655 Fixing basic lock test fail #26655 Adding checksum download test #26655 Multiple checksums support + tests#26655 Files without checksum in oc_filecache get a checksum on first read #26655 Revert strpos fix #26655 Caching of file id, and various small fixes #26655 Added test about adding a file to local storage Checksum was not well calculated Don't wrap failed fopen in checksum wrapper Fix locking tests in ViewTest Checksums tests removed from webdav-feature Fixed wrong strpos condition Signed-off-by: Morris Jobke --- apps/dav/lib/Connector/Sabre/File.php | 91 ++++++-- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 4 +- apps/files/lib/Capabilities.php | 4 + apps/files_trashbin/tests/TrashbinTest.php | 88 ++++---- apps/files_versions/tests/VersioningTest.php | 101 ++++----- .../integration/features/bootstrap/WebDav.php | 61 ++++++ build/integration/features/checksums.feature | 49 ++--- .../Files/Storage/Wrapper/Checksum.php | 171 +++++++++++++++ lib/private/Files/Stream/Checksum.php | 199 ++++++++++++++++++ lib/private/legacy/util.php | 11 + tests/lib/Files/ViewTest.php | 35 ++- 11 files changed, 674 insertions(+), 140 deletions(-) create mode 100644 lib/private/Files/Storage/Wrapper/Checksum.php create mode 100644 lib/private/Files/Stream/Checksum.php diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 1f878df1564eb..97ba91ee3ffe0 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -34,6 +34,7 @@ namespace OCA\DAV\Connector\Sabre; use OC\Files\Filesystem; +use OC\Files\Storage\Storage; use OCA\DAV\Connector\Sabre\Exception\EntityTooLarge; use OCA\DAV\Connector\Sabre\Exception\FileLocked; use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException; @@ -133,6 +134,10 @@ public function put($data) { list($count, $result) = \OC_Helper::streamCopy($data, $target); fclose($target); + if (!self::isChecksumValid($partStorage, $internalPartPath)) { + throw new BadRequest('The computed checksum does not match the one received from the client.'); + } + if ($result === false) { $expected = -1; if (isset($_SERVER['CONTENT_LENGTH'])) { @@ -222,15 +227,17 @@ public function put($data) { $this->refreshInfo(); - if (isset($request->server['HTTP_OC_CHECKSUM'])) { - $checksum = trim($request->server['HTTP_OC_CHECKSUM']); - $this->fileView->putFileInfo($this->path, ['checksum' => $checksum]); - $this->refreshInfo(); - } else if ($this->getChecksum() !== null && $this->getChecksum() !== '') { - $this->fileView->putFileInfo($this->path, ['checksum' => '']); - $this->refreshInfo(); + $meta = $partStorage->getMetaData($internalPartPath); + + if (isset($meta['checksum'])) { + $this->fileView->putFileInfo( + $this->path, + ['checksum' => $meta['checksum']] + ); } + $this->refreshInfo(); + } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to check file size: " . $e->getMessage()); } @@ -441,6 +448,10 @@ private function createFileChunked($data) { $chunk_handler->file_assemble($partStorage, $partInternalPath); + if (!self::isChecksumValid($partStorage, $partInternalPath)) { + throw new BadRequest('The computed checksum does not match the one received from the client.'); + } + // here is the final atomic rename $renameOkay = $targetStorage->moveFromStorage($partStorage, $partInternalPath, $targetInternalPath); $fileExists = $targetStorage->file_exists($targetInternalPath); @@ -479,13 +490,20 @@ private function createFileChunked($data) { // FIXME: should call refreshInfo but can't because $this->path is not the of the final file $info = $this->fileView->getFileInfo($targetPath); - if (isset($request->server['HTTP_OC_CHECKSUM'])) { - $checksum = trim($request->server['HTTP_OC_CHECKSUM']); - $this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]); - } else if ($info->getChecksum() !== null && $info->getChecksum() !== '') { - $this->fileView->putFileInfo($this->path, ['checksum' => '']); + + if (isset($partStorage) && isset($partInternalPath)) { + $checksums = $partStorage->getMetaData($partInternalPath)['checksum']; + } else { + $checksums = $targetStorage->getMetaData($targetInternalPath)['checksum']; } + $this->fileView->putFileInfo( + $targetPath, + ['checksum' => $checksums] + ); + + $this->refreshInfo(); + $this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED); return $info->getEtag(); @@ -500,6 +518,29 @@ private function createFileChunked($data) { return null; } + /** + * will return true if checksum was not provided in request + * + * @param Storage $storage + * @param $path + * @return bool + */ + private static function isChecksumValid(Storage $storage, $path) { + $meta = $storage->getMetaData($path); + $request = \OC::$server->getRequest(); + + if (!isset($request->server['HTTP_OC_CHECKSUM']) || !isset($meta['checksum'])) { + // No comparison possible, skip the check + return true; + } + + $expectedChecksum = trim($request->server['HTTP_OC_CHECKSUM']); + $computedChecksums = $meta['checksum']; + + return strpos($computedChecksums, $expectedChecksum) !== false; + + } + /** * Returns whether a part file is needed for the given storage * or whether the file can be assembled/uploaded directly on the @@ -562,12 +603,30 @@ private function convertToSabreException(\Exception $e) { throw new \Sabre\DAV\Exception($e->getMessage(), 0, $e); } + /** - * Get the checksum for this file - * + * Set $algo to get a specific checksum, leave null to get all checksums + * (space seperated) + * @param null $algo * @return string */ - public function getChecksum() { - return $this->info->getChecksum(); + public function getChecksum($algo = null) { + $allChecksums = $this->info->getChecksum(); + + if (!$algo) { + return $allChecksums; + } + + $checksums = explode(' ', $allChecksums); + $algoPrefix = strtoupper($algo) . ':'; + + foreach ($checksums as $checksum) { + // starts with $algoPrefix + if (substr($checksum, 0, strlen($algoPrefix)) === $algoPrefix) { + return $checksum; + } + } + + return ''; } } diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 2f86ce5bf4127..00bba474e5884 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -268,7 +268,7 @@ function httpGet(RequestInterface $request, ResponseInterface $response) { if ($node instanceof \OCA\DAV\Connector\Sabre\File) { //Add OC-Checksum header /** @var $node File */ - $checksum = $node->getChecksum(); + $checksum = $node->getChecksum('sha1'); if ($checksum !== null && $checksum !== '') { $response->addHeader('OC-Checksum', $checksum); } @@ -363,7 +363,7 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node) }); $propFind->handle(self::CHECKSUMS_PROPERTYNAME, function() use ($node) { - $checksum = $node->getChecksum(); + $checksum = $node->getChecksum('sha1'); if ($checksum === NULL || $checksum === '') { return null; } diff --git a/apps/files/lib/Capabilities.php b/apps/files/lib/Capabilities.php index 2b6bf57b90dd6..76919f8aa2293 100644 --- a/apps/files/lib/Capabilities.php +++ b/apps/files/lib/Capabilities.php @@ -53,6 +53,10 @@ public function __construct(IConfig $config) { */ public function getCapabilities() { return [ + 'checksums' => [ + 'supportedTypes' => ['SHA1'], + 'preferredUploadType' => 'SHA1' + ], 'files' => [ 'bigfilechunking' => true, 'blacklisted_files' => $this->config->getSystemValue('blacklisted_files', ['.htaccess']), diff --git a/apps/files_trashbin/tests/TrashbinTest.php b/apps/files_trashbin/tests/TrashbinTest.php index 7e4cdb112e86d..fe3c77944143c 100644 --- a/apps/files_trashbin/tests/TrashbinTest.php +++ b/apps/files_trashbin/tests/TrashbinTest.php @@ -147,50 +147,8 @@ protected function tearDown() { parent::tearDown(); } - /** - * test expiration of files older then the max storage time defined for the trash - */ - public function testExpireOldFiles() { - - $currentTime = time(); - $expireAt = $currentTime - 2 * 24 * 60 * 60; - $expiredDate = $currentTime - 3 * 24 * 60 * 60; - - // create some files - \OC\Files\Filesystem::file_put_contents('file1.txt', 'file1'); - \OC\Files\Filesystem::file_put_contents('file2.txt', 'file2'); - \OC\Files\Filesystem::file_put_contents('file3.txt', 'file3'); - - // delete them so that they end up in the trash bin - \OC\Files\Filesystem::unlink('file1.txt'); - \OC\Files\Filesystem::unlink('file2.txt'); - \OC\Files\Filesystem::unlink('file3.txt'); - - //make sure that files are in the trash bin - $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'name'); - $this->assertSame(3, count($filesInTrash)); - // every second file will get a date in the past so that it will get expired - $manipulatedList = $this->manipulateDeleteTime($filesInTrash, $this->trashRoot1, $expiredDate); - - $testClass = new TrashbinForTesting(); - list($sizeOfDeletedFiles, $count) = $testClass->dummyDeleteExpiredFiles($manipulatedList, $expireAt); - - $this->assertSame(10, $sizeOfDeletedFiles); - $this->assertSame(2, $count); - - // only file2.txt should be left - $remainingFiles = array_slice($manipulatedList, $count); - $this->assertSame(1, count($remainingFiles)); - $remainingFile = reset($remainingFiles); - $this->assertSame('file2.txt', $remainingFile['name']); - // check that file1.txt and file3.txt was really deleted - $newTrashContent = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1); - $this->assertSame(1, count($newTrashContent)); - $element = reset($newTrashContent); - $this->assertSame('file2.txt', $element['name']); - } /** * test expiration of files older then the max storage time defined for the trash @@ -268,6 +226,52 @@ public function testExpireOldFilesShared() { $this->verifyArray($filesInTrashUser1AfterDelete, array('user1-2.txt', 'user1-4.txt')); } + /** + * test expiration of files older then the max storage time defined for the trash + */ + public function testExpireOldFiles() { + + $currentTime = time(); + $expireAt = $currentTime - 2 * 24 * 60 * 60; + $expiredDate = $currentTime - 3 * 24 * 60 * 60; + + // create some files + \OC\Files\Filesystem::file_put_contents('file1.txt', 'file1'); + \OC\Files\Filesystem::file_put_contents('file2.txt', 'file2'); + \OC\Files\Filesystem::file_put_contents('file3.txt', 'file3'); + + // delete them so that they end up in the trash bin + \OC\Files\Filesystem::unlink('file1.txt'); + \OC\Files\Filesystem::unlink('file2.txt'); + \OC\Files\Filesystem::unlink('file3.txt'); + + //make sure that files are in the trash bin + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'name'); + $this->assertSame(3, count($filesInTrash)); + + // every second file will get a date in the past so that it will get expired + $manipulatedList = $this->manipulateDeleteTime($filesInTrash, $this->trashRoot1, $expiredDate); + + $testClass = new TrashbinForTesting(); + list($sizeOfDeletedFiles, $count) = $testClass->dummyDeleteExpiredFiles($manipulatedList, $expireAt); + + $this->assertSame(10, $sizeOfDeletedFiles); + $this->assertSame(2, $count); + + // only file2.txt should be left + $remainingFiles = array_slice($manipulatedList, $count); + $this->assertSame(1, count($remainingFiles)); + $remainingFile = reset($remainingFiles); + $this->assertSame('file2.txt', $remainingFile['name']); + + // check that file1.txt and file3.txt was really deleted + $newTrashContent = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1); + $this->assertSame(1, count($newTrashContent)); + $element = reset($newTrashContent); + $this->assertSame('file2.txt', $element['name']); + } + + /** * verify that the array contains the expected results * diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index a1649b2b600d0..2834236fe9e13 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -119,6 +119,57 @@ protected function tearDown() { parent::tearDown(); } + + public function testMoveFileIntoSharedFolderAsRecipient() { + + \OC\Files\Filesystem::mkdir('folder1'); + $fileInfo = \OC\Files\Filesystem::getFileInfo('folder1'); + + $node = \OC::$server->getUserFolder(self::TEST_VERSIONS_USER)->get('folder1'); + $share = \OC::$server->getShareManager()->newShare(); + $share->setNode($node) + ->setShareType(\OCP\Share::SHARE_TYPE_USER) + ->setSharedBy(self::TEST_VERSIONS_USER) + ->setSharedWith(self::TEST_VERSIONS_USER2) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + $share = \OC::$server->getShareManager()->createShare($share); + + self::loginHelper(self::TEST_VERSIONS_USER2); + $versionsFolder2 = '/' . self::TEST_VERSIONS_USER2 . '/files_versions'; + \OC\Files\Filesystem::file_put_contents('test.txt', 'test file'); + + $t1 = time(); + // second version is two weeks older, this way we make sure that no + // version will be expired + $t2 = $t1 - 60 * 60 * 24 * 14; + + $this->rootView->mkdir($versionsFolder2); + // create some versions + $v1 = $versionsFolder2 . '/test.txt.v' . $t1; + $v2 = $versionsFolder2 . '/test.txt.v' . $t2; + + $this->rootView->file_put_contents($v1, 'version1'); + $this->rootView->file_put_contents($v2, 'version2'); + + // move file into the shared folder as recipient + \OC\Files\Filesystem::rename('/test.txt', '/folder1/test.txt'); + + $this->assertFalse($this->rootView->file_exists($v1)); + $this->assertFalse($this->rootView->file_exists($v2)); + + self::loginHelper(self::TEST_VERSIONS_USER); + + $versionsFolder1 = '/' . self::TEST_VERSIONS_USER . '/files_versions'; + + $v1Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t1; + $v2Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t2; + + $this->assertTrue($this->rootView->file_exists($v1Renamed)); + $this->assertTrue($this->rootView->file_exists($v2Renamed)); + + \OC::$server->getShareManager()->deleteShare($share); + } + /** * @medium * test expire logic @@ -380,56 +431,6 @@ public function testMoveFolder() { } - public function testMoveFileIntoSharedFolderAsRecipient() { - - \OC\Files\Filesystem::mkdir('folder1'); - $fileInfo = \OC\Files\Filesystem::getFileInfo('folder1'); - - $node = \OC::$server->getUserFolder(self::TEST_VERSIONS_USER)->get('folder1'); - $share = \OC::$server->getShareManager()->newShare(); - $share->setNode($node) - ->setShareType(\OCP\Share::SHARE_TYPE_USER) - ->setSharedBy(self::TEST_VERSIONS_USER) - ->setSharedWith(self::TEST_VERSIONS_USER2) - ->setPermissions(\OCP\Constants::PERMISSION_ALL); - $share = \OC::$server->getShareManager()->createShare($share); - - self::loginHelper(self::TEST_VERSIONS_USER2); - $versionsFolder2 = '/' . self::TEST_VERSIONS_USER2 . '/files_versions'; - \OC\Files\Filesystem::file_put_contents('test.txt', 'test file'); - - $t1 = time(); - // second version is two weeks older, this way we make sure that no - // version will be expired - $t2 = $t1 - 60 * 60 * 24 * 14; - - $this->rootView->mkdir($versionsFolder2); - // create some versions - $v1 = $versionsFolder2 . '/test.txt.v' . $t1; - $v2 = $versionsFolder2 . '/test.txt.v' . $t2; - - $this->rootView->file_put_contents($v1, 'version1'); - $this->rootView->file_put_contents($v2, 'version2'); - - // move file into the shared folder as recipient - \OC\Files\Filesystem::rename('/test.txt', '/folder1/test.txt'); - - $this->assertFalse($this->rootView->file_exists($v1)); - $this->assertFalse($this->rootView->file_exists($v2)); - - self::loginHelper(self::TEST_VERSIONS_USER); - - $versionsFolder1 = '/' . self::TEST_VERSIONS_USER . '/files_versions'; - - $v1Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t1; - $v2Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t2; - - $this->assertTrue($this->rootView->file_exists($v1Renamed)); - $this->assertTrue($this->rootView->file_exists($v2Renamed)); - - \OC::$server->getShareManager()->deleteShare($share); - } - public function testMoveFolderIntoSharedFolderAsRecipient() { \OC\Files\Filesystem::mkdir('folder1'); diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 3a018a2d0faca..9d5b33ed8ae04 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -516,6 +516,46 @@ public function userUploadsAFileWithContentTo($user, $content, $destination) } } + + /** + * @When user :user uploads file with checksum :checksum and content :content to :destination + * @param $user + * @param $checksum + * @param $content + * @param $destination + */ + public function userUploadsAFileWithChecksumAndContentTo($user, $checksum, $content, $destination) + { + $file = \GuzzleHttp\Stream\Stream::factory($content); + try { + $this->response = $this->makeDavRequest( + $user, + "PUT", + $destination, + ['OC-Checksum' => $checksum], + $file + ); + } catch (\GuzzleHttp\Exception\BadResponseException $e) { + // 4xx and 5xx responses cause an exception + $this->response = $e->getResponse(); + } + } + + + /** + * @Given file :file does not exist for user :user + * @param string $file + * @param $user + */ + public function fileDoesNotExist($file, $user) { + try { + $this->response = $this->makeDavRequest($user, 'DELETE', $file, []); + } catch (\GuzzleHttp\Exception\BadResponseException $e) { + // 4xx and 5xx responses cause an exception + $this->response = $e->getResponse(); + } + } + /** * @When /^User "([^"]*)" deletes (file|folder) "([^"]*)"$/ * @param string $user @@ -581,6 +621,27 @@ public function userUploadsNewChunkFileOfWithToId($user, $num, $data, $id) $this->makeDavRequest($user, 'PUT', $destination, [], $data, "uploads"); } + /** + * @Given user :user uploads new chunk file :num with :data to id :id with checksum :checksum + */ + public function userUploadsNewChunkFileOfWithToIdWithChecksum($user, $num, $data, $id, $checksum) + { + try { + $data = \GuzzleHttp\Stream\Stream::factory($data); + $destination = '/uploads/' . $user . '/' . $id . '/' . $num; + $this->makeDavRequest( + $user, + 'PUT', + $destination, + ['OC-Checksum' => $checksum], + $data, + "uploads" + ); + } catch (\GuzzleHttp\Exception\BadResponseException $ex) { + $this->response = $ex->getResponse(); + } + } + /** * @Given user :user moves new chunk file with id :id to :dest */ diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index d391e93afe8da..c929e13c82ac7 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -9,68 +9,61 @@ Feature: checksums Given user "user0" exists And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" When user "user0" request the checksum of "/myChecksumFile.txt" via propfind - Then The webdav checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Uploading a file with checksum should return the checksum in the download header Given user "user0" exists And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" When user "user0" downloads the file "/myChecksumFile.txt" - Then The header checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + Then The header checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Moving a file with checksum should return the checksum in the propfind Given user "user0" exists And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" When User "user0" moved file "/myChecksumFile.txt" to "/myMovedChecksumFile.txt" And user "user0" request the checksum of "/myMovedChecksumFile.txt" via propfind - Then The webdav checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Moving file with checksum should return the checksum in the download header Given user "user0" exists And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" When User "user0" moved file "/myChecksumFile.txt" to "/myMovedChecksumFile.txt" And user "user0" downloads the file "/myMovedChecksumFile.txt" - Then The header checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + Then The header checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Copying a file with checksum should return the checksum in the propfind Given user "user0" exists And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" When User "user0" copied file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt" And user "user0" request the checksum of "/myChecksumFileCopy.txt" via propfind - Then The webdav checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Copying file with checksum should return the checksum in the download header Given user "user0" exists And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" When User "user0" copied file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt" And user "user0" downloads the file "/myChecksumFileCopy.txt" - Then The header checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" - - Scenario: Overwriting a file with checksum should remove the checksum and not return it in the propfind - Given user "user0" exists - And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" - When user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" - And user "user0" request the checksum of "/myChecksumFile.txt" via propfind - Then The webdav checksum should be empty - - Scenario: Overwriting a file with checksum should remove the checksum and not return it in the download header - Given user "user0" exists - And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" - When user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" - And user "user0" downloads the file "/myChecksumFile.txt" - Then The OC-Checksum header should not be there + Then The header checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Uploading a chunked file with checksum should return the checksum in the propfind Given user "user0" exists - And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" + And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e" + And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e" + And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e" When user "user0" request the checksum of "/myChecksumFile.txt" via propfind - Then The webdav checksum should match "MD5:e892fdd61a74bc89cd05673cc2e22f88" + Then The webdav checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" Scenario: Uploading a chunked file with checksum should return the checksum in the download header Given user "user0" exists - And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" + And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e" + And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e" + And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e" When user "user0" downloads the file "/myChecksumFile.txt" - Then The header checksum should match "MD5:e892fdd61a74bc89cd05673cc2e22f88" + Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" + + Scenario: Downloading a file from local storage has correct checksum + Given user "user0" exists + And file "prueba_cksum.txt" with text "Test file for checksums" is created in local storage + When user "user0" downloads the file "/local_storage/prueba_cksum.txt" + When user "user0" downloads the file "/local_storage/prueba_cksum.txt" + Then The header checksum should match "SHA1:a35b7605c8f586d735435535c337adc066c2ccb6" diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php new file mode 100644 index 0000000000000..902cc504c8dc9 --- /dev/null +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -0,0 +1,171 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ +namespace OC\Files\Storage\Wrapper; + +use Icewind\Streams\CallbackWrapper; +use OC\Cache\CappedMemoryCache; +use OC\Files\Stream\Checksum as ChecksumStream; +use OCP\ILogger; + +/** + * Class Checksum + * + * Computes checksums (default: SHA1, MD5, ADLER32) on all files under the /files path. + * The resulting checksum can be retrieved by call getMetadata($path) + * + * If a file is read and has no checksum oc_filecache gets updated accordingly. + * + * + * @package OC\Files\Storage\Wrapper + */ +class Checksum extends Wrapper { + + + const NOT_REQUIRED = 0; + /** Calculate checksum on write (to be stored in oc_filecache) */ + const PATH_NEW_OR_UPDATED = 1; + /** File needs to be checksummed on first read because it is already in cache but has no checksum */ + const PATH_IN_CACHE_WITHOUT_CHECKSUM = 2; + + /** @var array */ + private $pathsInCacheWithoutChecksum = []; + + /** + * @param string $path + * @param string $mode + * @return false|resource + */ + public function fopen($path, $mode) { + $stream = $this->getWrapperStorage()->fopen($path, $mode); + if (!is_resource($stream)) { + // don't wrap on error + return $stream; + } + + $requirement = $this->getChecksumRequirement($path, $mode); + + if ($requirement === self::PATH_NEW_OR_UPDATED) { + return \OC\Files\Stream\Checksum::wrap($stream, $path); + } + + // If file is without checksum we save the path and create + // a callback because we can only calculate the checksum + // after the client has read the entire filestream once. + // the checksum is then saved to oc_filecache for subsequent + // retrieval (see onClose()) + if ($requirement == self::PATH_IN_CACHE_WITHOUT_CHECKSUM) { + $checksumStream = \OC\Files\Stream\Checksum::wrap($stream, $path); + return CallbackWrapper::wrap( + $checksumStream, + null, + null, + [$this, 'onClose'] + ); + } + + return $stream; + } + + /** + * @param $mode + * @param $path + * @return int + */ + private function getChecksumRequirement($path, $mode) { + $isNormalFile = substr($path, 0, 6) === 'files/'; + $fileIsWritten = $mode !== 'r' && $mode !== 'rb'; + + if ($isNormalFile && $fileIsWritten) { + return self::PATH_NEW_OR_UPDATED; + } + + // file could be in cache but without checksum for example + // if mounted from ext. storage + $cache = $this->getCache($path); + $cacheEntry = $cache->get($path); + + if ($cacheEntry && empty($cacheEntry['checksum'])) { + $this->pathsInCacheWithoutChecksum[$cacheEntry->getId()] = $path; + return self::PATH_IN_CACHE_WITHOUT_CHECKSUM; + } + + return self::NOT_REQUIRED; + } + + /** + * Callback registered in fopen + */ + public function onClose() { + $cache = $this->getCache(); + foreach ($this->pathsInCacheWithoutChecksum as $cacheId => $path) { + $cache->update( + $cacheId, + ['checksum' => self::getChecksumsInDbFormat($path)] + ); + } + + $this->pathsInCacheWithoutChecksum = []; + } + + /** + * @param $path + * Format like "SHA1:abc MD5:def ADLER32:ghi" + * @return string + */ + private static function getChecksumsInDbFormat($path) { + $checksumString = ''; + foreach (ChecksumStream::getChecksums($path) as $algo => $checksum) { + $checksumString .= sprintf('%s:%s ', strtoupper($algo), $checksum); + } + + return rtrim($checksumString); + } + + /** + * @param string $path + * @param string $data + * @return bool + */ + public function file_put_contents($path, $data) { + $memoryStream = fopen('php://memory', 'r+'); + $checksumStream = \OC\Files\Stream\Checksum::wrap($memoryStream, $path); + + fwrite($checksumStream, $data); + fclose($checksumStream); + + return $this->getWrapperStorage()->file_put_contents($path, $data); + } + + /** + * @param string $path + * @return array + */ + public function getMetaData($path) { + $parentMetaData = $this->getWrapperStorage()->getMetaData($path); + $parentMetaData['checksum'] = self::getChecksumsInDbFormat($path); + + if (!isset($parentMetaData['mimetype'])) { + $parentMetaData['mimetype'] = 'application/octet-stream'; + } + + return $parentMetaData; + } +} diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php new file mode 100644 index 0000000000000..06157b3c0a39d --- /dev/null +++ b/lib/private/Files/Stream/Checksum.php @@ -0,0 +1,199 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Files\Stream; + + +use Icewind\Streams\Wrapper; +use OC\Cache\CappedMemoryCache; + +/** + * Computes the checksum of the wrapped stream. The checksum can be retrieved with + * getChecksum after the stream is closed. + * + * + * @package OC\Files\Stream + */ +class Checksum extends Wrapper { + + /** + * To stepwise compute a hash on a continuous stream + * of data a "context" is required which stores the intermediate + * hash result while the stream has not finished. + * + * After the stream ends the hashing contexts needs to be finalized + * to compute the final checksum. + * + * @var resource[] + */ + private $hashingContexts; + + /** @var CappedMemoryCache Key is path, value is array of checksums */ + private static $checksums; + + + public function __construct(array $algos = ['sha1', 'md5', 'adler32']) { + + foreach ($algos as $algo) { + $this->hashingContexts[$algo] = hash_init($algo); + } + + if (!self::$checksums) { + self::$checksums = new CappedMemoryCache(); + } + } + + + /** + * @param $source + * @param $path + * @return resource + */ + public static function wrap($source, $path) { + $context = stream_context_create([ + 'occhecksum' => [ + 'source' => $source, + 'path' => $path + ] + ]); + + return Wrapper::wrapSource( + $source, $context, 'occhecksum', self::class + ); + } + + + /** + * @param string $path + * @param array $options + * @return bool + */ + public function dir_opendir($path, $options) { + return true; + } + + /** + * @param string $path + * @param string $mode + * @param int $options + * @param string $opened_path + * @return bool + */ + public function stream_open($path, $mode, $options, &$opened_path) { + $context = parent::loadContext('occhecksum'); + $this->setSourceStream($context['source']); + + return true; + } + + /** + * @param int $count + * @return string + */ + public function stream_read($count) { + $data = parent::stream_read($count); + $this->updateHashingContexts($data); + + return $data; + } + + /** + * @param string $data + * @return int + */ + public function stream_write($data) { + $this->updateHashingContexts($data); + + return parent::stream_write($data); + } + + private function updateHashingContexts($data) { + foreach ($this->hashingContexts as $ctx) { + hash_update($ctx, $data); + } + } + + /** + * @return bool + */ + public function stream_close() { + $currentPath = $this->getPathFromStreamContext(); + self::$checksums[$currentPath] = $this->finalizeHashingContexts(); + + return parent::stream_close(); + } + + /** + * @return array + */ + private function finalizeHashingContexts() { + $hashes = []; + + foreach ($this->hashingContexts as $algo => $ctx) { + $hashes[$algo] = hash_final($ctx); + } + + return $hashes; + } + + public function dir_closedir() { + if (!isset($this->source)) { + return false; + } + return parent::dir_closedir(); + } + + /** + * @return mixed + * @return string + */ + private function getPathFromStreamContext() { + $ctx = stream_context_get_options($this->context); + + return $ctx['occhecksum']['path']; + } + + /** + * @param $path + * @return array + */ + public static function getChecksums($path) { + if (!isset(self::$checksums[$path])) { + return []; + } + + return self::$checksums[$path]; + } + + /** + * For debugging + * + * @return CappedMemoryCache + */ + public static function getChecksumsForAllPaths() { + if (!self::$checksums) { + self::$checksums = new CappedMemoryCache(); + } + + return self::$checksums; + } +} diff --git a/lib/private/legacy/util.php b/lib/private/legacy/util.php index 7f351c5b00e35..a22df0ecf36f0 100644 --- a/lib/private/legacy/util.php +++ b/lib/private/legacy/util.php @@ -180,6 +180,17 @@ public static function setupFS($user = '') { return $storage; }); + // install storage checksum wrapper + \OC\Files\Filesystem::addStorageWrapper('oc_checksum', function ($mountPoint, \OCP\Files\Storage\IStorage $storage) { + if (!$storage->instanceOfStorage('\OCA\Files_Sharing\SharedStorage')) { + return new \OC\Files\Storage\Wrapper\Checksum(['storage' => $storage]); + } + + return $storage; + + }, 1); + + \OC\Files\Filesystem::addStorageWrapper('oc_encoding', function ($mountPoint, \OCP\Files\Storage $storage, \OCP\Files\Mount\IMountPoint $mount) { if ($mount->getOption('encoding_compatibility', false) && !$storage->instanceOfStorage('\OCA\Files_Sharing\SharedStorage') && !$storage->isLocal()) { return new \OC\Files\Storage\Wrapper\Encoding(['storage' => $storage]); diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index f1e1ee7d417a0..d85969e7d194f 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -1755,6 +1755,8 @@ public function basicOperationProviderForLocks() { ILockingProvider::LOCK_SHARED, ILockingProvider::LOCK_SHARED, null, + // shared lock stays until fclose + ILockingProvider::LOCK_SHARED, ], [ 'opendir', @@ -1829,9 +1831,12 @@ public function testLockBasicOperation( $storage->expects($this->once()) ->method($operation) ->will($this->returnCallback( - function () use ($view, $lockedPath, &$lockTypeDuring) { + function () use ($view, $lockedPath, &$lockTypeDuring, $operation) { $lockTypeDuring = $this->getFileLockType($view, $lockedPath); + if ($operation === 'fopen') { + return fopen('data://text/plain,test', 'r'); + } return true; } )); @@ -1841,7 +1846,7 @@ function () use ($view, $lockedPath, &$lockTypeDuring) { $this->connectMockHooks($hookType, $view, $lockedPath, $lockTypePre, $lockTypePost); // do operation - call_user_func_array(array($view, $operation), $operationArgs); + $result = call_user_func_array([$view, $operation], $operationArgs); if ($hookType !== null) { $this->assertEquals($expectedLockBefore, $lockTypePre, 'File locked properly during pre-hook'); @@ -1852,6 +1857,13 @@ function () use ($view, $lockedPath, &$lockTypeDuring) { } $this->assertEquals($expectedStrayLock, $this->getFileLockType($view, $lockedPath)); + + if (is_resource($result)) { + fclose($result); + + // lock is cleared after fclose + $this->assertNull($this->getFileLockType($view, $lockedPath)); + } } /** @@ -2624,4 +2636,23 @@ public function testCreateParentDirectoriesWithExistingFile() { ->willReturn(true); $this->assertFalse(self::invokePrivate($view, 'createParentDirectories', ['/file.txt/folder/structure'])); } + + public function testFopenFail() { + // since stream wrappers influence the streams, + // this test makes sure that all stream wrappers properly return a failure + // to the caller instead of wrapping the boolean + /** @var Temporary | \PHPUnit_Framework_MockObject_MockObject $storage */ + $storage = $this->getMockBuilder(Temporary::class) + ->setMethods(['fopen']) + ->getMock(); + + $storage->expects($this->once()) + ->method('fopen') + ->willReturn(false); + + Filesystem::mount($storage, [], '/'); + $view = new View('/files'); + $result = $view->fopen('unexist.txt', 'r'); + $this->assertFalse($result); + } } From 4fbb73a3ad5272d439736746200c5511f64950e6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 24 Mar 2017 15:48:39 +0100 Subject: [PATCH 2/4] cleaner checksum check in test Signed-off-by: Robin Appelman --- .../features/bootstrap/ChecksumsContext.php | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/build/integration/features/bootstrap/ChecksumsContext.php b/build/integration/features/bootstrap/ChecksumsContext.php index 4dd43db852f65..0f00d40593d25 100644 --- a/build/integration/features/bootstrap/ChecksumsContext.php +++ b/build/integration/features/bootstrap/ChecksumsContext.php @@ -25,6 +25,7 @@ use GuzzleHttp\Client; use GuzzleHttp\Message\ResponseInterface; +use Sabre\DAV\Xml\Response\MultiStatus; class ChecksumsContext implements \Behat\Behat\Context\Context { /** @var string */ @@ -140,19 +141,19 @@ public function userRequestTheChecksumOfViaPropfind($user, $path) * @param string $checksum * @throws \Exception */ - public function theWebdavChecksumShouldMatch($checksum) - { - $service = new Sabre\Xml\Service(); + public function theWebdavChecksumShouldMatch($checksum) { + $service = new Sabre\DAV\Xml\Service(); + /** @var MultiStatus $parsed */ $parsed = $service->parse($this->response->getBody()->getContents()); - /* - * Fetch the checksum array - * Maybe we want to do this a bit cleaner ;) - */ - $checksums = $parsed[0]['value'][1]['value'][0]['value'][0]; - - if ($checksums['value'][0]['value'] !== $checksum) { - throw new \Exception("Expected $checksum, got ".$checksums['value'][0]['value']); + $props = $parsed->getResponses()[0]->getResponseProperties(); + if (isset($props[200]) && isset($props[200]['{http://owncloud.org/ns}checksums'])) { + $checksums = $props[200]['{http://owncloud.org/ns}checksums'][0]; + if ($checksums['value'] !== $checksum) { + throw new \Exception("Expected $checksum, got " . $checksums['value']); + } + } else { + throw new \Exception('No checksum provided'); } } From 9935c62f81310cd98c27310080957c678de144e9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 24 Mar 2017 17:07:00 +0100 Subject: [PATCH 3/4] cleanup checksum wrappers Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/File.php | 6 +- lib/private/Files/Cache/Cache.php | 1 - .../Files/Storage/Wrapper/Checksum.php | 86 ++++++------------- lib/private/Files/Stream/Checksum.php | 68 +++------------ 4 files changed, 39 insertions(+), 122 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 97ba91ee3ffe0..c21b65017a8c3 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -227,7 +227,7 @@ public function put($data) { $this->refreshInfo(); - $meta = $partStorage->getMetaData($internalPartPath); + $meta = $partStorage->getCache()->get($internalPartPath); if (isset($meta['checksum'])) { $this->fileView->putFileInfo( @@ -492,9 +492,9 @@ private function createFileChunked($data) { if (isset($partStorage) && isset($partInternalPath)) { - $checksums = $partStorage->getMetaData($partInternalPath)['checksum']; + $checksums = $partStorage->getCache()->get($partInternalPath)['checksum']; } else { - $checksums = $targetStorage->getMetaData($targetInternalPath)['checksum']; + $checksums = $targetStorage->getCache()->get($targetInternalPath)['checksum']; } $this->fileView->putFileInfo( diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 1f3f2433e45cc..27943341bd6ff 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -256,7 +256,6 @@ public function insert($file, array $data) { return -1; } } - $data['path'] = $file; $data['parent'] = $this->getParentId($file); $data['name'] = \OC_Util::basename($file); diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 902cc504c8dc9..3ee331ad90015 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -18,11 +18,13 @@ * along with this program. If not, see * */ + namespace OC\Files\Storage\Wrapper; use Icewind\Streams\CallbackWrapper; use OC\Cache\CappedMemoryCache; use OC\Files\Stream\Checksum as ChecksumStream; +use OCP\Files\IHomeStorage; use OCP\ILogger; /** @@ -45,9 +47,6 @@ class Checksum extends Wrapper { /** File needs to be checksummed on first read because it is already in cache but has no checksum */ const PATH_IN_CACHE_WITHOUT_CHECKSUM = 2; - /** @var array */ - private $pathsInCacheWithoutChecksum = []; - /** * @param string $path * @param string $mode @@ -62,35 +61,31 @@ public function fopen($path, $mode) { $requirement = $this->getChecksumRequirement($path, $mode); - if ($requirement === self::PATH_NEW_OR_UPDATED) { - return \OC\Files\Stream\Checksum::wrap($stream, $path); - } - - // If file is without checksum we save the path and create - // a callback because we can only calculate the checksum - // after the client has read the entire filestream once. - // the checksum is then saved to oc_filecache for subsequent - // retrieval (see onClose()) - if ($requirement == self::PATH_IN_CACHE_WITHOUT_CHECKSUM) { - $checksumStream = \OC\Files\Stream\Checksum::wrap($stream, $path); - return CallbackWrapper::wrap( - $checksumStream, - null, - null, - [$this, 'onClose'] - ); + if ($requirement === self::PATH_NEW_OR_UPDATED || + $requirement === self::PATH_IN_CACHE_WITHOUT_CHECKSUM + ) { + return $this->wrapChecksumStream($stream, $path); } return $stream; } + private function wrapChecksumStream($stream, $path) { + return \OC\Files\Stream\Checksum::wrap($stream, function (array $hashes) use ($path) { + $cache = $this->getCache(); + $cache->put($path, [ + 'checksum' => self::getChecksumsInDbFormat($hashes) + ]); + }); + } + /** * @param $mode * @param $path * @return int */ private function getChecksumRequirement($path, $mode) { - $isNormalFile = substr($path, 0, 6) === 'files/'; + $isNormalFile = (!$this->getWrapperStorage() instanceof IHomeStorage) || strpos($path, 'files/') === 0; $fileIsWritten = $mode !== 'r' && $mode !== 'rb'; if ($isNormalFile && $fileIsWritten) { @@ -103,7 +98,6 @@ private function getChecksumRequirement($path, $mode) { $cacheEntry = $cache->get($path); if ($cacheEntry && empty($cacheEntry['checksum'])) { - $this->pathsInCacheWithoutChecksum[$cacheEntry->getId()] = $path; return self::PATH_IN_CACHE_WITHOUT_CHECKSUM; } @@ -111,28 +105,12 @@ private function getChecksumRequirement($path, $mode) { } /** - * Callback registered in fopen - */ - public function onClose() { - $cache = $this->getCache(); - foreach ($this->pathsInCacheWithoutChecksum as $cacheId => $path) { - $cache->update( - $cacheId, - ['checksum' => self::getChecksumsInDbFormat($path)] - ); - } - - $this->pathsInCacheWithoutChecksum = []; - } - - /** - * @param $path - * Format like "SHA1:abc MD5:def ADLER32:ghi" + * @param array $hashes * @return string */ - private static function getChecksumsInDbFormat($path) { + private static function getChecksumsInDbFormat(array $hashes) { $checksumString = ''; - foreach (ChecksumStream::getChecksums($path) as $algo => $checksum) { + foreach ($hashes as $algo => $checksum) { $checksumString .= sprintf('%s:%s ', strtoupper($algo), $checksum); } @@ -145,27 +123,13 @@ private static function getChecksumsInDbFormat($path) { * @return bool */ public function file_put_contents($path, $data) { - $memoryStream = fopen('php://memory', 'r+'); - $checksumStream = \OC\Files\Stream\Checksum::wrap($memoryStream, $path); - - fwrite($checksumStream, $data); - fclose($checksumStream); - - return $this->getWrapperStorage()->file_put_contents($path, $data); - } - - /** - * @param string $path - * @return array - */ - public function getMetaData($path) { - $parentMetaData = $this->getWrapperStorage()->getMetaData($path); - $parentMetaData['checksum'] = self::getChecksumsInDbFormat($path); - - if (!isset($parentMetaData['mimetype'])) { - $parentMetaData['mimetype'] = 'application/octet-stream'; + $fh = $this->fopen($path, 'w'); + if (!$fh) { + return false; } + fwrite($fh, $data); + fclose($fh); - return $parentMetaData; + return true; } } diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index 06157b3c0a39d..975119bb28ca0 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -45,34 +45,29 @@ class Checksum extends Wrapper { * * @var resource[] */ - private $hashingContexts; - - /** @var CappedMemoryCache Key is path, value is array of checksums */ - private static $checksums; + private $hashingContexts; + /** @var callable */ + private $callback; public function __construct(array $algos = ['sha1', 'md5', 'adler32']) { foreach ($algos as $algo) { $this->hashingContexts[$algo] = hash_init($algo); } - - if (!self::$checksums) { - self::$checksums = new CappedMemoryCache(); - } } /** - * @param $source - * @param $path + * @param resource $source + * @param callable $callback * @return resource */ - public static function wrap($source, $path) { + public static function wrap($source, callable $callback) { $context = stream_context_create([ 'occhecksum' => [ 'source' => $source, - 'path' => $path + 'callback' => $callback ] ]); @@ -88,7 +83,7 @@ public static function wrap($source, $path) { * @return bool */ public function dir_opendir($path, $options) { - return true; + return false; } /** @@ -101,6 +96,7 @@ public function dir_opendir($path, $options) { public function stream_open($path, $mode, $options, &$opened_path) { $context = parent::loadContext('occhecksum'); $this->setSourceStream($context['source']); + $this->callback = $context['callback']; return true; } @@ -136,8 +132,8 @@ private function updateHashingContexts($data) { * @return bool */ public function stream_close() { - $currentPath = $this->getPathFromStreamContext(); - self::$checksums[$currentPath] = $this->finalizeHashingContexts(); + $callback = $this->callback; + $callback($this->finalizeHashingContexts()); return parent::stream_close(); } @@ -154,46 +150,4 @@ private function finalizeHashingContexts() { return $hashes; } - - public function dir_closedir() { - if (!isset($this->source)) { - return false; - } - return parent::dir_closedir(); - } - - /** - * @return mixed - * @return string - */ - private function getPathFromStreamContext() { - $ctx = stream_context_get_options($this->context); - - return $ctx['occhecksum']['path']; - } - - /** - * @param $path - * @return array - */ - public static function getChecksums($path) { - if (!isset(self::$checksums[$path])) { - return []; - } - - return self::$checksums[$path]; - } - - /** - * For debugging - * - * @return CappedMemoryCache - */ - public static function getChecksumsForAllPaths() { - if (!self::$checksums) { - self::$checksums = new CappedMemoryCache(); - } - - return self::$checksums; - } } From 6b0fc89d3f33aa6a115755d53e8561da257e17b6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 4 Apr 2017 13:25:18 +0200 Subject: [PATCH 4/4] fix error when trying to remove non existing entry from cache Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Cache.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 27943341bd6ff..3e97ba284af39 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -438,6 +438,9 @@ public function inCache($file) { */ public function remove($file) { $entry = $this->get($file); + if (!$entry || !$entry['fileid']) { + return; + } $sql = 'DELETE FROM `*PREFIX*filecache` WHERE `fileid` = ?'; $this->connection->executeQuery($sql, array($entry['fileid'])); if ($entry['mimetype'] === 'httpd/unix-directory') {