From 6605874f97fad39361c9907c2b87ccf142ec998e Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 7 Feb 2017 11:06:25 +0100 Subject: [PATCH 01/41] Server side checksum verification #26655 --- apps/dav/lib/Connector/Sabre/File.php | 36 ++++ apps/files/lib/Capabilities.php | 4 + .../Files/Storage/Wrapper/Checksum.php | 88 ++++++++++ lib/private/Files/Stream/Checksum.php | 161 ++++++++++++++++++ lib/private/legacy/util.php | 6 + 5 files changed, 295 insertions(+) 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..ca7c63fc41340 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'])) { @@ -441,6 +446,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); @@ -500,6 +509,33 @@ 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']); + $computedChecksum = $meta['checksum']; + + if ($expectedChecksum !== $computedChecksum) { + return false; + } + + return true; + } + /** * Returns whether a part file is needed for the given storage * or whether the file can be assembled/uploaded directly on the 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/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php new file mode 100644 index 0000000000000..780ecc435d450 --- /dev/null +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -0,0 +1,88 @@ + + * + * @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 OC\Files\Stream\Checksum as ChecksumStream; + +/** + * Class Checksum + * + * Computes checksums (default: SHA1) on all files under the /files path. + * The resulting checksum can be retrieved by call getMetadata($path) + * + * @package OC\Files\Storage\Wrapper + */ +class Checksum extends Wrapper { + + /** + * @param array $parameters + */ + public function __construct($parameters) { + if (isset($parameters['algo'])) { + ChecksumStream::setAlgo($parameters['algo']); + } + parent::__construct($parameters); + } + + /** + * @param string $path + * @param string $mode + * @return false|resource + */ + public function fopen($path, $mode) { + $stream = $this->getWrapperStorage()->fopen($path, $mode); + if (!self::requiresChecksum($path)) { + return $stream; + } + + return \OC\Files\Stream\Checksum::wrap($stream, $path); + } + + + /** + * Checksum is only required for everything under files/ + * @param $path + * @return bool + */ + private static function requiresChecksum($path) { + return substr($path, 0, 6) === 'files/'; + } + + /** + * @param string $path + * @param string $data + * @return bool + */ + public function file_put_contents($path, $data) { + return parent::file_put_contents($path, $data); + } + + /** + * @param string $path + * @return array + */ + public function getMetaData($path) { + $parentMetaData = parent::getMetaData($path); + $parentMetaData['checksum'] = ChecksumStream::getChecksum($path); + + return $parentMetaData; + } +} \ No newline at end of file diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php new file mode 100644 index 0000000000000..b3442d940244b --- /dev/null +++ b/lib/private/Files/Stream/Checksum.php @@ -0,0 +1,161 @@ + + * + * @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; + +class Checksum extends Wrapper { + + private static $algo = 'sha1'; + + /** @var resource */ + private $hashCtx; + + /** @var array Key is path, value is the checksum */ + private static $checksums = []; + + public function __construct() { + $this->hashCtx = hash_init(self::$algo); + } + + + /** + * @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); + hash_update($this->hashCtx, $data); + + return $data; + } + + /** + * @param string $data + * @return int + */ + public function stream_write($data) { + hash_update($this->hashCtx, $data); + + return parent::stream_write($data); + } + + /** + * @return bool + */ + public function stream_close() { + self::$checksums[$this->getPathFromContext()] = sprintf( + '%s:%s', strtoupper(self::$algo), hash_final($this->hashCtx) + ); + + return parent::stream_close(); + } + + /** + * @return mixed + * @return string + */ + private function getPathFromContext() { + $ctx = stream_context_get_options($this->context); + + return $ctx['occhecksum']['path']; + } + + /** + * @param $path + * @return string + */ + public static function getChecksum($path) { + if (!isset(self::$checksums[$path])) { + return null; + } + + return self::$checksums[$path]; + } + + /** + * @return array + */ + public static function getChecksums() { + return self::$checksums; + } + + /** + * @return string + */ + public static function getAlgo() { + return self::$algo; + } + + /** + * @param string $algo + */ + public static function setAlgo($algo) { + self::$algo = $algo; + } + + +} \ No newline at end of file diff --git a/lib/private/legacy/util.php b/lib/private/legacy/util.php index 9516a67af48c0..1ce7604aa8cb5 100644 --- a/lib/private/legacy/util.php +++ b/lib/private/legacy/util.php @@ -180,6 +180,12 @@ public static function setupFS($user = '') { return $storage; }); + // install storage checksum wrapper + \OC\Files\Filesystem::addStorageWrapper('oc_checksum', function ($mountPoint, $storage) { + return new \OC\Files\Storage\Wrapper\Checksum(['storage' => $storage]); + }); + + \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]); From 87a3331f2ace55dbaaee904794cda86a0042a711 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 7 Feb 2017 14:57:40 +0100 Subject: [PATCH 02/41] adding file_put_contents implementation to checksum wrapper #26655 --- lib/private/Files/Storage/Wrapper/Checksum.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 780ecc435d450..66ccb637bcf0c 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -72,6 +72,10 @@ private static function requiresChecksum($path) { * @return bool */ public function file_put_contents($path, $data) { + $stream = fopen('occhecksum://','r+'); + fwrite($stream, $data); + fclose($stream); + return parent::file_put_contents($path, $data); } From cedc3d31fcc56ffca6bbb32e021e3c2f1fd302ac Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 7 Feb 2017 15:11:05 +0100 Subject: [PATCH 03/41] fixing bug where stream was not defined #26655 --- lib/private/Files/Storage/Wrapper/Checksum.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 66ccb637bcf0c..d900473041c0d 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -72,7 +72,7 @@ private static function requiresChecksum($path) { * @return bool */ public function file_put_contents($path, $data) { - $stream = fopen('occhecksum://','r+'); + $stream = $this->fopen('php://memory','r+'); fwrite($stream, $data); fclose($stream); From b109f0bfd3f05b7cdb511b3c5deed388450e3c77 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 7 Feb 2017 15:42:20 +0100 Subject: [PATCH 04/41] fixing bug where wrapper could not write to memory stream#26655 --- lib/private/Files/Storage/Wrapper/Checksum.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index d900473041c0d..deda95ba3897d 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -72,9 +72,11 @@ private static function requiresChecksum($path) { * @return bool */ public function file_put_contents($path, $data) { - $stream = $this->fopen('php://memory','r+'); - fwrite($stream, $data); - fclose($stream); + $memoryStream = fopen('php://memory', 'r+'); + $checksumStream = \OC\Files\Stream\Checksum::wrap($memoryStream, $path); + + fwrite($checksumStream, $data); + fclose($checksumStream); return parent::file_put_contents($path, $data); } From 9c9551721ae52d8aafcd9723d995f63447e21dcd Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 7 Feb 2017 17:55:44 +0100 Subject: [PATCH 05/41] fixing priority #26655 --- lib/private/legacy/util.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/legacy/util.php b/lib/private/legacy/util.php index 1ce7604aa8cb5..8e283ef8cb8be 100644 --- a/lib/private/legacy/util.php +++ b/lib/private/legacy/util.php @@ -183,7 +183,7 @@ public static function setupFS($user = '') { // install storage checksum wrapper \OC\Files\Filesystem::addStorageWrapper('oc_checksum', function ($mountPoint, $storage) { return new \OC\Files\Storage\Wrapper\Checksum(['storage' => $storage]); - }); + }, 1); \OC\Files\Filesystem::addStorageWrapper('oc_encoding', function ($mountPoint, \OCP\Files\Storage $storage, \OCP\Files\Mount\IMountPoint $mount) { From 8ed6854b363d794ca24570526c2b69cf69a7e1fa Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 7 Feb 2017 18:06:27 +0100 Subject: [PATCH 06/41] fixing broken indents --- lib/private/Files/Stream/Checksum.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index b3442d940244b..014b2c0ad1fff 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -77,7 +77,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->setSourceStream($context['source']); return true; } @@ -107,7 +107,7 @@ public function stream_write($data) { * @return bool */ public function stream_close() { - self::$checksums[$this->getPathFromContext()] = sprintf( + self::$checksums[$this->getPathFromContext()] = sprintf( '%s:%s', strtoupper(self::$algo), hash_final($this->hashCtx) ); @@ -156,6 +156,4 @@ public static function getAlgo() { public static function setAlgo($algo) { self::$algo = $algo; } - - -} \ No newline at end of file +} From d126d10108d10c4180281da0b47de9f6c2d16ba9 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 7 Feb 2017 18:09:37 +0100 Subject: [PATCH 07/41] adding class description #26655 --- lib/private/Files/Stream/Checksum.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index 014b2c0ad1fff..cd4bf4659c8fa 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -25,6 +25,13 @@ use Icewind\Streams\Wrapper; +/** + * 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 { private static $algo = 'sha1'; From fbddc810fe82f48cfd6a6d9da2b302af3da3f959 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 8 Feb 2017 14:53:16 +0100 Subject: [PATCH 08/41] Integration tests for checksuming #26655 --- .../integration/features/bootstrap/WebDav.php | 40 ++++++++++++++++++- .../features/webdav-related.feature | 12 ++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 3a018a2d0faca..221a6c1172358 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -84,7 +84,7 @@ public function makeDavRequest($user, $method, $path, $headers, $body = null, $t $fullUrl = substr($this->baseUrl, 0, -4) . $this->getDavFilesPath($user) . "$path"; } else if ( $type === "uploads" ){ $fullUrl = substr($this->baseUrl, 0, -4) . $this->davPath . "$path"; - } + } $client = new GClient(); $options = []; if ($user === 'admin') { @@ -486,6 +486,44 @@ public function userUploadsAFileTo($user, $source, $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 :user adds a file of :bytes bytes to :destination * @param string $user diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index 6aee59036d3b0..3dc686d3b0bf3 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -407,6 +407,18 @@ Feature: webdav-related And Downloading file "/myChunkedFile.txt" Then Downloaded content should be "AAAAABBBBBCCCCC" + Scenario: Upload a file where checksum does not match + Given user "user0" exists + And file "/chksumtst.txt" does not exist for user "user0" + And user "user0" uploads file with checksum "SHA1:f005ba11" and content "Some Text" to "/chksumtst.txt" + Then the HTTP status code should be "400" + + Scenario: Upload a file where checksum does match + Given user "user0" exists + And file "/chksumtst.txt" does not exist for user "user0" + And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" + Then the HTTP status code should be "201" + Scenario: A disabled user cannot use webdav Given user "userToBeDisabled" exists And As an "admin" From 2cfc921bac2e08d2fa3eb069979dc6f2058493c1 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 8 Feb 2017 15:06:48 +0100 Subject: [PATCH 09/41] - adding memory capped cache for paths - do not register checksum wrapper for #26655 --- lib/private/Files/Stream/Checksum.php | 16 +++++++++++++--- lib/private/legacy/util.php | 9 +++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index cd4bf4659c8fa..d1ded83008de8 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -24,6 +24,7 @@ use Icewind\Streams\Wrapper; +use OC\Cache\CappedMemoryCache; /** * Computes the checksum of the wrapped stream. The checksum can be retrieved with @@ -39,11 +40,16 @@ class Checksum extends Wrapper { /** @var resource */ private $hashCtx; - /** @var array Key is path, value is the checksum */ - private static $checksums = []; + /** @var CappedMemoryCache Key is path, value is the checksum */ + private static $checksums; + public function __construct() { $this->hashCtx = hash_init(self::$algo); + + if (!self::$checksums) { + self::$checksums = new CappedMemoryCache(); + } } @@ -144,9 +150,13 @@ public static function getChecksum($path) { } /** - * @return array + * @return CappedMemoryCache */ public static function getChecksums() { + 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 8e283ef8cb8be..9fb974b47f086 100644 --- a/lib/private/legacy/util.php +++ b/lib/private/legacy/util.php @@ -181,8 +181,13 @@ public static function setupFS($user = '') { }); // install storage checksum wrapper - \OC\Files\Filesystem::addStorageWrapper('oc_checksum', function ($mountPoint, $storage) { - return new \OC\Files\Storage\Wrapper\Checksum(['storage' => $storage]); + \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); From 677b9f9e7f97da072f9ebbe89ddcdc30894a8e7a Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 8 Feb 2017 17:19:20 +0100 Subject: [PATCH 10/41] no mimetype handling code #26655 --- lib/private/Files/Storage/Wrapper/Checksum.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index deda95ba3897d..1afa65a85ca25 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -89,6 +89,11 @@ public function getMetaData($path) { $parentMetaData = parent::getMetaData($path); $parentMetaData['checksum'] = ChecksumStream::getChecksum($path); + // Need to investigate more + if (!isset($parentMetaData['mimetype'])) { + $parentMetaData['mimetype'] = ''; + } + return $parentMetaData; } } \ No newline at end of file From a5da6116def24924971577a8334634683eda6fda Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 9 Feb 2017 15:11:54 +0100 Subject: [PATCH 11/41] Fixing failing testExpireOldFilesShared by puting it before testExpireOldFiles. If testExpireOldFiles runs before testExpireOldFilesShared, the second test fails randomly. #26655 --- apps/files_trashbin/tests/TrashbinTest.php | 88 +++++++++++----------- 1 file changed, 46 insertions(+), 42 deletions(-) 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 * From 866ddb2da73f99727e89d49feebf9c0660898951 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 9 Feb 2017 15:15:42 +0100 Subject: [PATCH 12/41] Checksum only on write #26655 --- lib/private/Files/Storage/Wrapper/Checksum.php | 11 ++++++----- lib/private/Files/Stream/Checksum.php | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 1afa65a85ca25..66994fe4d101f 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -49,7 +49,7 @@ public function __construct($parameters) { */ public function fopen($path, $mode) { $stream = $this->getWrapperStorage()->fopen($path, $mode); - if (!self::requiresChecksum($path)) { + if (!self::requiresChecksum($path, $mode)) { return $stream; } @@ -59,11 +59,12 @@ public function fopen($path, $mode) { /** * Checksum is only required for everything under files/ + * @param $mode * @param $path * @return bool */ - private static function requiresChecksum($path) { - return substr($path, 0, 6) === 'files/'; + private static function requiresChecksum($path, $mode) { + return substr($path, 0, 6) === 'files/' && $mode != 'r'; } /** @@ -78,7 +79,7 @@ public function file_put_contents($path, $data) { fwrite($checksumStream, $data); fclose($checksumStream); - return parent::file_put_contents($path, $data); + return $this->getWrapperStorage()->file_put_contents($path, $data); } /** @@ -86,7 +87,7 @@ public function file_put_contents($path, $data) { * @return array */ public function getMetaData($path) { - $parentMetaData = parent::getMetaData($path); + $parentMetaData = $this->getWrapperStorage()->getMetaData($path); $parentMetaData['checksum'] = ChecksumStream::getChecksum($path); // Need to investigate more diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index d1ded83008de8..ae622d2fc7a7c 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -78,7 +78,7 @@ public static function wrap($source, $path) { * @return bool */ public function dir_opendir($path, $options) { - return true; + return false; } /** From cb3a969a55295a8aede85441bc45c9899534035f Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 9 Feb 2017 15:16:16 +0100 Subject: [PATCH 13/41] strong comparison --- lib/private/Files/Storage/Wrapper/Checksum.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 66994fe4d101f..0015fcd16ff1d 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -64,7 +64,7 @@ public function fopen($path, $mode) { * @return bool */ private static function requiresChecksum($path, $mode) { - return substr($path, 0, 6) === 'files/' && $mode != 'r'; + return substr($path, 0, 6) === 'files/' && $mode !== 'r'; } /** From f03db14549baee1b2631c81fc41619890921c215 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 9 Feb 2017 15:36:53 +0100 Subject: [PATCH 14/41] Reverting accidentally commited testcode #26655 --- lib/private/Files/Stream/Checksum.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index ae622d2fc7a7c..d1ded83008de8 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -78,7 +78,7 @@ public static function wrap($source, $path) { * @return bool */ public function dir_opendir($path, $options) { - return false; + return true; } /** From 16a8188f961ca213aea0a1a2dadc879097b8e46c Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 9 Feb 2017 17:58:01 +0100 Subject: [PATCH 15/41] Fixing basic lock test fail #26655 --- lib/private/Files/Stream/Checksum.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index d1ded83008de8..821fc056fa526 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -127,6 +127,13 @@ public function stream_close() { return parent::stream_close(); } + public function dir_closedir() { + if (!isset($this->source)) { + return false; + } + return parent::dir_closedir(); + } + /** * @return mixed * @return string From af297321e95bbed75ea0f81cf4c8a8840f0fc2d0 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 9 Feb 2017 19:12:16 +0100 Subject: [PATCH 16/41] #26655 Moved MoveFileIntoSharedFolderAsRecepient up because it has some weird sideeffects if other tests are running before it, couldn`t find out the root cause, test runs ok in isolation. --- apps/files_versions/tests/VersioningTest.php | 101 ++++++++++--------- 1 file changed, 51 insertions(+), 50 deletions(-) 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'); From c17ca392f78b1a0d5a961113eed6254d93bf9528 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 9 Feb 2017 20:09:07 +0100 Subject: [PATCH 17/41] Adding checksum download test #26655 --- build/integration/features/webdav-related.feature | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index 3dc686d3b0bf3..721dea74f55f3 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -419,6 +419,14 @@ Feature: webdav-related And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" Then the HTTP status code should be "201" + Scenario: Uploaded file should have the same checksum when downloaded + Given user "user0" exists + And file "/chksumtst.txt" does not exist for user "user0" + And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" + When Downloading file "/chksumtst.txt" as "user0" + Then The following headers should be set + | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 | + Scenario: A disabled user cannot use webdav Given user "userToBeDisabled" exists And As an "admin" From e43b50acf780c6a4d84d6960782423341191f140 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Fri, 10 Feb 2017 19:59:38 +0100 Subject: [PATCH 18/41] Multiple checksums support + tests#26655 --- apps/dav/lib/Connector/Sabre/File.php | 39 ++++++----- build/integration/features/checksums.feature | 42 ++++------- .../features/webdav-related.feature | 2 +- .../Files/Storage/Wrapper/Checksum.php | 17 ++--- lib/private/Files/Stream/Checksum.php | 70 ++++++++++--------- 5 files changed, 80 insertions(+), 90 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index ca7c63fc41340..9507268d7a545 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -227,14 +227,12 @@ 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(); - } + $this->fileView->putFileInfo( + $this->path, + ['checksum' => $partStorage->getMetaData($internalPartPath)['checksum']] + ); + + $this->refreshInfo(); } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to check file size: " . $e->getMessage()); @@ -488,13 +486,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(); @@ -520,20 +525,16 @@ 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']); - $computedChecksum = $meta['checksum']; + $computedChecksums = $meta['checksum']; - if ($expectedChecksum !== $computedChecksum) { - return false; - } + return strpos($computedChecksums, $expectedChecksum) !== false; - return true; } /** diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index d391e93afe8da..303fd43bba168 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -9,68 +9,54 @@ 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 MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" 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 MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" 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 MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" 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 MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" 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 MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" 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 MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" 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 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df" 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 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df" diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index 721dea74f55f3..862cf48691346 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -425,7 +425,7 @@ Feature: webdav-related And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" When Downloading file "/chksumtst.txt" as "user0" Then The following headers should be set - | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 | + | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 MD5:56e57920c3c8c727bfe7a5288cdf61c4 ADLER32:1048035a | Scenario: A disabled user cannot use webdav Given user "userToBeDisabled" exists diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 0015fcd16ff1d..917df77a84dbd 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -32,15 +32,6 @@ */ class Checksum extends Wrapper { - /** - * @param array $parameters - */ - public function __construct($parameters) { - if (isset($parameters['algo'])) { - ChecksumStream::setAlgo($parameters['algo']); - } - parent::__construct($parameters); - } /** * @param string $path @@ -87,8 +78,14 @@ public function file_put_contents($path, $data) { * @return array */ public function getMetaData($path) { + $checksumString = ''; + + foreach (ChecksumStream::getChecksums($path) as $algo => $checksum) { + $checksumString .= sprintf('%s:%s ', strtoupper($algo), $checksum); + } + $parentMetaData = $this->getWrapperStorage()->getMetaData($path); - $parentMetaData['checksum'] = ChecksumStream::getChecksum($path); + $parentMetaData['checksum'] = rtrim($checksumString); // Need to investigate more if (!isset($parentMetaData['mimetype'])) { diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index 821fc056fa526..ef564ed959d85 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -35,17 +35,18 @@ */ class Checksum extends Wrapper { - private static $algo = 'sha1'; + /** @var resource[] */ + private $hashingContexts; - /** @var resource */ - private $hashCtx; - - /** @var CappedMemoryCache Key is path, value is the checksum */ + /** @var CappedMemoryCache Key is path, value is array of checksums */ private static $checksums; - public function __construct() { - $this->hashCtx = hash_init(self::$algo); + 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(); @@ -101,7 +102,7 @@ public function stream_open($path, $mode, $options, &$opened_path) { */ public function stream_read($count) { $data = parent::stream_read($count); - hash_update($this->hashCtx, $data); + $this->updateHashingContexts($data); return $data; } @@ -111,22 +112,39 @@ public function stream_read($count) { * @return int */ public function stream_write($data) { - hash_update($this->hashCtx, $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() { - self::$checksums[$this->getPathFromContext()] = sprintf( - '%s:%s', strtoupper(self::$algo), hash_final($this->hashCtx) - ); + $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; @@ -138,7 +156,7 @@ public function dir_closedir() { * @return mixed * @return string */ - private function getPathFromContext() { + private function getPathFromStreamContext() { $ctx = stream_context_get_options($this->context); return $ctx['occhecksum']['path']; @@ -146,38 +164,26 @@ private function getPathFromContext() { /** * @param $path - * @return string + * @return array */ - public static function getChecksum($path) { + public static function getChecksums($path) { if (!isset(self::$checksums[$path])) { - return null; + return []; } return self::$checksums[$path]; } /** + * For debugging + * * @return CappedMemoryCache */ - public static function getChecksums() { + public static function getChecksumsForAllPaths() { if (!self::$checksums) { self::$checksums = new CappedMemoryCache(); } return self::$checksums; } - - /** - * @return string - */ - public static function getAlgo() { - return self::$algo; - } - - /** - * @param string $algo - */ - public static function setAlgo($algo) { - self::$algo = $algo; - } } From 45e9d34a7b54fd91797596f14e111dfa5f4c2f76 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Mon, 13 Feb 2017 15:23:33 +0100 Subject: [PATCH 19/41] #26655 eventual mimetype error fix --- lib/private/Files/Storage/Wrapper/Checksum.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 917df77a84dbd..3b3c5ffa6e07e 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -88,10 +88,14 @@ public function getMetaData($path) { $parentMetaData['checksum'] = rtrim($checksumString); // Need to investigate more + + if (!isset($parentMetaData['mimetype'])) { - $parentMetaData['mimetype'] = ''; + $parentMetaData['mimetype'] = 'application/octet-stream'; } + + return $parentMetaData; } } \ No newline at end of file From f21e2fd6137cdf3a9e4cb7789a1de5e2db4f6061 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 14 Feb 2017 15:13:51 +0100 Subject: [PATCH 20/41] Webdav returns only one checksum (SHA1) --- apps/dav/lib/Connector/Sabre/File.php | 26 ++++++++++++++++--- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 4 +-- build/integration/features/checksums.feature | 16 ++++++------ .../features/webdav-related.feature | 2 +- .../Files/Storage/Wrapper/Checksum.php | 5 ---- 5 files changed, 33 insertions(+), 20 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 9507268d7a545..39b630c41cdc4 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -599,12 +599,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); + $algo = strtoupper($algo); + + foreach ($checksums as $checksum) { + // starts with $algo + if (substr($checksum, 0, strlen($algo)) === $algo) { + return $checksum; + } + } + + return ''; } } diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 1b1f43df9eac4..bd4c55b140165 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -265,7 +265,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); } @@ -360,7 +360,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/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index 303fd43bba168..1a0739df7ee0e 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -9,41 +9,41 @@ 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" + 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" + 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" + 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" + 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" + 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" + 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 @@ -51,7 +51,7 @@ Feature: checksums 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 "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df" + 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 @@ -59,4 +59,4 @@ Feature: checksums 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 "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df" + Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index 862cf48691346..721dea74f55f3 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -425,7 +425,7 @@ Feature: webdav-related And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" When Downloading file "/chksumtst.txt" as "user0" Then The following headers should be set - | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 MD5:56e57920c3c8c727bfe7a5288cdf61c4 ADLER32:1048035a | + | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 | Scenario: A disabled user cannot use webdav Given user "userToBeDisabled" exists diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 3b3c5ffa6e07e..6aaf14b34fcd3 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -87,15 +87,10 @@ public function getMetaData($path) { $parentMetaData = $this->getWrapperStorage()->getMetaData($path); $parentMetaData['checksum'] = rtrim($checksumString); - // Need to investigate more - - if (!isset($parentMetaData['mimetype'])) { $parentMetaData['mimetype'] = 'application/octet-stream'; } - - return $parentMetaData; } } \ No newline at end of file From 981f4f3f5e1e5adc51e05277d28f7c6c00dc2157 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 15 Feb 2017 14:02:45 +0100 Subject: [PATCH 21/41] Various smaller improvements, additional upload chunked with checksum test --- apps/dav/lib/Connector/Sabre/File.php | 20 +++++++++++------- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 2 +- .../integration/features/bootstrap/WebDav.php | 21 +++++++++++++++++++ .../features/webdav-related.feature | 10 +++++++++ .../Files/Storage/Wrapper/Checksum.php | 3 ++- lib/private/Files/Stream/Checksum.php | 12 ++++++++++- 6 files changed, 57 insertions(+), 11 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 39b630c41cdc4..2fc7b72f9b353 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -227,10 +227,14 @@ public function put($data) { $this->refreshInfo(); - $this->fileView->putFileInfo( - $this->path, - ['checksum' => $partStorage->getMetaData($internalPartPath)['checksum']] - ); + $meta = $partStorage->getMetaData($internalPartPath); + + if (isset($meta['checksum'])) { + $this->fileView->putFileInfo( + $this->path, + ['checksum' => $meta['checksum']] + ); + } $this->refreshInfo(); @@ -533,7 +537,7 @@ private static function isChecksumValid(Storage $storage, $path) { $expectedChecksum = trim($request->server['HTTP_OC_CHECKSUM']); $computedChecksums = $meta['checksum']; - return strpos($computedChecksums, $expectedChecksum) !== false; + return strpos($computedChecksums, $expectedChecksum) == true; } @@ -614,11 +618,11 @@ public function getChecksum($algo = null) { } $checksums = explode(' ', $allChecksums); - $algo = strtoupper($algo); + $algoPrefix = strtoupper($algo) . ':'; foreach ($checksums as $checksum) { - // starts with $algo - if (substr($checksum, 0, strlen($algo)) === $algo) { + // starts with $algoPrefix + if (substr($checksum, 0, strlen($algoPrefix)) === $algoPrefix) { return $checksum; } } diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index bd4c55b140165..b240d62304ba1 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -360,7 +360,7 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node) }); $propFind->handle(self::CHECKSUMS_PROPERTYNAME, function() use ($node) { - $checksum = $node->getChecksum('SHA1'); + $checksum = $node->getChecksum('sha1'); if ($checksum === NULL || $checksum === '') { return null; } diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 221a6c1172358..4df1288dcb7d8 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -584,6 +584,27 @@ public function userCreatedAFolder($user, $destination) { } } + /** + * @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 uploads chunk file :num of :total with :data to :destination * @param string $user diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index 721dea74f55f3..8a59aa4eeb444 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -407,6 +407,16 @@ Feature: webdav-related And Downloading file "/myChunkedFile.txt" Then Downloaded content should be "AAAAABBBBBCCCCC" + Scenario: Upload chunked file where checksum does not match + Given using new dav path + And user "user0" exists + And user "user0" creates a new chunking upload with id "chunking-42" + And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42" + And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42" with checksum "SHA1:f005ba11" + And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42" + And user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt" + Then the HTTP status code should be "400" + Scenario: Upload a file where checksum does not match Given user "user0" exists And file "/chksumtst.txt" does not exist for user "user0" diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 6aaf14b34fcd3..99efa68462a82 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -55,7 +55,8 @@ public function fopen($path, $mode) { * @return bool */ private static function requiresChecksum($path, $mode) { - return substr($path, 0, 6) === 'files/' && $mode !== 'r'; + return substr($path, 0, 6) === 'files/' + && $mode !== 'r' && $mode !== 'rb'; } /** diff --git a/lib/private/Files/Stream/Checksum.php b/lib/private/Files/Stream/Checksum.php index ef564ed959d85..06157b3c0a39d 100644 --- a/lib/private/Files/Stream/Checksum.php +++ b/lib/private/Files/Stream/Checksum.php @@ -35,7 +35,16 @@ */ class Checksum extends Wrapper { - /** @var resource[] */ + /** + * 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 */ @@ -113,6 +122,7 @@ public function stream_read($count) { */ public function stream_write($data) { $this->updateHashingContexts($data); + return parent::stream_write($data); } From 2ef9897f2271383c5494d8ae3f15530d794b19d5 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 16 Feb 2017 10:17:52 +0100 Subject: [PATCH 22/41] #26655 strpos bugfix --- apps/dav/lib/Connector/Sabre/File.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 2fc7b72f9b353..c40b2a9036a89 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -537,7 +537,7 @@ private static function isChecksumValid(Storage $storage, $path) { $expectedChecksum = trim($request->server['HTTP_OC_CHECKSUM']); $computedChecksums = $meta['checksum']; - return strpos($computedChecksums, $expectedChecksum) == true; + return strpos($computedChecksums, $expectedChecksum) === true; } From b174da8b89071c16f93f169b1db4d606f57c9577 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Fri, 17 Feb 2017 14:03:29 +0100 Subject: [PATCH 23/41] Files without checksum in oc_filecache get a checksum on first read #26655 --- .../Files/Storage/Wrapper/Checksum.php | 106 +++++++++++++++--- 1 file changed, 89 insertions(+), 17 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 99efa68462a82..2d1ba2e0cb7c8 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -20,19 +20,34 @@ */ namespace OC\Files\Storage\Wrapper; +use Icewind\Streams\CallbackWrapper; +use OC\Cache\CappedMemoryCache; use OC\Files\Stream\Checksum as ChecksumStream; /** * Class Checksum * - * Computes checksums (default: SHA1) on all files under the /files path. + * 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 @@ -40,23 +55,86 @@ class Checksum extends Wrapper { */ public function fopen($path, $mode) { $stream = $this->getWrapperStorage()->fopen($path, $mode); - if (!self::requiresChecksum($path, $mode)) { - return $stream; + $requirement = $this->getChecksumRequirement($path, $mode); + + if ($requirement === self::PATH_NEW_OR_UPDATED) { + return \OC\Files\Stream\Checksum::wrap($stream, $path); } - return \OC\Files\Stream\Checksum::wrap($stream, $path); - } + // If file is without checksum we save the path and create + // a callback becaause 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) { + $this->pathsInCacheWithoutChecksum[] = $path; + $checksumStream = \OC\Files\Stream\Checksum::wrap($stream, $path); + return CallbackWrapper::wrap( + $checksumStream, + null, + null, + [$this, 'onClose'] + ); + } + return $stream; + } /** - * Checksum is only required for everything under files/ * @param $mode * @param $path - * @return bool + * @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); + + if ($cache->inCache($path)) { + $cacheEntry = $cache->get($path); + if (empty($cacheEntry['checksum'])) { + return self::PATH_IN_CACHE_WITHOUT_CHECKSUM; + } + } + + return self::NOT_REQUIRED; + } + + /** + * Callback registered in fopen */ - private static function requiresChecksum($path, $mode) { - return substr($path, 0, 6) === 'files/' - && $mode !== 'r' && $mode !== 'rb'; + public function onClose() { + $cache = $this->getCache(); + foreach ($this->pathsInCacheWithoutChecksum as $path) { + $entry = $cache->get($path); + $cache->update( + $entry->getId(), + ['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); } /** @@ -79,14 +157,8 @@ public function file_put_contents($path, $data) { * @return array */ public function getMetaData($path) { - $checksumString = ''; - - foreach (ChecksumStream::getChecksums($path) as $algo => $checksum) { - $checksumString .= sprintf('%s:%s ', strtoupper($algo), $checksum); - } - $parentMetaData = $this->getWrapperStorage()->getMetaData($path); - $parentMetaData['checksum'] = rtrim($checksumString); + $parentMetaData['checksum'] = self::getChecksumsInDbFormat($path); if (!isset($parentMetaData['mimetype'])) { $parentMetaData['mimetype'] = 'application/octet-stream'; From 80cd139a3bba054166421d3011aaea1e43049b19 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Fri, 17 Feb 2017 14:16:54 +0100 Subject: [PATCH 24/41] Revert strpos fix #26655 --- apps/dav/lib/Connector/Sabre/File.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index c40b2a9036a89..0074772c11777 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -537,7 +537,7 @@ private static function isChecksumValid(Storage $storage, $path) { $expectedChecksum = trim($request->server['HTTP_OC_CHECKSUM']); $computedChecksums = $meta['checksum']; - return strpos($computedChecksums, $expectedChecksum) === true; + return strpos($computedChecksums, $expectedChecksum) !== true; } From cfdf25fc0e4aa6c25977665afa1c92f3343b4e3c Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 22 Feb 2017 11:32:47 +0100 Subject: [PATCH 25/41] Caching of file id, and various small fixes #26655 --- .../Files/Storage/Wrapper/Checksum.php | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 2d1ba2e0cb7c8..78f4da3062e32 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -23,6 +23,7 @@ use Icewind\Streams\CallbackWrapper; use OC\Cache\CappedMemoryCache; use OC\Files\Stream\Checksum as ChecksumStream; +use OCP\ILogger; /** * Class Checksum @@ -44,7 +45,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 = []; @@ -62,12 +62,11 @@ public function fopen($path, $mode) { } // If file is without checksum we save the path and create - // a callback becaause we can only calculate the checksum + // 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) { - $this->pathsInCacheWithoutChecksum[] = $path; $checksumStream = \OC\Files\Stream\Checksum::wrap($stream, $path); return CallbackWrapper::wrap( $checksumStream, @@ -96,12 +95,11 @@ private function getChecksumRequirement($path, $mode) { // file could be in cache but without checksum for example // if mounted from ext. storage $cache = $this->getCache($path); + $cacheEntry = $cache->get($path); - if ($cache->inCache($path)) { - $cacheEntry = $cache->get($path); - if (empty($cacheEntry['checksum'])) { - return self::PATH_IN_CACHE_WITHOUT_CHECKSUM; - } + if ($cacheEntry && empty($cacheEntry['checksum'])) { + $this->pathsInCacheWithoutChecksum[$cacheEntry->getId()] = $path; + return self::PATH_IN_CACHE_WITHOUT_CHECKSUM; } return self::NOT_REQUIRED; @@ -112,10 +110,9 @@ private function getChecksumRequirement($path, $mode) { */ public function onClose() { $cache = $this->getCache(); - foreach ($this->pathsInCacheWithoutChecksum as $path) { - $entry = $cache->get($path); + foreach ($this->pathsInCacheWithoutChecksum as $cacheId => $path) { $cache->update( - $entry->getId(), + $cacheId, ['checksum' => self::getChecksumsInDbFormat($path)] ); } From 86ba432e27edcab09f99b320f60a34f4aa7e8cad Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 22 Feb 2017 11:32:47 +0100 Subject: [PATCH 26/41] Added test about adding a file to local storage --- build/integration/features/checksums.feature | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index 1a0739df7ee0e..2010863d6bdbf 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -60,3 +60,10 @@ Feature: checksums 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 "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:b14628561796cb8bd049f036bff7948d39a0180a" From 7b1f6d9b1a78c50f9c1c4417255e9f267065bc12 Mon Sep 17 00:00:00 2001 From: Sergio Bertolin Date: Tue, 7 Mar 2017 13:04:09 +0000 Subject: [PATCH 27/41] Checksum was not well calculated --- build/integration/features/checksums.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index 2010863d6bdbf..c929e13c82ac7 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -66,4 +66,4 @@ Feature: checksums 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:b14628561796cb8bd049f036bff7948d39a0180a" + Then The header checksum should match "SHA1:a35b7605c8f586d735435535c337adc066c2ccb6" From 9932b41863f1ebfcdb12a3a478471819e0be7c0d Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 9 Mar 2017 16:12:38 +0100 Subject: [PATCH 28/41] Don't wrap failed fopen in checksum wrapper --- lib/private/Files/Storage/Wrapper/Checksum.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 78f4da3062e32..902cc504c8dc9 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -55,6 +55,11 @@ class Checksum extends Wrapper { */ 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) { @@ -163,4 +168,4 @@ public function getMetaData($path) { return $parentMetaData; } -} \ No newline at end of file +} From 96848915b50d6b1a9023c5a607a16ba05fae5dc2 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 9 Mar 2017 16:31:51 +0100 Subject: [PATCH 29/41] Fix locking tests in ViewTest --- tests/lib/Files/ViewTest.php | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) 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 5db5d60d91caed1592b2fa597f1ce614ebdd223d Mon Sep 17 00:00:00 2001 From: Sergio Bertolin Date: Fri, 10 Mar 2017 09:05:41 +0000 Subject: [PATCH 30/41] Checksums tests removed from webdav-feature --- .../features/webdav-related.feature | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index 8a59aa4eeb444..6aee59036d3b0 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -407,36 +407,6 @@ Feature: webdav-related And Downloading file "/myChunkedFile.txt" Then Downloaded content should be "AAAAABBBBBCCCCC" - Scenario: Upload chunked file where checksum does not match - Given using new dav path - And user "user0" exists - And user "user0" creates a new chunking upload with id "chunking-42" - And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42" - And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42" with checksum "SHA1:f005ba11" - And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42" - And user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt" - Then the HTTP status code should be "400" - - Scenario: Upload a file where checksum does not match - Given user "user0" exists - And file "/chksumtst.txt" does not exist for user "user0" - And user "user0" uploads file with checksum "SHA1:f005ba11" and content "Some Text" to "/chksumtst.txt" - Then the HTTP status code should be "400" - - Scenario: Upload a file where checksum does match - Given user "user0" exists - And file "/chksumtst.txt" does not exist for user "user0" - And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" - Then the HTTP status code should be "201" - - Scenario: Uploaded file should have the same checksum when downloaded - Given user "user0" exists - And file "/chksumtst.txt" does not exist for user "user0" - And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" - When Downloading file "/chksumtst.txt" as "user0" - Then The following headers should be set - | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 | - Scenario: A disabled user cannot use webdav Given user "userToBeDisabled" exists And As an "admin" From 5be65472b0886349a23039ae4a00f74f289c13d8 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Fri, 10 Mar 2017 10:23:07 +0100 Subject: [PATCH 31/41] Fixed wrong strpos condition --- apps/dav/lib/Connector/Sabre/File.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 0074772c11777..97ba91ee3ffe0 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -537,7 +537,7 @@ private static function isChecksumValid(Storage $storage, $path) { $expectedChecksum = trim($request->server['HTTP_OC_CHECKSUM']); $computedChecksums = $meta['checksum']; - return strpos($computedChecksums, $expectedChecksum) !== true; + return strpos($computedChecksums, $expectedChecksum) !== false; } From be8d61f0ec77eb99691335845ba3ad8e8512a9a6 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Fri, 17 Mar 2017 10:28:10 +0100 Subject: [PATCH 32/41] Fixing failing integration tests with checksum and encryption: OC_TEST_ALT_HOME=1 OC_TEST_ENCRYPTION_ENABLED=1 ./run.sh features/sharing-v1.feature:538 --- lib/private/Files/Storage/Wrapper/Checksum.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 902cc504c8dc9..151ab6f204159 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -23,6 +23,7 @@ use Icewind\Streams\CallbackWrapper; use OC\Cache\CappedMemoryCache; use OC\Files\Stream\Checksum as ChecksumStream; +use OC\OCS\Exception; use OCP\ILogger; /** @@ -100,9 +101,12 @@ private function getChecksumRequirement($path, $mode) { // 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'])) { + // Cache entry is sometimes an array (partial) when encryption is enabled without id so + // we ignore it. + if ($cacheEntry && empty($cacheEntry['checksum']) && is_object($cacheEntry)) { $this->pathsInCacheWithoutChecksum[$cacheEntry->getId()] = $path; return self::PATH_IN_CACHE_WITHOUT_CHECKSUM; } From 3f731f3650e1c4c40d13ca8ca7a78bd203b960da Mon Sep 17 00:00:00 2001 From: Sergio Bertolin Date: Thu, 9 Mar 2017 12:47:12 +0000 Subject: [PATCH 33/41] Removed checksums context and integrate it with the rest --- build/integration/config/behat.yml | 2 - .../features/bootstrap/BasicStructure.php | 1 + .../{ChecksumsContext.php => Checksums.php} | 104 +++++------------- .../integration/features/bootstrap/WebDav.php | 2 +- build/integration/features/checksums.feature | 30 +++-- 5 files changed, 51 insertions(+), 88 deletions(-) rename build/integration/features/bootstrap/{ChecksumsContext.php => Checksums.php} (64%) diff --git a/build/integration/config/behat.yml b/build/integration/config/behat.yml index 3573f9d6a6b93..7882966b20a45 100644 --- a/build/integration/config/behat.yml +++ b/build/integration/config/behat.yml @@ -20,8 +20,6 @@ default: baseUrl: http://localhost:8080 - CalDavContext: baseUrl: http://localhost:8080 - - ChecksumsContext: - baseUrl: http://localhost:8080 - CommandLineContext: baseUrl: http://localhost:8080 ocPath: ../../ diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index 9e7aa272d0aff..aeaacb932e5ec 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -35,6 +35,7 @@ trait BasicStructure { use Auth; + use Checksums; use Trashbin; /** @var string */ diff --git a/build/integration/features/bootstrap/ChecksumsContext.php b/build/integration/features/bootstrap/Checksums.php similarity index 64% rename from build/integration/features/bootstrap/ChecksumsContext.php rename to build/integration/features/bootstrap/Checksums.php index 4dd43db852f65..d00311a14b2a9 100644 --- a/build/integration/features/bootstrap/ChecksumsContext.php +++ b/build/integration/features/bootstrap/Checksums.php @@ -1,71 +1,24 @@ - * @author Roeland Jago Douma - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * 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 - * along with this program. If not, see . - * - */ -require __DIR__ . '/../../vendor/autoload.php'; +require __DIR__ . '/../../../../lib/composer/autoload.php'; use GuzzleHttp\Client; use GuzzleHttp\Message\ResponseInterface; -class ChecksumsContext implements \Behat\Behat\Context\Context { - /** @var string */ - private $baseUrl; - /** @var Client */ - private $client; - /** @var ResponseInterface */ - private $response; - - /** - * @param string $baseUrl - */ - public function __construct($baseUrl) { - $this->baseUrl = $baseUrl; - - // in case of ci deployment we take the server url from the environment - $testServerUrl = getenv('TEST_SERVER_URL'); - if ($testServerUrl !== false) { - $this->baseUrl = substr($testServerUrl, 0, -5); - } - } - - /** @BeforeScenario */ - public function setUpScenario() { - $this->client = new Client(); - } - - /** @AfterScenario */ - public function tearDownScenario() { - } +trait Checksums { + use Webdav; /** * @param string $userName * @return string */ private function getPasswordForUser($userName) { - if($userName === 'admin') { - return 'admin'; + if ($userName === 'admin') { + return $this->adminUser; + } else { + return $this->regularUser; } - return '123456'; } /** @@ -75,12 +28,12 @@ private function getPasswordForUser($userName) { * @param string $destination * @param string $checksum */ - public function userUploadsFileToWithChecksum($user, $source, $destination, $checksum) - { + public function userUploadsFileToWithChecksum($user, $source, $destination, $checksum) { + $client = new Client(); $file = \GuzzleHttp\Stream\Stream::factory(fopen($source, 'r')); try { - $this->response = $this->client->put( - $this->baseUrl . '/remote.php/webdav' . $destination, + $this->response = $client->put( + substr($this->baseUrl, 0, -4) . $this->davPath . $destination, [ 'auth' => [ $user, @@ -114,11 +67,11 @@ public function theWebdavResponseShouldHaveAStatusCode($statusCode) { * @param string $user * @param string $path */ - public function userRequestTheChecksumOfViaPropfind($user, $path) - { - $request = $this->client->createRequest( + public function userRequestTheChecksumOfViaPropfind($user, $path) { + $client = new Client(); + $request = $client->createRequest( 'PROPFIND', - $this->baseUrl . '/remote.php/webdav' . $path, + substr($this->baseUrl, 0, -4) . $this->davPath . $path, [ 'body' => ' @@ -132,7 +85,7 @@ public function userRequestTheChecksumOfViaPropfind($user, $path) ] ] ); - $this->response = $this->client->send($request); + $this->response = $client->send($request); } /** @@ -161,10 +114,10 @@ public function theWebdavChecksumShouldMatch($checksum) * @param string $user * @param string $path */ - public function userDownloadsTheFile($user, $path) - { - $this->response = $this->client->get( - $this->baseUrl . '/remote.php/webdav' . $path, + public function userDownloadsTheFile($user, $path) { + $client = new Client(); + $this->response = $client->get( + substr($this->baseUrl, 0, -4) . $this->davPath . $path, [ 'auth' => [ $user, @@ -192,22 +145,22 @@ public function theHeaderChecksumShouldMatch($checksum) * @param string $source * @param string $destination */ - public function userCopiedFileTo($user, $source, $destination) - { - $request = $this->client->createRequest( + public function userCopiedFileTo($user, $source, $destination) { + $client = new Client(); + $request = $client->createRequest( 'MOVE', - $this->baseUrl . '/remote.php/webdav' . $source, + substr($this->baseUrl, 0, -4) . $this->davPath . $source, [ 'auth' => [ $user, $this->getPasswordForUser($user), ], 'headers' => [ - 'Destination' => $this->baseUrl . '/remote.php/webdav' . $destination, + 'Destination' => substr($this->baseUrl, 0, -4) . $this->davPath . $destination, ], ] ); - $this->response = $this->client->send($request); + $this->response = $client->send($request); } /** @@ -250,9 +203,10 @@ public function theOcChecksumHeaderShouldNotBeThere() */ public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) { + $client = new Client(); $num -= 1; - $this->response = $this->client->put( - $this->baseUrl . '/remote.php/webdav' . $destination . '-chunking-42-'.$total.'-'.$num, + $this->response = $client->put( + substr($this->baseUrl, 0, -4) . $this->davPath . $destination . '-chunking-42-'.$total.'-'.$num, [ 'auth' => [ $user, diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 4df1288dcb7d8..5805e497f4db2 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -613,7 +613,7 @@ public function userUploadsNewChunkFileOfWithToIdWithChecksum($user, $num, $data * @param string $data * @param string $destination */ - public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination) + public function userUploadsChunkedFile($user, $num, $total, $data, $destination) { $num -= 1; $data = \GuzzleHttp\Stream\Stream::factory($data); diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index c929e13c82ac7..b721521ba4baa 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -1,52 +1,60 @@ Feature: checksums Scenario: Uploading a file with checksum should work - Given user "user0" exists + Given using old dav path + And user "user0" exists When user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" Then The webdav response should have a status code "201" Scenario: Uploading a file with checksum should return the checksum in the propfind - Given user "user0" exists + Given using old dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Uploading a file with checksum should return the checksum in the download header - Given user "user0" exists + Given using old dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Moving a file with checksum should return the checksum in the propfind - Given user "user0" exists + Given using old dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Moving file with checksum should return the checksum in the download header - Given user "user0" exists + Given using old dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Copying a file with checksum should return the checksum in the propfind - Given user "user0" exists + Given using old dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Copying file with checksum should return the checksum in the download header - Given user "user0" exists + Given using old dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" Scenario: Uploading a chunked file with checksum should return the checksum in the propfind - Given user "user0" exists + Given using old dav path + And user "user0" exists 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" @@ -54,7 +62,8 @@ Feature: checksums 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 + Given using old dav path + And user "user0" exists 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" @@ -62,7 +71,8 @@ Feature: checksums Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" Scenario: Downloading a file from local storage has correct checksum - Given user "user0" exists + Given using old dav path + And 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" From 12b3b5f8341a1a15484b4c97418f4359c3c54e9b Mon Sep 17 00:00:00 2001 From: Sergio Bertolin Date: Thu, 9 Mar 2017 14:27:06 +0000 Subject: [PATCH 34/41] Adapted tests to use new dav endpoint --- .../features/bootstrap/Checksums.php | 73 ++++----------- .../integration/features/bootstrap/WebDav.php | 13 +++ build/integration/features/checksums.feature | 88 ++++++++++++++++--- 3 files changed, 105 insertions(+), 69 deletions(-) diff --git a/build/integration/features/bootstrap/Checksums.php b/build/integration/features/bootstrap/Checksums.php index d00311a14b2a9..2a0f51d9369c4 100644 --- a/build/integration/features/bootstrap/Checksums.php +++ b/build/integration/features/bootstrap/Checksums.php @@ -29,26 +29,13 @@ private function getPasswordForUser($userName) { * @param string $checksum */ public function userUploadsFileToWithChecksum($user, $source, $destination, $checksum) { - $client = new Client(); $file = \GuzzleHttp\Stream\Stream::factory(fopen($source, 'r')); - try { - $this->response = $client->put( - substr($this->baseUrl, 0, -4) . $this->davPath . $destination, - [ - 'auth' => [ - $user, - $this->getPasswordForUser($user) - ], - 'body' => $file, - 'headers' => [ - 'OC-Checksum' => $checksum - ] - ] - ); - } catch (\GuzzleHttp\Exception\ServerException $e) { - // 4xx and 5xx responses cause an exception - $this->response = $e->getResponse(); - } + $this->response = $this->makeDavRequest($user, + 'PUT', + $destination, + ['OC-Checksum' => $checksum], + $file, + "files"); } /** @@ -71,7 +58,7 @@ public function userRequestTheChecksumOfViaPropfind($user, $path) { $client = new Client(); $request = $client->createRequest( 'PROPFIND', - substr($this->baseUrl, 0, -4) . $this->davPath . $path, + substr($this->baseUrl, 0, -4) . $this->getDavFilesPath($user) . $path, [ 'body' => ' @@ -109,24 +96,6 @@ public function theWebdavChecksumShouldMatch($checksum) } } - /** - * @When user :user downloads the file :path - * @param string $user - * @param string $path - */ - public function userDownloadsTheFile($user, $path) { - $client = new Client(); - $this->response = $client->get( - substr($this->baseUrl, 0, -4) . $this->davPath . $path, - [ - 'auth' => [ - $user, - $this->getPasswordForUser($user), - ] - ] - ); - } - /** * @Then The header checksum should match :checksum * @param string $checksum @@ -201,24 +170,16 @@ public function theOcChecksumHeaderShouldNotBeThere() * @param string $destination * @param string $checksum */ - public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) - { - $client = new Client(); + public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) { + //$client = new Client(); $num -= 1; - $this->response = $client->put( - substr($this->baseUrl, 0, -4) . $this->davPath . $destination . '-chunking-42-'.$total.'-'.$num, - [ - 'auth' => [ - $user, - $this->getPasswordForUser($user) - ], - 'body' => $data, - 'headers' => [ - 'OC-Checksum' => $checksum, - 'OC-Chunked' => '1', - ] - ] - ); - + $data = \GuzzleHttp\Stream\Stream::factory($data); + $file = $destination . '-chunking-42-' . $total . '-' . $num; + $this->response = $this->makeDavRequest($user, + 'PUT', + $file, + ['OC-Checksum' => $checksum, 'OC-Chunked' => '1'], + $data, + "files"); } } diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 5805e497f4db2..4a908de2155c0 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -230,6 +230,19 @@ public function downloadingFile($fileName) { } } + /** + * @When user :user downloads the file :fileName + * @param string $user + * @param string $fileName + */ + public function userDownloadsTheFile($user, $fileName) { + try { + $this->response = $this->makeDavRequest($user, 'GET', $fileName, []); + } catch (\GuzzleHttp\Exception\ClientException $e) { + $this->response = $e->getResponse(); + } + } + /** * @Then The following headers should be set * @param \Behat\Gherkin\Node\TableNode $table diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index b721521ba4baa..050984165aaf7 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -24,7 +24,7 @@ Feature: checksums Given using old dav path And 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" + When User "user0" moves file "/myChecksumFile.txt" to "/myMovedChecksumFile.txt" And user "user0" request the checksum of "/myMovedChecksumFile.txt" via propfind Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" @@ -32,28 +32,90 @@ Feature: checksums Given using old dav path And 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" + When User "user0" moves file "/myChecksumFile.txt" to "/myMovedChecksumFile.txt" And user "user0" downloads the file "/myMovedChecksumFile.txt" Then The header checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" - Scenario: Copying a file with checksum should return the checksum in the propfind + Scenario: Uploading a chunked file with checksum should return the checksum in the propfind + Given using old dav path + And user "user0" exists + 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 "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" + + Scenario: Uploading a chunked file with checksum should return the checksum in the download header + Given using old dav path + And user "user0" exists + 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 "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" + + Scenario: Downloading a file from local storage has correct checksum Given using old dav path And 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" + + Scenario: Uploading a file with checksum should work using new dav path + Given using new dav path + And user "user0" exists + When user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + Then The webdav response should have a status code "201" + + Scenario: Uploading a file with checksum should return the checksum in the propfind using new dav path + Given using new dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + + Scenario: Uploading a file with checksum should return the checksum in the download header using new dav path + Given using new dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + + Scenario: Moving a file with checksum should return the checksum in the propfind using new dav path + Given using new dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + + Scenario: Moving file with checksum should return the checksum in the download header using new dav path + Given using new dav path + And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + + Scenario: Copying a file with checksum should return the checksum in the propfind using new dav path + Given using new dav path + And 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" + When User "user0" copies file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt" And user "user0" request the checksum of "/myChecksumFileCopy.txt" via propfind Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" - Scenario: Copying file with checksum should return the checksum in the download header - Given using old dav path + Scenario: Copying file with checksum should return the checksum in the download header using new dav path + Given using new dav path And 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" + When User "user0" copies file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt" And user "user0" downloads the file "/myChecksumFileCopy.txt" Then The header checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" - Scenario: Uploading a chunked file with checksum should return the checksum in the propfind - Given using old dav path + Scenario: Uploading a chunked file with checksum should return the checksum in the propfind using new dav path + Given using new dav path And user "user0" exists 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" @@ -61,8 +123,8 @@ Feature: checksums When user "user0" request the checksum of "/myChecksumFile.txt" via propfind Then The webdav checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" - Scenario: Uploading a chunked file with checksum should return the checksum in the download header - Given using old dav path + Scenario: Uploading a chunked file with checksum should return the checksum in the download header using new dav path + Given using new dav path And user "user0" exists 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" @@ -70,8 +132,8 @@ Feature: checksums When user "user0" downloads the file "/myChecksumFile.txt" Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" - Scenario: Downloading a file from local storage has correct checksum - Given using old dav path + Scenario: Downloading a file from local storage has correct checksum using new dav path + Given using new dav path And 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" From 01759e24cc673bd7e9ecacc07392dc9c8299242d Mon Sep 17 00:00:00 2001 From: Sergio Bertolin Date: Fri, 10 Mar 2017 09:42:51 +0000 Subject: [PATCH 35/41] Added Ilja's tests here --- build/integration/features/checksums.feature | 33 ++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index 050984165aaf7..952a422f305e5 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -139,3 +139,36 @@ Feature: checksums 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" + + Scenario: Upload chunked file where checksum does not match + Given using new dav path + And user "user0" exists + And user "user0" creates a new chunking upload with id "chunking-42" + And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42" + And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42" with checksum "SHA1:f005ba11" + And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42" + And user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt" + Then the HTTP status code should be "400" + + Scenario: Upload a file where checksum does not match + Given using old dav path + Given user "user0" exists + And file "/chksumtst.txt" does not exist for user "user0" + And user "user0" uploads file with checksum "SHA1:f005ba11" and content "Some Text" to "/chksumtst.txt" + Then the HTTP status code should be "400" + + Scenario: Upload a file where checksum does match + Given using old dav path + Given user "user0" exists + And file "/chksumtst.txt" does not exist for user "user0" + And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" + Then the HTTP status code should be "201" + + Scenario: Uploaded file should have the same checksum when downloaded + Given using old dav path + Given user "user0" exists + And file "/chksumtst.txt" does not exist for user "user0" + And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/chksumtst.txt" + When Downloading file "/chksumtst.txt" as "user0" + Then The following headers should be set + | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 | From d693c110d1506105f1b9364b650e8cfa81ff627c Mon Sep 17 00:00:00 2001 From: Sergio Bertolin Date: Tue, 14 Mar 2017 09:35:45 +0000 Subject: [PATCH 36/41] Copy will copy and not move --- build/integration/features/bootstrap/Checksums.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/build/integration/features/bootstrap/Checksums.php b/build/integration/features/bootstrap/Checksums.php index 2a0f51d9369c4..10941e94d49d5 100644 --- a/build/integration/features/bootstrap/Checksums.php +++ b/build/integration/features/bootstrap/Checksums.php @@ -117,7 +117,7 @@ public function theHeaderChecksumShouldMatch($checksum) public function userCopiedFileTo($user, $source, $destination) { $client = new Client(); $request = $client->createRequest( - 'MOVE', + 'COPY', substr($this->baseUrl, 0, -4) . $this->davPath . $source, [ 'auth' => [ @@ -171,7 +171,6 @@ public function theOcChecksumHeaderShouldNotBeThere() * @param string $checksum */ public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) { - //$client = new Client(); $num -= 1; $data = \GuzzleHttp\Stream\Stream::factory($data); $file = $destination . '-chunking-42-' . $total . '-' . $num; From 2785749a3a40b9fe021d622f1c00aca22adcfab0 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 15 Mar 2017 13:09:04 +0100 Subject: [PATCH 37/41] Webdav returns all checksums + integration tests --- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 2 +- build/integration/features/checksums.feature | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index b240d62304ba1..1b33c1685086e 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -360,7 +360,7 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node) }); $propFind->handle(self::CHECKSUMS_PROPERTYNAME, function() use ($node) { - $checksum = $node->getChecksum('sha1'); + $checksum = $node->getChecksum(); if ($checksum === NULL || $checksum === '') { return null; } diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index 952a422f305e5..99f956ae96f55 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -11,7 +11,7 @@ Feature: checksums And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" Scenario: Uploading a file with checksum should return the checksum in the download header Given using old dav path @@ -26,7 +26,7 @@ Feature: checksums And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" When User "user0" moves file "/myChecksumFile.txt" to "/myMovedChecksumFile.txt" And user "user0" request the checksum of "/myMovedChecksumFile.txt" via propfind - Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" Scenario: Moving file with checksum should return the checksum in the download header Given using old dav path @@ -43,7 +43,7 @@ Feature: checksums 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 "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" + Then The webdav checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df" Scenario: Uploading a chunked file with checksum should return the checksum in the download header Given using old dav path @@ -73,7 +73,7 @@ Feature: checksums And 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" Scenario: Uploading a file with checksum should return the checksum in the download header using new dav path Given using new dav path @@ -88,7 +88,7 @@ Feature: checksums 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 "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" Scenario: Moving file with checksum should return the checksum in the download header using new dav path Given using new dav path @@ -104,7 +104,7 @@ Feature: checksums And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" When User "user0" copies file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt" And user "user0" request the checksum of "/myChecksumFileCopy.txt" via propfind - Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f" + Then The webdav checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f MD5:d70b40f177b14b470d1756a3c12b963a ADLER32:8ae90960" Scenario: Copying file with checksum should return the checksum in the download header using new dav path Given using new dav path @@ -121,7 +121,7 @@ Feature: checksums 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 "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" + Then The webdav checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df" Scenario: Uploading a chunked file with checksum should return the checksum in the download header using new dav path Given using new dav path From d68d11d59a0c0117adbe0dd139e4a57b898a53be Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 19 Apr 2017 17:53:19 +0200 Subject: [PATCH 38/41] Fix checksum check when uploading to external storage --- build/integration/features/checksums.feature | 13 +++++++++++++ lib/private/Files/Storage/Wrapper/Checksum.php | 7 ++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature index 99f956ae96f55..a2eeee8ece149 100644 --- a/build/integration/features/checksums.feature +++ b/build/integration/features/checksums.feature @@ -54,6 +54,7 @@ Feature: checksums When user "user0" downloads the file "/myChecksumFile.txt" Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" + @local_storage Scenario: Downloading a file from local storage has correct checksum Given using old dav path And user "user0" exists @@ -132,6 +133,7 @@ Feature: checksums When user "user0" downloads the file "/myChecksumFile.txt" Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8" + @local_storage Scenario: Downloading a file from local storage has correct checksum using new dav path Given using new dav path And user "user0" exists @@ -172,3 +174,14 @@ Feature: checksums When Downloading file "/chksumtst.txt" as "user0" Then The following headers should be set | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 | + + @local_storage + Scenario: Uploaded file to external storage should have the same checksum when downloaded + Given using old dav path + Given user "user0" exists + And file "/local_storage/chksumtst.txt" does not exist for user "user0" + And user "user0" uploads file with checksum "SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399" and content "Some Text" to "/local_storage/chksumtst.txt" + When Downloading file "/local_storage/chksumtst.txt" as "user0" + Then The following headers should be set + | OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 | + diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index 151ab6f204159..a4d56e6c32a5b 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -25,6 +25,7 @@ use OC\Files\Stream\Checksum as ChecksumStream; use OC\OCS\Exception; use OCP\ILogger; +use OCP\Files\IHomeStorage; /** * Class Checksum @@ -91,7 +92,11 @@ public function fopen($path, $mode) { * @return int */ private function getChecksumRequirement($path, $mode) { - $isNormalFile = substr($path, 0, 6) === 'files/'; + $isNormalFile = true; + if ($this->instanceOfStorage(IHomeStorage::class)) { + // home storage stores files in "files" + $isNormalFile = substr($path, 0, 6) === 'files/'; + } $fileIsWritten = $mode !== 'r' && $mode !== 'rb'; if ($isNormalFile && $fileIsWritten) { From 8ac3da31289690f95ad763707e7415d7b7e5802b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 26 Apr 2017 22:31:45 +0200 Subject: [PATCH 39/41] update autoloader Signed-off-by: Roeland Jago Douma --- lib/composer/composer/autoload_classmap.php | 2 ++ lib/composer/composer/autoload_static.php | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 9dea4d10fb225..e9d16e8f59f22 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -601,12 +601,14 @@ 'OC\\Files\\Storage\\StorageFactory' => $baseDir . '/lib/private/Files/Storage/StorageFactory.php', 'OC\\Files\\Storage\\Temporary' => $baseDir . '/lib/private/Files/Storage/Temporary.php', 'OC\\Files\\Storage\\Wrapper\\Availability' => $baseDir . '/lib/private/Files/Storage/Wrapper/Availability.php', + 'OC\\Files\\Storage\\Wrapper\\Checksum' => $baseDir . '/lib/private/Files/Storage/Wrapper/Checksum.php', 'OC\\Files\\Storage\\Wrapper\\Encoding' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encoding.php', 'OC\\Files\\Storage\\Wrapper\\Encryption' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encryption.php', 'OC\\Files\\Storage\\Wrapper\\Jail' => $baseDir . '/lib/private/Files/Storage/Wrapper/Jail.php', 'OC\\Files\\Storage\\Wrapper\\PermissionsMask' => $baseDir . '/lib/private/Files/Storage/Wrapper/PermissionsMask.php', 'OC\\Files\\Storage\\Wrapper\\Quota' => $baseDir . '/lib/private/Files/Storage/Wrapper/Quota.php', 'OC\\Files\\Storage\\Wrapper\\Wrapper' => $baseDir . '/lib/private/Files/Storage/Wrapper/Wrapper.php', + 'OC\\Files\\Stream\\Checksum' => $baseDir . '/lib/private/Files/Stream/Checksum.php', 'OC\\Files\\Stream\\Encryption' => $baseDir . '/lib/private/Files/Stream/Encryption.php', 'OC\\Files\\Stream\\Quota' => $baseDir . '/lib/private/Files/Stream/Quota.php', 'OC\\Files\\Type\\Detection' => $baseDir . '/lib/private/Files/Type/Detection.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 11d949de34a1e..d11ea437283db 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -631,12 +631,14 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Files\\Storage\\StorageFactory' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/StorageFactory.php', 'OC\\Files\\Storage\\Temporary' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Temporary.php', 'OC\\Files\\Storage\\Wrapper\\Availability' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Availability.php', + 'OC\\Files\\Storage\\Wrapper\\Checksum' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Checksum.php', 'OC\\Files\\Storage\\Wrapper\\Encoding' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encoding.php', 'OC\\Files\\Storage\\Wrapper\\Encryption' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encryption.php', 'OC\\Files\\Storage\\Wrapper\\Jail' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Jail.php', 'OC\\Files\\Storage\\Wrapper\\PermissionsMask' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/PermissionsMask.php', 'OC\\Files\\Storage\\Wrapper\\Quota' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Quota.php', 'OC\\Files\\Storage\\Wrapper\\Wrapper' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Wrapper.php', + 'OC\\Files\\Stream\\Checksum' => __DIR__ . '/../../..' . '/lib/private/Files/Stream/Checksum.php', 'OC\\Files\\Stream\\Encryption' => __DIR__ . '/../../..' . '/lib/private/Files/Stream/Encryption.php', 'OC\\Files\\Stream\\Quota' => __DIR__ . '/../../..' . '/lib/private/Files/Stream/Quota.php', 'OC\\Files\\Type\\Detection' => __DIR__ . '/../../..' . '/lib/private/Files/Type/Detection.php', From 0f29b4cacec5f6892454e6ae712785a0dfa47dd9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 24 Mar 2017 15:48:39 +0100 Subject: [PATCH 40/41] squashed commit from icewind1991 - see #4023 * cleaner checksum check in test * cleanup checksum wrappers * fix error when trying to remove non existing entry from cache Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/File.php | 6 +- .../features/bootstrap/Checksums.php | 23 ++--- lib/private/Files/Cache/Cache.php | 3 + .../Files/Storage/Wrapper/Checksum.php | 83 +++++-------------- lib/private/Files/Stream/Checksum.php | 68 +++------------ 5 files changed, 52 insertions(+), 131 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/build/integration/features/bootstrap/Checksums.php b/build/integration/features/bootstrap/Checksums.php index 10941e94d49d5..6495b58443a54 100644 --- a/build/integration/features/bootstrap/Checksums.php +++ b/build/integration/features/bootstrap/Checksums.php @@ -4,6 +4,7 @@ use GuzzleHttp\Client; use GuzzleHttp\Message\ResponseInterface; +use Sabre\DAV\Xml\Response\MultiStatus; trait Checksums { @@ -80,19 +81,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'); } } diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 1f3f2433e45cc..e7cb8d494e369 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -439,6 +439,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') { diff --git a/lib/private/Files/Storage/Wrapper/Checksum.php b/lib/private/Files/Storage/Wrapper/Checksum.php index a4d56e6c32a5b..ac186c3b43b88 100644 --- a/lib/private/Files/Storage/Wrapper/Checksum.php +++ b/lib/private/Files/Storage/Wrapper/Checksum.php @@ -18,6 +18,7 @@ * along with this program. If not, see * */ + namespace OC\Files\Storage\Wrapper; use Icewind\Streams\CallbackWrapper; @@ -47,9 +48,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 @@ -64,28 +62,24 @@ 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 @@ -112,7 +106,6 @@ private function getChecksumRequirement($path, $mode) { // Cache entry is sometimes an array (partial) when encryption is enabled without id so // we ignore it. if ($cacheEntry && empty($cacheEntry['checksum']) && is_object($cacheEntry)) { - $this->pathsInCacheWithoutChecksum[$cacheEntry->getId()] = $path; return self::PATH_IN_CACHE_WITHOUT_CHECKSUM; } @@ -120,28 +113,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); } @@ -154,27 +131,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 c7132a44756f2a414b554b952a767519b1c76817 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 27 Apr 2017 00:17:45 -0300 Subject: [PATCH 41/41] Diff to #4023 Signed-off-by: Morris Jobke --- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 1b33c1685086e..b240d62304ba1 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -360,7 +360,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; }