diff options
author | Vincent Petry <pvince81@owncloud.com> | 2013-11-13 14:59:34 +0400 |
---|---|---|
committer | Vincent Petry <pvince81@owncloud.com> | 2013-11-13 14:59:34 +0400 |
commit | 3fa651f2b1754963e3c0c1a30ab6584a72296b02 (patch) | |
tree | 2db912d87658ab5446f5d0a7587d009ed593560b | |
parent | dc30056025f63d32de0846bdf14e971988f83cdb (diff) | |
parent | 77429c28fd6a363959a1c4cf034a956059849d6b (diff) |
Merge pull request #5447 from owncloud/fixing-5117-master
No data corruption duriing parallel upload
-rwxr-xr-x | apps/files_encryption/lib/helper.php | 43 | ||||
-rwxr-xr-x | apps/files_encryption/lib/keymanager.php | 54 | ||||
-rw-r--r-- | apps/files_encryption/lib/proxy.php | 6 | ||||
-rw-r--r-- | apps/files_encryption/lib/util.php | 5 | ||||
-rw-r--r-- | apps/files_encryption/tests/helper.php | 54 | ||||
-rw-r--r-- | apps/files_encryption/tests/keymanager.php | 17 | ||||
-rw-r--r-- | lib/private/connector/sabre/file.php | 28 |
7 files changed, 132 insertions, 75 deletions
diff --git a/apps/files_encryption/lib/helper.php b/apps/files_encryption/lib/helper.php index 91dd08ec08d..314ac577b2f 100755 --- a/apps/files_encryption/lib/helper.php +++ b/apps/files_encryption/lib/helper.php @@ -157,6 +157,49 @@ class Helper { return $return; } + /** + * @brief Check if a path is a .part file + * @param string $path Path that may identify a .part file + * @return bool + */ + public static function isPartialFilePath($path) { + + $extension = pathinfo($path, PATHINFO_EXTENSION); + if ( $extension === 'part' || $extension === 'etmp') { + return true; + } else { + return false; + } + + } + + + /** + * @brief Remove .path extension from a file path + * @param string $path Path that may identify a .part file + * @return string File path without .part extension + * @note this is needed for reusing keys + */ + public static function stripPartialFileExtension($path) { + $extension = pathinfo($path, PATHINFO_EXTENSION); + + if ( $extension === 'part' || $extension === 'etmp') { + + $newLength = strlen($path) - 5; // 5 = strlen(".part") = strlen(".etmp") + $fPath = substr($path, 0, $newLength); + + // if path also contains a transaction id, we remove it too + $extension = pathinfo($fPath, PATHINFO_EXTENSION); + if(substr($extension, 0, 12) === 'ocTransferId') { // 12 = strlen("ocTransferId") + $newLength = strlen($fPath) - strlen($extension) -1; + $fPath = substr($fPath, 0, $newLength); + } + return $fPath; + + } else { + return $path; + } + } /** * @brief disable recovery diff --git a/apps/files_encryption/lib/keymanager.php b/apps/files_encryption/lib/keymanager.php index 7143fcff0f6..6dadd12a62e 100755 --- a/apps/files_encryption/lib/keymanager.php +++ b/apps/files_encryption/lib/keymanager.php @@ -152,10 +152,10 @@ class Keymanager { } // try reusing key file if part file - if (self::isPartialFilePath($targetPath)) { + if (Helper::isPartialFilePath($targetPath)) { $result = $view->file_put_contents( - $basePath . '/' . self::fixPartialFilePath($targetPath) . '.key', $catfile); + $basePath . '/' . Helper::stripPartialFileExtension($targetPath) . '.key', $catfile); } else { @@ -170,48 +170,6 @@ class Keymanager { } /** - * @brief Remove .path extension from a file path - * @param string $path Path that may identify a .part file - * @return string File path without .part extension - * @note this is needed for reusing keys - */ - public static function fixPartialFilePath($path) { - - if (preg_match('/\.part$/', $path) || preg_match('/\.etmp$/', $path)) { - - $newLength = strlen($path) - 5; - $fPath = substr($path, 0, $newLength); - - return $fPath; - - } else { - - return $path; - - } - - } - - /** - * @brief Check if a path is a .part file - * @param string $path Path that may identify a .part file - * @return bool - */ - public static function isPartialFilePath($path) { - - if (preg_match('/\.part$/', $path) || preg_match('/\.etmp$/', $path)) { - - return true; - - } else { - - return false; - - } - - } - - /** * @brief retrieve keyfile for an encrypted file * @param \OC_FilesystemView $view * @param $userId @@ -226,7 +184,7 @@ class Keymanager { $util = new Util($view, \OCP\User::getUser()); list($owner, $filename) = $util->getUidAndFilename($filePath); - $filename = self::fixPartialFilePath($filename); + $filename = Helper::stripPartialFileExtension($filename); $filePath_f = ltrim($filename, '/'); // in case of system wide mount points the keys are stored directly in the data directory @@ -385,8 +343,8 @@ class Keymanager { foreach ($shareKeys as $userId => $shareKey) { // try reusing key file if part file - if (self::isPartialFilePath($shareKeyPath)) { - $writePath = $basePath . '/' . self::fixPartialFilePath($shareKeyPath) . '.' . $userId . '.shareKey'; + if (Helper::isPartialFilePath($shareKeyPath)) { + $writePath = $basePath . '/' . Helper::stripPartialFileExtension($shareKeyPath) . '.' . $userId . '.shareKey'; } else { $writePath = $basePath . '/' . $shareKeyPath . '.' . $userId . '.shareKey'; } @@ -422,7 +380,7 @@ class Keymanager { $util = new Util($view, \OCP\User::getUser()); list($owner, $filename) = $util->getUidAndFilename($filePath); - $filename = self::fixPartialFilePath($filename); + $filename = Helper::stripPartialFileExtension($filename); // in case of system wide mount points the keys are stored directly in the data directory if ($util->isSystemWideMountPoint($filename)) { $shareKeyPath = '/files_encryption/share-keys/' . $filename . '.' . $userId . '.shareKey'; diff --git a/apps/files_encryption/lib/proxy.php b/apps/files_encryption/lib/proxy.php index 6dc5c9ce1b8..e2bc8f6b163 100644 --- a/apps/files_encryption/lib/proxy.php +++ b/apps/files_encryption/lib/proxy.php @@ -342,7 +342,7 @@ class Proxy extends \OC_FileProxy { $fileInfo = false; // get file info from database/cache if not .part file - if (!Keymanager::isPartialFilePath($path)) { + if (!Helper::isPartialFilePath($path)) { $fileInfo = $view->getFileInfo($path); } @@ -353,7 +353,7 @@ class Proxy extends \OC_FileProxy { $fixSize = $util->getFileSize($path); $fileInfo['unencrypted_size'] = $fixSize; // put file info if not .part file - if (!Keymanager::isPartialFilePath($relativePath)) { + if (!Helper::isPartialFilePath($relativePath)) { $view->putFileInfo($path, $fileInfo); } } @@ -372,7 +372,7 @@ class Proxy extends \OC_FileProxy { $fileInfo['unencrypted_size'] = $size; // put file info if not .part file - if (!Keymanager::isPartialFilePath($relativePath)) { + if (!Helper::isPartialFilePath($relativePath)) { $view->putFileInfo($path, $fileInfo); } } diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index 5e855abd973..f099a36d0c0 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -1145,10 +1145,7 @@ class Util { // Make sure that a share key is generated for the owner too list($owner, $ownerPath) = $this->getUidAndFilename($filePath); - $pathinfo = pathinfo($ownerPath); - if(array_key_exists('extension', $pathinfo) && $pathinfo['extension'] === 'part') { - $ownerPath = $pathinfo['dirname'] . '/' . $pathinfo['filename']; - } + $ownerPath = \OCA\Encryption\Helper::stripPartialFileExtension($ownerPath); $userIds = array(); if ($sharingEnabled) { diff --git a/apps/files_encryption/tests/helper.php b/apps/files_encryption/tests/helper.php new file mode 100644 index 00000000000..067fc763a95 --- /dev/null +++ b/apps/files_encryption/tests/helper.php @@ -0,0 +1,54 @@ +<?php +/** + * Copyright (c) 2013 Bjoern Schiessle <schiessle@owncloud.com> + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + + +require_once __DIR__ . '/../lib/helper.php'; + +use OCA\Encryption; + +/** + * Class Test_Encryption_Helper + */ +class Test_Encryption_Helper extends \PHPUnit_Framework_TestCase { + + /** + * @medium + */ + function testStripPartialFileExtension() { + + $partFilename = 'testfile.txt.part'; + $filename = 'testfile.txt'; + + $this->assertTrue(Encryption\Helper::isPartialFilePath($partFilename)); + + $this->assertEquals('testfile.txt', Encryption\Helper::stripPartialFileExtension($partFilename)); + + $this->assertFalse(Encryption\Helper::isPartialFilePath($filename)); + + $this->assertEquals('testfile.txt', Encryption\Helper::stripPartialFileExtension($filename)); + } + + + /** + * @medium + */ + function testStripPartialFileExtensionWithTransferIdPath() { + + $partFilename = 'testfile.txt.ocTransferId643653835.part'; + $filename = 'testfile.txt'; + + $this->assertTrue(Encryption\Helper::isPartialFilePath($partFilename)); + + $this->assertEquals('testfile.txt', Encryption\Helper::stripPartialFileExtension($partFilename)); + + $this->assertFalse(Encryption\Helper::isPartialFilePath($filename)); + + $this->assertEquals('testfile.txt', Encryption\Helper::stripPartialFileExtension($filename)); + } + +}
\ No newline at end of file diff --git a/apps/files_encryption/tests/keymanager.php b/apps/files_encryption/tests/keymanager.php index b2d200cca3e..ad6bbd3a7e9 100644 --- a/apps/files_encryption/tests/keymanager.php +++ b/apps/files_encryption/tests/keymanager.php @@ -191,23 +191,6 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase { /** * @medium */ - function testFixPartialFilePath() { - - $partFilename = 'testfile.txt.part'; - $filename = 'testfile.txt'; - - $this->assertTrue(Encryption\Keymanager::isPartialFilePath($partFilename)); - - $this->assertEquals('testfile.txt', Encryption\Keymanager::fixPartialFilePath($partFilename)); - - $this->assertFalse(Encryption\Keymanager::isPartialFilePath($filename)); - - $this->assertEquals('testfile.txt', Encryption\Keymanager::fixPartialFilePath($filename)); - } - - /** - * @medium - */ function testRecursiveDelShareKeys() { // generate filename diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 919bb1fc6f4..26b5d200bde 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -230,9 +230,31 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D } if ($chunk_handler->isComplete()) { - $newPath = $path . '/' . $info['name']; - $chunk_handler->file_assemble($newPath); - return OC_Connector_Sabre_Node::getETagPropertyForPath($newPath); + + // we first assembly the target file as a part file + $partFile = $path . '/' . $info['name'] . '.ocTransferId' . $info['transferid'] . '.part'; + $chunk_handler->file_assemble($partFile); + + // here is the final atomic rename + $fs = $this->getFS(); + $targetPath = $path . '/' . $info['name']; + $renameOkay = $fs->rename($partFile, $targetPath); + $fileExists = $fs->file_exists($targetPath); + if ($renameOkay === false || $fileExists === false) { + \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR); + $fs->unlink($targetPath); + throw new Sabre_DAV_Exception(); + } + + // allow sync clients to send the mtime along in a header + $mtime = OC_Request::hasModificationTime(); + if ($mtime !== false) { + if($fs->touch($this->path, $mtime)) { + header('X-OC-MTime: accepted'); + } + } + + return OC_Connector_Sabre_Node::getETagPropertyForPath($targetPath); } return null; |