From 03d0fa012f967dd058d3926751dc147537760ce3 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 28 Feb 2016 11:53:56 +0100 Subject: Checksum intergration test * Upload file with checksum * Chunked upload with checksum * Copy file with checksum should also copy the checksum * Moving a file with checksum should also move the checksum * Uploading a file with checksum and overwriting it with a file without cheksum should remove the checksum --- build/integration/config/behat.yml | 2 + .../features/bootstrap/ChecksumsContext.php | 227 +++++++++++++++++++++ build/integration/features/checksums.feature | 76 +++++++ 3 files changed, 305 insertions(+) create mode 100644 build/integration/features/bootstrap/ChecksumsContext.php create mode 100644 build/integration/features/checksums.feature diff --git a/build/integration/config/behat.yml b/build/integration/config/behat.yml index d0c4586d285..4b5b5b16ef8 100644 --- a/build/integration/config/behat.yml +++ b/build/integration/config/behat.yml @@ -20,6 +20,8 @@ default: baseUrl: http://localhost:8080 - CalDavContext: baseUrl: http://localhost:8080 + - ChecksumsContext: + baseUrl: http://localhost:8080 federation: paths: - %paths.base%/../federation_features diff --git a/build/integration/features/bootstrap/ChecksumsContext.php b/build/integration/features/bootstrap/ChecksumsContext.php new file mode 100644 index 00000000000..a5d20ba965d --- /dev/null +++ b/build/integration/features/bootstrap/ChecksumsContext.php @@ -0,0 +1,227 @@ +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 tearUpScenario() { + $this->client = new Client(); + } + + /** @AfterScenario */ + public function tearDownScenario() { + } + + + /** + * @param string $userName + * @return string + */ + private function getPasswordForUser($userName) { + if($userName === 'admin') { + return 'admin'; + } + return '123456'; + } + + /** + * @When user :user uploads file :source to :destination with checksum :checksum + */ + public function userUploadsFileToWithChecksum($user, $source, $destination, $checksum) + { + $file = \GuzzleHttp\Stream\Stream::factory(fopen($source, 'r')); + try { + $this->response = $this->client->put( + $this->baseUrl . '/remote.php/webdav' . $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(); + } + } + + /** + * @Then The webdav response should have a status code :statusCode + */ + public function theWebdavResponseShouldHaveAStatusCode($statusCode) { + if((int)$statusCode !== $this->response->getStatusCode()) { + throw new \Exception("Expected $statusCode, got ".$this->response->getStatusCode()); + } + } + + /** + * @When user :user request the checksum of :path via propfind + */ + public function userRequestTheChecksumOfViaPropfind($user, $path) + { + $request = $this->client->createRequest( + 'PROPFIND', + $this->baseUrl . '/remote.php/webdav' . $path, + [ + 'body' => ' + + + + +', + 'auth' => [ + $user, + $this->getPasswordForUser($user), + ] + ] + ); + $this->response = $this->client->send($request); + } + + /** + * @Then The webdav checksum should match :checksum + */ + public function theWebdavChecksumShouldMatch($checksum) + { + $service = new Sabre\Xml\Service(); + $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']); + } + } + + /** + * @When user :user downloads the file :path + */ + public function userDownloadsTheFile($user, $path) + { + $this->response = $this->client->get( + $this->baseUrl . '/remote.php/webdav' . $path, + [ + 'auth' => [ + $user, + $this->getPasswordForUser($user), + ] + ] + ); + } + + /** + * @Then The header checksum should match :checksum + */ + public function theHeaderChecksumShouldMatch($checksum) + { + if ($this->response->getHeader('OC-Checksum') !== $checksum) { + throw new \Exception("Expected $checksum, got ".$this->response->getHeader('OC-Checksum')); + } + } + + /** + * @Given User :user copied file :source to :destination + */ + public function userCopiedFileTo($user, $source, $destination) + { + $request = $this->client->createRequest( + 'MOVE', + $this->baseUrl . '/remote.php/webdav' . $source, + [ + 'auth' => [ + $user, + $this->getPasswordForUser($user), + ], + 'headers' => [ + 'Destination' => $this->baseUrl . '/remote.php/webdav' . $destination, + ], + ] + ); + $this->response = $this->client->send($request); + } + + /** + * @Then The webdav checksum should be empty + */ + public function theWebdavChecksumShouldBeEmpty() + { + $service = new Sabre\Xml\Service(); + $parsed = $service->parse($this->response->getBody()->getContents()); + + /* + * Fetch the checksum array + * Maybe we want to do this a bit cleaner ;) + */ + $status = $parsed[0]['value'][1]['value'][1]['value']; + + if ($status !== 'HTTP/1.1 404 Not Found') { + throw new \Exception("Expected 'HTTP/1.1 404 Not Found', got ".$status); + } + } + + /** + * @Then The OC-Checksum header should not be there + */ + public function theOcChecksumHeaderShouldNotBeThere() + { + if ($this->response->hasHeader('OC-Checksum')) { + throw new \Exception("Expected no checksum header but got ".$this->response->getHeader('OC-Checksum')); + } + } + + /** + * @Given user :user uploads chunk file :num of :total with :data to :destination with checksum :checksum + */ + public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) + { + $num -= 1; + $this->response = $this->client->put( + $this->baseUrl . '/remote.php/webdav' . $destination . '-chunking-42-'.$total.'-'.$num, + [ + 'auth' => [ + $user, + $this->getPasswordForUser($user) + ], + 'body' => $data, + 'headers' => [ + 'OC-Checksum' => $checksum, + 'OC-Chunked' => '1', + ] + ] + ); + + } +} diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature new file mode 100644 index 00000000000..d391e93afe8 --- /dev/null +++ b/build/integration/features/checksums.feature @@ -0,0 +1,76 @@ +Feature: checksums + + Scenario: Uploading a file with checksum should work + Given 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 + 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" + + 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" + + 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" + + 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" + + 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" + + 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 + + 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" + When user "user0" request the checksum of "/myChecksumFile.txt" via propfind + Then The webdav checksum should match "MD5:e892fdd61a74bc89cd05673cc2e22f88" + + 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" + When user "user0" downloads the file "/myChecksumFile.txt" + Then The header checksum should match "MD5:e892fdd61a74bc89cd05673cc2e22f88" -- cgit v1.2.3 From 3e88a5067f3ef6fd81068f85a00ee2a49a8f9b79 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 28 Feb 2016 19:41:46 +0100 Subject: Remove checksum on upload of non checksumed file When we overwrite a checksumed file with a file without a checksum we should remove the checksum from the server. This is done by setting the column to empty. --- apps/dav/lib/connector/sabre/file.php | 2 ++ apps/dav/lib/connector/sabre/filesplugin.php | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index 38a1ee5f4e2..b7e2108db3b 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -218,6 +218,8 @@ class File extends Node implements IFile { if (isset($request->server['HTTP_OC_CHECKSUM'])) { $checksum = trim($request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($this->path, ['checksum' => $checksum]); + } else if ($this->getChecksum() !== NULL && $this->getChecksum() !== '') { + $this->fileView->putFileInfo($this->path, ['checksum' => '']); } $this->refreshInfo(); diff --git a/apps/dav/lib/connector/sabre/filesplugin.php b/apps/dav/lib/connector/sabre/filesplugin.php index eb9116d219b..4b05922adfd 100644 --- a/apps/dav/lib/connector/sabre/filesplugin.php +++ b/apps/dav/lib/connector/sabre/filesplugin.php @@ -27,6 +27,7 @@ namespace OCA\DAV\Connector\Sabre; +use Sabre\DAV\Exception\NotFound; use Sabre\DAV\IFile; use \Sabre\DAV\PropFind; use \Sabre\DAV\PropPatch; @@ -197,7 +198,7 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin { //Add OC-Checksum header /** @var $node File */ $checksum = $node->getChecksum(); - if ($checksum !== null) { + if ($checksum !== null && $checksum !== '') { $response->addHeader('OC-Checksum', $checksum); } } @@ -252,6 +253,10 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin { $propFind->handle(self::CHECKSUMS_PROPERTYNAME, function() use ($node) { $checksum = $node->getChecksum(); + if ($checksum === NULL || $checksum === '') { + return null; + } + return new ChecksumList($checksum); }); -- cgit v1.2.3 From ec140fa2ec2aaf489bae728971ec18e58719a38a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 28 Feb 2016 20:21:43 +0100 Subject: Checksums on chunked files We should also store checksums on chunked files. We do not checksum individual chunks but only the final file. --- apps/dav/lib/connector/sabre/file.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index b7e2108db3b..b71c4ccc621 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -457,6 +457,14 @@ class File extends Node implements IFile { $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_SHARED); + if (isset($request->server['HTTP_OC_CHECKSUM'])) { + $checksum = trim($request->server['HTTP_OC_CHECKSUM']); + $this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]); + } else if ($this->getChecksum() !== NULL && $this->getChecksum() !== '') { + $this->fileView->putFileInfo($this->path, ['checksum' => '']); + } + $this->refreshInfo(); + $this->emitPostHooks($exists, $targetPath); $info = $this->fileView->getFileInfo($targetPath); -- cgit v1.2.3 From ac392457f24aed34dc2c5635d9ffd64ec256b061 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 29 Feb 2016 10:29:48 +0100 Subject: Fix unit tests --- apps/dav/lib/connector/sabre/file.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index b71c4ccc621..0bf7d9cf2bc 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -215,6 +215,8 @@ class File extends Node implements IFile { } } + $this->refreshInfo(); + if (isset($request->server['HTTP_OC_CHECKSUM'])) { $checksum = trim($request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($this->path, ['checksum' => $checksum]); @@ -457,17 +459,17 @@ class File extends Node implements IFile { $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_SHARED); + $this->emitPostHooks($exists, $targetPath); + + $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 ($this->getChecksum() !== NULL && $this->getChecksum() !== '') { + } else if ($info->getChecksum() !== NULL && $info->getChecksum() !== '') { $this->fileView->putFileInfo($this->path, ['checksum' => '']); } - $this->refreshInfo(); - - $this->emitPostHooks($exists, $targetPath); - $info = $this->fileView->getFileInfo($targetPath); $this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED); -- cgit v1.2.3 From 57babe032b1c3abcf4e4208bb19488151a934470 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 1 Mar 2016 11:24:10 +0100 Subject: Save some calls to refreshInfo during upload --- apps/dav/lib/connector/sabre/file.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index 0bf7d9cf2bc..9c8344bc5db 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -220,10 +220,11 @@ class File extends Node implements IFile { if (isset($request->server['HTTP_OC_CHECKSUM'])) { $checksum = trim($request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($this->path, ['checksum' => $checksum]); - } else if ($this->getChecksum() !== NULL && $this->getChecksum() !== '') { + $this->refreshInfo(); + } else if ($this->getChecksum() !== null && $this->getChecksum() !== '') { $this->fileView->putFileInfo($this->path, ['checksum' => '']); + $this->refreshInfo(); } - $this->refreshInfo(); } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to check file size: " . $e->getMessage()); @@ -461,16 +462,16 @@ class File extends Node implements IFile { $this->emitPostHooks($exists, $targetPath); + // 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() !== '') { + } else if ($info->getChecksum() !== null && $info->getChecksum() !== '') { $this->fileView->putFileInfo($this->path, ['checksum' => '']); } - $this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED); return $info->getEtag(); -- cgit v1.2.3