From 3a21755963d8d9897a48ab58292345c0b710e239 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 25 Feb 2014 16:23:09 +0100 Subject: [PATCH 01/14] Pass the filesystem view as argument in the sabredav connectors and use the fileinfo object --- apps/files/appinfo/remote.php | 22 ++- .../sabre/aborteduploaddetectionplugin.php | 33 ++-- lib/private/connector/sabre/directory.php | 35 ++-- lib/private/connector/sabre/file.php | 64 +++---- lib/private/connector/sabre/node.php | 168 ++++++------------ lib/private/connector/sabre/objecttree.php | 78 ++++---- lib/private/connector/sabre/quotaplugin.php | 30 ++-- lib/private/connector/sabre/server.php | 80 ++++++--- 8 files changed, 232 insertions(+), 278 deletions(-) diff --git a/apps/files/appinfo/remote.php b/apps/files/appinfo/remote.php index ef22fe9218..1922bc4fcd 100644 --- a/apps/files/appinfo/remote.php +++ b/apps/files/appinfo/remote.php @@ -34,12 +34,8 @@ $authBackend = new OC_Connector_Sabre_Auth(); $lockBackend = new OC_Connector_Sabre_Locks(); $requestBackend = new OC_Connector_Sabre_Request(); -// Create ownCloud Dir -$rootDir = new OC_Connector_Sabre_Directory(''); -$objectTree = new \OC\Connector\Sabre\ObjectTree($rootDir); - // Fire up server -$server = new OC_Connector_Sabre_Server($objectTree); +$server = new OC_Connector_Sabre_Server(); $server->httpRequest = $requestBackend; $server->setBaseUri($baseuri); @@ -49,10 +45,22 @@ $server->addPlugin(new Sabre_DAV_Auth_Plugin($authBackend, $defaults->getName()) $server->addPlugin(new Sabre_DAV_Locks_Plugin($lockBackend)); $server->addPlugin(new Sabre_DAV_Browser_Plugin(false)); // Show something in the Browser, but no upload $server->addPlugin(new OC_Connector_Sabre_FilesPlugin()); -$server->addPlugin(new OC_Connector_Sabre_AbortedUploadDetectionPlugin()); -$server->addPlugin(new OC_Connector_Sabre_QuotaPlugin()); $server->addPlugin(new OC_Connector_Sabre_MaintenancePlugin()); $server->addPlugin(new OC_Connector_Sabre_ExceptionLoggerPlugin('webdav')); +// wait with registering these until auth is handled and the filesystem is setup +$server->subscribeEvent('beforeMethod', function () use ($server) { + $view = \OC\Files\Filesystem::getView(); + $rootInfo = $view->getFileInfo(''); + + // Create ownCloud Dir + $rootDir = new OC_Connector_Sabre_Directory($view, $rootInfo); + $objectTree = new \OC\Connector\Sabre\ObjectTree($rootDir, $view); + $server->setObjectTree($objectTree); + + $server->addPlugin(new OC_Connector_Sabre_AbortedUploadDetectionPlugin($view)); + $server->addPlugin(new OC_Connector_Sabre_QuotaPlugin($view)); +}, 30); // priority 30: after auth (10) and acl(20), before lock(50) and handling the request + // And off we go! $server->exec(); diff --git a/lib/private/connector/sabre/aborteduploaddetectionplugin.php b/lib/private/connector/sabre/aborteduploaddetectionplugin.php index ad759d1d84..1a092a59a8 100644 --- a/lib/private/connector/sabre/aborteduploaddetectionplugin.php +++ b/lib/private/connector/sabre/aborteduploaddetectionplugin.php @@ -22,11 +22,16 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl private $server; /** - * is kept public to allow overwrite for unit testing - * * @var \OC\Files\View */ - public $fileView; + private $fileView; + + /** + * @param \OC\Files\View $view + */ + public function __construct($view) { + $this->fileView = $view; + } /** * This initializes the plugin. @@ -55,7 +60,7 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl // we should only react on PUT which is used for upload // e.g. with LOCK this will not work, but LOCK uses createFile() as well - if ($this->server->httpRequest->getMethod() !== 'PUT' ) { + if ($this->server->httpRequest->getMethod() !== 'PUT') { return; } @@ -70,9 +75,9 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl if (!$expected) { return; } - $actual = $this->getFileView()->filesize($filePath); + $actual = $this->fileView->filesize($filePath); if ($actual != $expected) { - $this->getFileView()->unlink($filePath); + $this->fileView->unlink($filePath); throw new Sabre_DAV_Exception_BadRequest('expected filesize ' . $expected . ' got ' . $actual); } @@ -81,8 +86,7 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl /** * @return string */ - public function getLength() - { + public function getLength() { $req = $this->server->httpRequest; $length = $req->getHeader('X-Expected-Entity-Length'); if (!$length) { @@ -91,17 +95,4 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl return $length; } - - /** - * @return \OC\Files\View - */ - public function getFileView() - { - if (is_null($this->fileView)) { - // initialize fileView - $this->fileView = \OC\Files\Filesystem::getView(); - } - - return $this->fileView; - } } diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index 02d1a9f4ba..619aec21ea 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -60,20 +60,22 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa // exit if we can't create a new file and we don't updatable existing file $info = OC_FileChunking::decodeName($name); - if (!\OC\Files\Filesystem::isCreatable($this->path) && - !\OC\Files\Filesystem::isUpdatable($this->path . '/' . $info['name'])) { + if (!$this->fileView->isCreatable($this->path) && + !$this->fileView->isUpdatable($this->path . '/' . $info['name'])) { throw new \Sabre_DAV_Exception_Forbidden(); } } else { // For non-chunked upload it is enough to check if we can create a new file - if (!\OC\Files\Filesystem::isCreatable($this->path)) { + if (!$this->fileView->isCreatable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); } } $path = $this->path . '/' . $name; - $node = new OC_Connector_Sabre_File($path); + // using a dummy FileInfo is acceptable here since it will be refreshed after the put is complete + $info = new \OC\Files\FileInfo($path, null, null, array()); + $node = new OC_Connector_Sabre_File($this->fileView, $info); return $node->put($data); } @@ -90,12 +92,12 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa throw new \Sabre_DAV_Exception_Forbidden(); } - if (!\OC\Files\Filesystem::isCreatable($this->path)) { + if (!$this->fileView->isCreatable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); } $newPath = $this->path . '/' . $name; - if(!\OC\Files\Filesystem::mkdir($newPath)) { + if(!$this->fileView->mkdir($newPath)) { throw new Sabre_DAV_Exception_Forbidden('Could not create directory '.$newPath); } @@ -105,6 +107,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa * Returns a specific child node, referenced by its name * * @param string $name + * @param \OCP\Files\FileInfo $info * @throws Sabre_DAV_Exception_FileNotFound * @return Sabre_DAV_INode */ @@ -112,7 +115,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa $path = $this->path . '/' . $name; if (is_null($info)) { - $info = \OC\Files\Filesystem::getFileInfo($path); + $info = $this->fileView->getFileInfo($path); } if (!$info) { @@ -120,12 +123,10 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa } if ($info['mimetype'] == 'httpd/unix-directory') { - $node = new OC_Connector_Sabre_Directory($path); + $node = new OC_Connector_Sabre_Directory($this->fileView, $info); } else { - $node = new OC_Connector_Sabre_File($path); + $node = new OC_Connector_Sabre_File($this->fileView, $info); } - - $node->setFileinfoCache($info); return $node; } @@ -136,7 +137,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa */ public function getChildren() { - $folder_content = \OC\Files\Filesystem::getDirectoryContent($this->path); + $folder_content = $this->fileView->getDirectoryContent($this->path); $paths = array(); foreach($folder_content as $info) { $paths[] = $this->path.'/'.$info['name']; @@ -167,7 +168,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa $nodes = array(); foreach($folder_content as $info) { - $node = $this->getChild($info['name'], $info); + $node = $this->getChild($info->getName(), $info); $node->setPropertyCache($properties[$this->path.'/'.$info['name']]); $nodes[] = $node; } @@ -183,7 +184,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa public function childExists($name) { $path = $this->path . '/' . $name; - return \OC\Files\Filesystem::file_exists($path); + return $this->fileView->file_exists($path); } @@ -199,11 +200,11 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa throw new \Sabre_DAV_Exception_Forbidden(); } - if (!\OC\Files\Filesystem::isDeletable($this->path)) { + if (!$this->info->isDeletable()) { throw new \Sabre_DAV_Exception_Forbidden(); } - \OC\Files\Filesystem::rmdir($this->path); + $this->fileView->rmdir($this->path); } @@ -235,7 +236,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa public function getProperties($properties) { $props = parent::getProperties($properties); if (in_array(self::GETETAG_PROPERTYNAME, $properties) && !isset($props[self::GETETAG_PROPERTYNAME])) { - $props[self::GETETAG_PROPERTYNAME] = $this->getETagPropertyForPath($this->path); + $props[self::GETETAG_PROPERTYNAME] = $this->info->getEtag(); } return $props; } diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index ef6caaf22a..cbecd90b7d 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -45,11 +45,8 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D * @return string|null */ public function put($data) { - - $fs = $this->getFS(); - - if ($fs->file_exists($this->path) && - !$fs->isUpdatable($this->path)) { + if ($this->info && $this->fileView->file_exists($this->path) && + !$this->info->isUpdateable()) { throw new \Sabre_DAV_Exception_Forbidden(); } @@ -79,10 +76,10 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D } try { - $putOkay = $fs->file_put_contents($partpath, $data); + $putOkay = $this->fileView->file_put_contents($partpath, $data); if ($putOkay === false) { \OC_Log::write('webdav', '\OC\Files\Filesystem::file_put_contents() failed', \OC_Log::ERROR); - $fs->unlink($partpath); + $this->fileView->unlink($partpath); // because we have no clue about the cause we can only throw back a 500/Internal Server Error throw new Sabre_DAV_Exception('Could not write file contents'); } @@ -105,29 +102,30 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D } // rename to correct path - $renameOkay = $fs->rename($partpath, $this->path); - $fileExists = $fs->file_exists($this->path); + $renameOkay = $this->fileView->rename($partpath, $this->path); + $fileExists = $this->fileView->file_exists($this->path); if ($renameOkay === false || $fileExists === false) { \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR); - $fs->unlink($partpath); + $this->fileView->unlink($partpath); throw new Sabre_DAV_Exception('Could not rename part file to final file'); } // allow sync clients to send the mtime along in a header $mtime = OC_Request::hasModificationTime(); if ($mtime !== false) { - if($fs->touch($this->path, $mtime)) { + if($this->fileView->touch($this->path, $mtime)) { header('X-OC-MTime: accepted'); } } + $this->refreshInfo(); - return $this->getETagPropertyForPath($this->path); + return '"' . $this->info->getEtag() . '"'; } /** * Returns the data * - * @return string + * @return string | resource */ public function get() { @@ -135,7 +133,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D if (\OC_Util::encryptedFiles()) { throw new \Sabre_DAV_Exception_ServiceUnavailable(); } else { - return \OC\Files\Filesystem::fopen($this->path, 'rb'); + return $this->fileView->fopen($this->path, 'rb'); } } @@ -147,16 +145,14 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D * @throws Sabre_DAV_Exception_Forbidden */ public function delete() { - $fs = $this->getFS(); - if ($this->path === 'Shared') { throw new \Sabre_DAV_Exception_Forbidden(); } - if (!$fs->isDeletable($this->path)) { + if (!$this->info->isDeletable()) { throw new \Sabre_DAV_Exception_Forbidden(); } - $fs->unlink($this->path); + $this->fileView->unlink($this->path); // remove properties $this->removeProperties(); @@ -169,12 +165,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D * @return int */ public function getSize() { - $this->getFileinfoCache(); - if ($this->fileinfo_cache['size'] > -1) { - return $this->fileinfo_cache['size']; - } else { - return null; - } + return $this->info->getSize(); } /** @@ -189,11 +180,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D * @return mixed */ public function getETag() { - $properties = $this->getProperties(array(self::GETETAG_PROPERTYNAME)); - if (isset($properties[self::GETETAG_PROPERTYNAME])) { - return $properties[self::GETETAG_PROPERTYNAME]; - } - return null; + return $this->info->getEtag(); } /** @@ -204,12 +191,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D * @return mixed */ public function getContentType() { - if (isset($this->fileinfo_cache['mimetype'])) { - return $this->fileinfo_cache['mimetype']; - } - - return \OC\Files\Filesystem::getMimeType($this->path); - + return $this->info->getMimetype(); } /** @@ -245,15 +227,14 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D $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); + $renameOkay = $this->fileView->rename($partFile, $targetPath); + $fileExists = $this->fileView->file_exists($targetPath); if ($renameOkay === false || $fileExists === false) { \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR); // only delete if an error occurred and the target file was already created if ($fileExists) { - $fs->unlink($targetPath); + $this->fileView->unlink($targetPath); } throw new Sabre_DAV_Exception('Could not rename part file assembled from chunks'); } @@ -261,12 +242,13 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D // allow sync clients to send the mtime along in a header $mtime = OC_Request::hasModificationTime(); if ($mtime !== false) { - if($fs->touch($targetPath, $mtime)) { + if($this->fileView->touch($targetPath, $mtime)) { header('X-OC-MTime: accepted'); } } - return OC_Connector_Sabre_Node::getETagPropertyForPath($targetPath); + $info = $this->fileView->getFileInfo($targetPath); + return $info->getEtag(); } return null; diff --git a/lib/private/connector/sabre/node.php b/lib/private/connector/sabre/node.php index 5807c5c7f7..3a5c721dda 100644 --- a/lib/private/connector/sabre/node.php +++ b/lib/private/connector/sabre/node.php @@ -20,7 +20,6 @@ * License along with this library. If not, see . * */ - abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IProperties { const GETETAG_PROPERTYNAME = '{DAV:}getetag'; const LASTMODIFIED_PROPERTYNAME = '{DAV:}lastmodified'; @@ -29,15 +28,13 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr * Allow configuring the method used to generate Etags * * @var array(class_name, function_name) - */ + */ public static $ETagFunction = null; /** - * is kept public to allow overwrite for unit testing - * * @var \OC\Files\View */ - public $fileView; + protected $fileView; /** * The path to the current node @@ -46,53 +43,53 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr */ protected $path; - /** - * node fileinfo cache - * @var array - */ - protected $fileinfo_cache; /** * node properties cache + * * @var array */ protected $property_cache = null; /** - * @brief Sets up the node, expects a full path name - * @param string $path - * @return void + * @var \OCP\Files\FileInfo */ - public function __construct($path) { - $this->path = $path; + protected $info; + + /** + * @brief Sets up the node, expects a full path name + * @param \OC\Files\View $view + * @param \OCP\Files\FileInfo $info + */ + public function __construct($view, $info) { + $this->fileView = $view; + $this->path = $this->fileView->getRelativePath($info->getPath()); + $this->info = $info; } - + protected function refreshInfo() { + $this->info = $this->fileView->getFileInfo($this->path); + } /** * @brief Returns the name of the node * @return string */ public function getName() { - - list(, $name) = Sabre_DAV_URLUtil::splitPath($this->path); - return $name; - + return $this->info->getName(); } /** * @brief Renames the node * @param string $name The new name - * @return void */ public function setName($name) { - $fs = $this->getFS(); // rename is only allowed if the update privilege is granted - if (!$fs->isUpdatable($this->path)) { + if (!$this->info->isUpdateable()) { throw new \Sabre_DAV_Exception_Forbidden(); } - list($parentPath, ) = Sabre_DAV_URLUtil::splitPath($this->path); + list($parentPath,) = Sabre_DAV_URLUtil::splitPath($this->path); list(, $newName) = Sabre_DAV_URLUtil::splitPath($name); if (!\OCP\Util::isValidFileName($newName)) { @@ -102,38 +99,17 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr $newPath = $parentPath . '/' . $newName; $oldPath = $this->path; - $fs->rename($this->path, $newPath); + $this->fileView->rename($this->path, $newPath); $this->path = $newPath; - $query = OC_DB::prepare( 'UPDATE `*PREFIX*properties` SET `propertypath` = ?' - .' WHERE `userid` = ? AND `propertypath` = ?' ); - $query->execute( array( $newPath, OC_User::getUser(), $oldPath )); - + $query = OC_DB::prepare('UPDATE `*PREFIX*properties` SET `propertypath` = ?' + . ' WHERE `userid` = ? AND `propertypath` = ?'); + $query->execute(array($newPath, OC_User::getUser(), $oldPath)); + $this->refreshInfo(); } - public function setFileinfoCache($fileinfo_cache) - { - $this->fileinfo_cache = $fileinfo_cache; - } - - /** - * @brief Ensure that the fileinfo cache is filled - * @note Uses OC_FileCache or a direct stat - */ - protected function getFileinfoCache() { - if (!isset($this->fileinfo_cache)) { - if ($fileinfo_cache = \OC\Files\Filesystem::getFileInfo($this->path)) { - } else { - $fileinfo_cache = \OC\Files\Filesystem::stat($this->path); - } - - $this->fileinfo_cache = $fileinfo_cache; - } - } - - public function setPropertyCache($property_cache) - { + public function setPropertyCache($property_cache) { $this->property_cache = $property_cache; } @@ -142,8 +118,7 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr * @return int */ public function getLastModified() { - $this->getFileinfoCache(); - return $this->fileinfo_cache['mtime']; + return $this->info->getMtime(); } @@ -153,7 +128,8 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr * Even if the modification time is set to a custom value the access time is set to now. */ public function touch($mtime) { - \OC\Files\Filesystem::touch($this->path, $mtime); + $this->fileView->touch($this->path, $mtime); + $this->refreshInfo(); } /** @@ -163,29 +139,28 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr */ public function updateProperties($properties) { $existing = $this->getProperties(array()); - foreach($properties as $propertyName => $propertyValue) { + foreach ($properties as $propertyName => $propertyValue) { // If it was null, we need to delete the property if (is_null($propertyValue)) { - if(array_key_exists( $propertyName, $existing )) { - $query = OC_DB::prepare( 'DELETE FROM `*PREFIX*properties`' - .' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?' ); - $query->execute( array( OC_User::getUser(), $this->path, $propertyName )); + if (array_key_exists($propertyName, $existing)) { + $query = OC_DB::prepare('DELETE FROM `*PREFIX*properties`' + . ' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?'); + $query->execute(array(OC_User::getUser(), $this->path, $propertyName)); } - } - else { - if( strcmp( $propertyName, self::GETETAG_PROPERTYNAME) === 0 ) { - \OC\Files\Filesystem::putFileInfo($this->path, array('etag'=> $propertyValue)); - } elseif( strcmp( $propertyName, self::LASTMODIFIED_PROPERTYNAME) === 0 ) { + } else { + if (strcmp($propertyName, self::GETETAG_PROPERTYNAME) === 0) { + \OC\Files\Filesystem::putFileInfo($this->path, array('etag' => $propertyValue)); + } elseif (strcmp($propertyName, self::LASTMODIFIED_PROPERTYNAME) === 0) { $this->touch($propertyValue); } else { - if(!array_key_exists( $propertyName, $existing )) { - $query = OC_DB::prepare( 'INSERT INTO `*PREFIX*properties`' - .' (`userid`,`propertypath`,`propertyname`,`propertyvalue`) VALUES(?,?,?,?)' ); - $query->execute( array( OC_User::getUser(), $this->path, $propertyName,$propertyValue )); + if (!array_key_exists($propertyName, $existing)) { + $query = OC_DB::prepare('INSERT INTO `*PREFIX*properties`' + . ' (`userid`,`propertypath`,`propertyname`,`propertyvalue`) VALUES(?,?,?,?)'); + $query->execute(array(OC_User::getUser(), $this->path, $propertyName, $propertyValue)); } else { - $query = OC_DB::prepare( 'UPDATE `*PREFIX*properties` SET `propertyvalue` = ?' - .' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?' ); - $query->execute( array( $propertyValue,OC_User::getUser(), $this->path, $propertyName )); + $query = OC_DB::prepare('UPDATE `*PREFIX*properties` SET `propertyvalue` = ?' + . ' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?'); + $query->execute(array($propertyValue, OC_User::getUser(), $this->path, $propertyName)); } } } @@ -199,9 +174,9 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr * removes all properties for this node and user */ public function removeProperties() { - $query = OC_DB::prepare( 'DELETE FROM `*PREFIX*properties`' - .' WHERE `userid` = ? AND `propertypath` = ?' ); - $query->execute( array( OC_User::getUser(), $this->path)); + $query = OC_DB::prepare('DELETE FROM `*PREFIX*properties`' + . ' WHERE `userid` = ? AND `propertypath` = ?'); + $query->execute(array(OC_User::getUser(), $this->path)); $this->setPropertyCache(null); } @@ -219,29 +194,23 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr if (is_null($this->property_cache)) { $sql = 'SELECT * FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?'; - $result = OC_DB::executeAudited( $sql, array( OC_User::getUser(), $this->path ) ); + $result = OC_DB::executeAudited($sql, array(OC_User::getUser(), $this->path)); $this->property_cache = array(); - while( $row = $result->fetchRow()) { + while ($row = $result->fetchRow()) { $this->property_cache[$row['propertyname']] = $row['propertyvalue']; } - // Don't call the static getETagPropertyForPath, its result is not cached - $this->getFileinfoCache(); - if ($this->fileinfo_cache['etag']) { - $this->property_cache[self::GETETAG_PROPERTYNAME] = '"'.$this->fileinfo_cache['etag'].'"'; - } else { - $this->property_cache[self::GETETAG_PROPERTYNAME] = null; - } + $this->property_cache[self::GETETAG_PROPERTYNAME] = '"' . $this->info->getEtag() . '"'; } // if the array was empty, we need to return everything - if(count($properties) == 0) { + if (count($properties) == 0) { return $this->property_cache; } $props = array(); - foreach($properties as $property) { + foreach ($properties as $property) { if (isset($this->property_cache[$property])) { $props[$property] = $this->property_cache[$property]; } @@ -250,36 +219,13 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr return $props; } - /** - * Returns the ETag surrounded by double-quotes for this path. - * @param string $path Path of the file - * @return string|null Returns null if the ETag can not effectively be determined - */ - protected function getETagPropertyForPath($path) { - $data = $this->getFS()->getFileInfo($path); - if (isset($data['etag'])) { - return '"'.$data['etag'].'"'; - } - return null; - } - - protected function getFS() { - if (is_null($this->fileView)) { - $this->fileView = \OC\Files\Filesystem::getView(); - } - return $this->fileView; - } - /** * @return string|null */ - public function getFileId() - { - $this->getFileinfoCache(); - - if (isset($this->fileinfo_cache['fileid'])) { + public function getFileId() { + if ($this->info->getId()) { $instanceId = OC_Util::getInstanceId(); - $id = sprintf('%08d', $this->fileinfo_cache['fileid']); + $id = sprintf('%08d', $this->info->getId()); return $id . $instanceId; } diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index d2fa425b22..d176d3333f 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -8,16 +8,28 @@ namespace OC\Connector\Sabre; +use OC\Files\FileInfo; use OC\Files\Filesystem; class ObjectTree extends \Sabre_DAV_ObjectTree { /** - * keep this public to allow mock injection during unit test - * * @var \OC\Files\View */ - public $fileView; + protected $fileView; + + /** + * Creates the object + * + * This method expects the rootObject to be passed as a parameter + * + * @param \Sabre_DAV_ICollection $rootNode + * @param \OC\Files\View $view + */ + public function __construct(\Sabre_DAV_ICollection $rootNode, $view) { + parent::__construct($rootNode); + $this->fileView = $view; + } /** * Returns the INode object for the requested path @@ -40,31 +52,34 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { if (pathinfo($path, PATHINFO_EXTENSION) === 'part') { // read from storage - $absPath = $this->getFileView()->getAbsolutePath($path); + $absPath = $this->fileView->getAbsolutePath($path); list($storage, $internalPath) = Filesystem::resolvePath('/' . $absPath); if ($storage) { + /** + * @var \OC\Files\Storage\Storage $storage + */ $scanner = $storage->getScanner($internalPath); // get data directly - $info = $scanner->getData($internalPath); + $data = $scanner->getData($internalPath); + $info = new FileInfo($absPath, $storage, $internalPath, $data); + } else { + $info = null; } - } - else { + } else { // read from cache - $info = $this->getFileView()->getFileInfo($path); + $info = $this->fileView->getFileInfo($path); } if (!$info) { throw new \Sabre_DAV_Exception_NotFound('File with name ' . $path . ' could not be located'); } - if ($info['mimetype'] === 'httpd/unix-directory') { - $node = new \OC_Connector_Sabre_Directory($path); + if ($info->getType() === 'dir') { + $node = new \OC_Connector_Sabre_Directory($this->fileView, $info); } else { - $node = new \OC_Connector_Sabre_File($path); + $node = new \OC_Connector_Sabre_File($this->fileView, $info); } - $node->setFileinfoCache($info); - $this->cache[$path] = $node; return $node; @@ -88,19 +103,18 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { list($destinationDir,) = \Sabre_DAV_URLUtil::splitPath($destinationPath); // check update privileges - $fs = $this->getFileView(); - if (!$fs->isUpdatable($sourcePath)) { + if (!$this->fileView->isUpdatable($sourcePath)) { throw new \Sabre_DAV_Exception_Forbidden(); } if ($sourceDir !== $destinationDir) { // for a full move we need update privileges on sourcePath and sourceDir as well as destinationDir - if (!$fs->isUpdatable($sourceDir)) { + if (!$this->fileView->isUpdatable($sourceDir)) { throw new \Sabre_DAV_Exception_Forbidden(); } - if (!$fs->isUpdatable($destinationDir)) { + if (!$this->fileView->isUpdatable($destinationDir)) { throw new \Sabre_DAV_Exception_Forbidden(); } - if (!$fs->isDeletable($sourcePath)) { + if (!$this->fileView->isDeletable($sourcePath)) { throw new \Sabre_DAV_Exception_Forbidden(); } } @@ -110,15 +124,15 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { throw new \Sabre_DAV_Exception_BadRequest(); } - $renameOkay = $fs->rename($sourcePath, $destinationPath); + $renameOkay = $this->fileView->rename($sourcePath, $destinationPath); if (!$renameOkay) { throw new \Sabre_DAV_Exception_Forbidden(''); } // update properties - $query = \OC_DB::prepare( 'UPDATE `*PREFIX*properties` SET `propertypath` = ?' - .' WHERE `userid` = ? AND `propertypath` = ?' ); - $query->execute( array( $destinationPath, \OC_User::getUser(), $sourcePath )); + $query = \OC_DB::prepare('UPDATE `*PREFIX*properties` SET `propertypath` = ?' + . ' WHERE `userid` = ? AND `propertypath` = ?'); + $query->execute(array($destinationPath, \OC_User::getUser(), $sourcePath)); $this->markDirty($sourceDir); $this->markDirty($destinationDir); @@ -137,12 +151,12 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { */ public function copy($source, $destination) { - if (Filesystem::is_file($source)) { - Filesystem::copy($source, $destination); + if ($this->fileView->is_file($source)) { + $this->fileView->copy($source, $destination); } else { - Filesystem::mkdir($destination); - $dh = Filesystem::opendir($source); - if(is_resource($dh)) { + $this->fileView->mkdir($destination); + $dh = $this->fileView->opendir($source); + if (is_resource($dh)) { while (($subnode = readdir($dh)) !== false) { if ($subnode == '.' || $subnode == '..') continue; @@ -155,14 +169,4 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { list($destinationDir,) = \Sabre_DAV_URLUtil::splitPath($destination); $this->markDirty($destinationDir); } - - /** - * @return \OC\Files\View - */ - public function getFileView() { - if (is_null($this->fileView)) { - $this->fileView = \OC\Files\Filesystem::getView(); - } - return $this->fileView; - } } diff --git a/lib/private/connector/sabre/quotaplugin.php b/lib/private/connector/sabre/quotaplugin.php index 8099794f67..13fb187eed 100644 --- a/lib/private/connector/sabre/quotaplugin.php +++ b/lib/private/connector/sabre/quotaplugin.php @@ -9,6 +9,11 @@ */ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { + /** + * @var \OC\Files\View + */ + private $view; + /** * Reference to main server object * @@ -17,11 +22,11 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { private $server; /** - * is kept public to allow overwrite for unit testing - * - * @var \OC\Files\View + * @param \OC\Files\View $view */ - public $fileView; + public function __construct($view) { + $this->view = $view; + } /** * This initializes the plugin. @@ -52,8 +57,8 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { public function checkQuota($uri, $data = null) { $length = $this->getLength(); if ($length) { - if (substr($uri, 0, 1)!=='/') { - $uri='/'.$uri; + if (substr($uri, 0, 1) !== '/') { + $uri = '/' . $uri; } list($parentUri, $newName) = Sabre_DAV_URLUtil::splitPath($uri); $freeSpace = $this->getFreeSpace($parentUri); @@ -64,8 +69,7 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { return true; } - public function getLength() - { + public function getLength() { $req = $this->server->httpRequest; $length = $req->getHeader('X-Expected-Entity-Length'); if (!$length) { @@ -84,14 +88,8 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { * @param $parentUri * @return mixed */ - public function getFreeSpace($parentUri) - { - if (is_null($this->fileView)) { - // initialize fileView - $this->fileView = \OC\Files\Filesystem::getView(); - } - - $freeSpace = $this->fileView->free_space($parentUri); + public function getFreeSpace($parentUri) { + $freeSpace = $this->view->free_space($parentUri); return $freeSpace; } } diff --git a/lib/private/connector/sabre/server.php b/lib/private/connector/sabre/server.php index 2660b043f4..bb7a7171d1 100644 --- a/lib/private/connector/sabre/server.php +++ b/lib/private/connector/sabre/server.php @@ -27,6 +27,22 @@ * @see Sabre_DAV_Server */ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { + /** + * Sets up the server + * + * Unlike Sabre_DAV_Server's constructor this does not take an INode or ObjectTree as argument, + * the object tree needs to be set later with setObjectTree + */ + public function __construct() { + $this->httpResponse = new Sabre_HTTP_Response(); + $this->httpRequest = new Sabre_HTTP_Request(); + + } + + public function setObjectTree($tree) { + $this->tree = $tree; + } + /** * @see Sabre_DAV_Server @@ -40,22 +56,22 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // The only two options for the depth of a propfind is 0 or 1 // if ($depth!=0) $depth = 1; - $newProperties = $this->getPropertiesForPath($uri,$requestedProperties,$depth); + $newProperties = $this->getPropertiesForPath($uri, $requestedProperties, $depth); // This is a multi-status response $this->httpResponse->sendStatus(207); - $this->httpResponse->setHeader('Content-Type','application/xml; charset=utf-8'); - $this->httpResponse->setHeader('Vary','Brief,Prefer'); + $this->httpResponse->setHeader('Content-Type', 'application/xml; charset=utf-8'); + $this->httpResponse->setHeader('Vary', 'Brief,Prefer'); // Normally this header is only needed for OPTIONS responses, however.. // iCal seems to also depend on these being set for PROPFIND. Since // this is not harmful, we'll add it. - $features = array('1','3', 'extended-mkcol'); - foreach($this->plugins as $plugin) { - $features = array_merge($features,$plugin->getFeatures()); + $features = array('1', '3', 'extended-mkcol'); + foreach ($this->plugins as $plugin) { + $features = array_merge($features, $plugin->getFeatures()); } - $this->httpResponse->setHeader('DAV',implode(', ',$features)); + $this->httpResponse->setHeader('DAV', implode(', ', $features)); $prefer = $this->getHTTPPrefer(); $minimal = $prefer['return-minimal']; @@ -67,10 +83,11 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { /** * Small helper to support PROPFIND with DEPTH_INFINITY. + * * @param string $path */ private function addPathNodesRecursively(&$nodes, $path) { - foreach($this->tree->getChildren($path) as $childNode) { + foreach ($this->tree->getChildren($path) as $childNode) { $nodes[$path . '/' . $childNode->getName()] = $childNode; if ($childNode instanceof Sabre_DAV_ICollection) $this->addPathNodesRecursively($nodes, $path . '/' . $childNode->getName()); @@ -81,7 +98,7 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // if ($depth!=0) $depth = 1; - $path = rtrim($path,'/'); + $path = rtrim($path, '/'); $returnPropertyList = array(); @@ -89,9 +106,10 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { $nodes = array( $path => $parentNode ); - if ($depth==1 && $parentNode instanceof Sabre_DAV_ICollection) { - foreach($this->tree->getChildren($path) as $childNode) + if ($depth == 1 && $parentNode instanceof Sabre_DAV_ICollection) { + foreach ($this->tree->getChildren($path) as $childNode) { $nodes[$path . '/' . $childNode->getName()] = $childNode; + } } else if ($depth == self::DEPTH_INFINITY && $parentNode instanceof Sabre_DAV_ICollection) { $this->addPathNodesRecursively($nodes, $path); } @@ -99,9 +117,9 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // If the propertyNames array is empty, it means all properties are requested. // We shouldn't actually return everything we know though, and only return a // sensible list. - $allProperties = count($propertyNames)==0; + $allProperties = count($propertyNames) == 0; - foreach($nodes as $myPath=>$node) { + foreach ($nodes as $myPath => $node) { $currentPropertyNames = $propertyNames; @@ -128,15 +146,15 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // to make certain decisions about the entry. // WebDAV dictates we should add a / and the end of href's for collections $removeRT = false; - if (!in_array('{DAV:}resourcetype',$currentPropertyNames)) { + if (!in_array('{DAV:}resourcetype', $currentPropertyNames)) { $currentPropertyNames[] = '{DAV:}resourcetype'; $removeRT = true; } - $result = $this->broadcastEvent('beforeGetProperties',array($myPath, $node, &$currentPropertyNames, &$newProperties)); + $result = $this->broadcastEvent('beforeGetProperties', array($myPath, $node, &$currentPropertyNames, &$newProperties)); // If this method explicitly returned false, we must ignore this // node as it is inaccessible. - if ($result===false) continue; + if ($result === false) continue; if (count($currentPropertyNames) > 0) { @@ -149,7 +167,7 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // So as we loop through this list, we will only take the // properties that were actually requested and discard the // rest. - foreach($currentPropertyNames as $k=>$currentPropertyName) { + foreach ($currentPropertyNames as $k => $currentPropertyName) { if (isset($nodeProperties[$currentPropertyName])) { unset($currentPropertyNames[$k]); $newProperties[200][$currentPropertyName] = $nodeProperties[$currentPropertyName]; @@ -160,12 +178,14 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { } - foreach($currentPropertyNames as $prop) { + foreach ($currentPropertyNames as $prop) { if (isset($newProperties[200][$prop])) continue; - switch($prop) { - case '{DAV:}getlastmodified' : if ($node->getLastModified()) $newProperties[200][$prop] = new Sabre_DAV_Property_GetLastModified($node->getLastModified()); break; + switch ($prop) { + case '{DAV:}getlastmodified' : + if ($node->getLastModified()) $newProperties[200][$prop] = new Sabre_DAV_Property_GetLastModified($node->getLastModified()); + break; case '{DAV:}getcontentlength' : if ($node instanceof Sabre_DAV_IFile) { $size = $node->getSize(); @@ -186,18 +206,22 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { $newProperties[200][$prop] = $quotaInfo[1]; } break; - case '{DAV:}getetag' : if ($node instanceof Sabre_DAV_IFile && $etag = $node->getETag()) $newProperties[200][$prop] = $etag; break; - case '{DAV:}getcontenttype' : if ($node instanceof Sabre_DAV_IFile && $ct = $node->getContentType()) $newProperties[200][$prop] = $ct; break; + case '{DAV:}getetag' : + if ($node instanceof Sabre_DAV_IFile && $etag = $node->getETag()) $newProperties[200][$prop] = $etag; + break; + case '{DAV:}getcontenttype' : + if ($node instanceof Sabre_DAV_IFile && $ct = $node->getContentType()) $newProperties[200][$prop] = $ct; + break; case '{DAV:}supported-report-set' : $reports = array(); - foreach($this->plugins as $plugin) { + foreach ($this->plugins as $plugin) { $reports = array_merge($reports, $plugin->getSupportedReportSet($myPath)); } $newProperties[200][$prop] = new Sabre_DAV_Property_SupportedReportSet($reports); break; case '{DAV:}resourcetype' : $newProperties[200]['{DAV:}resourcetype'] = new Sabre_DAV_Property_ResourceType(); - foreach($this->resourceTypeMapping as $className => $resourceType) { + foreach ($this->resourceTypeMapping as $className => $resourceType) { if ($node instanceof $className) $newProperties[200]['{DAV:}resourcetype']->add($resourceType); } break; @@ -209,16 +233,16 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { } - $this->broadcastEvent('afterGetProperties',array(trim($myPath,'/'),&$newProperties, $node)); + $this->broadcastEvent('afterGetProperties', array(trim($myPath, '/'), &$newProperties, $node)); - $newProperties['href'] = trim($myPath,'/'); + $newProperties['href'] = trim($myPath, '/'); // Its is a WebDAV recommendation to add a trailing slash to collectionnames. // Apple's iCal also requires a trailing slash for principals (rfc 3744), though this is non-standard. - if ($myPath!='' && isset($newProperties[200]['{DAV:}resourcetype'])) { + if ($myPath != '' && isset($newProperties[200]['{DAV:}resourcetype'])) { $rt = $newProperties[200]['{DAV:}resourcetype']; if ($rt->is('{DAV:}collection') || $rt->is('{DAV:}principal')) { - $newProperties['href'] .='/'; + $newProperties['href'] .= '/'; } } From 9231195c98b5fc709903e8a0b639b882aa3f9ec8 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 3 Mar 2014 13:51:49 +0100 Subject: [PATCH 02/14] Fix FileInfo->getType --- lib/private/files/fileinfo.php | 8 ++++++-- lib/public/files/fileinfo.php | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/private/files/fileinfo.php b/lib/private/files/fileinfo.php index 2dbdd80a26..916346b608 100644 --- a/lib/private/files/fileinfo.php +++ b/lib/private/files/fileinfo.php @@ -144,10 +144,14 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { * @return \OCP\Files\FileInfo::TYPE_FILE | \OCP\Files\FileInfo::TYPE_FOLDER */ public function getType() { - return $this->data['type']; + if ($this->data['type']) { + return $this->data['type']; + } else { + return $this->getMimetype() === 'httpd/unix-directory' ? self::TYPE_FOLDER : self::TYPE_FILE; + } } - public function getData(){ + public function getData() { return $this->data; } diff --git a/lib/public/files/fileinfo.php b/lib/public/files/fileinfo.php index 68ce45d3fa..37162e0933 100644 --- a/lib/public/files/fileinfo.php +++ b/lib/public/files/fileinfo.php @@ -9,7 +9,7 @@ namespace OCP\Files; interface FileInfo { const TYPE_FILE = 'file'; - const TYPE_FOLDER = 'folder'; + const TYPE_FOLDER = 'dir'; /** * Get the Etag of the file or folder From fe994669cdec336f6c4a3e724a9d468fd8f69dfe Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 3 Mar 2014 13:57:08 +0100 Subject: [PATCH 03/14] Make path for dummy fileinfo absolute --- lib/private/connector/sabre/directory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index 619aec21ea..641354e763 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -72,7 +72,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa } } - $path = $this->path . '/' . $name; + $path = $this->fileView->getAbsolutePath($this->path) . '/' . $name; // using a dummy FileInfo is acceptable here since it will be refreshed after the put is complete $info = new \OC\Files\FileInfo($path, null, null, array()); $node = new OC_Connector_Sabre_File($this->fileView, $info); From 331fc55e2d876123580986264f47318afcf9352b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 3 Mar 2014 14:27:24 +0100 Subject: [PATCH 04/14] Update unit tests to use the new injection --- .../sabre/aborteduploaddetectionplugin.php | 27 +++---- tests/lib/connector/sabre/directory.php | 22 ++++- tests/lib/connector/sabre/file.php | 81 +++++++++++++++---- tests/lib/connector/sabre/objecttree.php | 14 +++- tests/lib/connector/sabre/quotaplugin.php | 23 +++--- 5 files changed, 120 insertions(+), 47 deletions(-) diff --git a/tests/lib/connector/sabre/aborteduploaddetectionplugin.php b/tests/lib/connector/sabre/aborteduploaddetectionplugin.php index 201f126386..60d141e72b 100644 --- a/tests/lib/connector/sabre/aborteduploaddetectionplugin.php +++ b/tests/lib/connector/sabre/aborteduploaddetectionplugin.php @@ -1,11 +1,11 @@ * This file is licensed under the Affero General Public License version 3 or * later. * See the COPYING-README file. */ - class Test_OC_Connector_Sabre_AbortedUploadDetectionPlugin extends PHPUnit_Framework_TestCase { /** @@ -18,17 +18,18 @@ class Test_OC_Connector_Sabre_AbortedUploadDetectionPlugin extends PHPUnit_Frame */ private $plugin; - public function setUp() { + private function init($view) { $this->server = new Sabre_DAV_Server(); - $this->plugin = new OC_Connector_Sabre_AbortedUploadDetectionPlugin(); + $this->plugin = new OC_Connector_Sabre_AbortedUploadDetectionPlugin($view); $this->plugin->initialize($this->server); } /** * @dataProvider lengthProvider */ - public function testLength($expected, $headers) - { + public function testLength($expected, $headers) { + $this->init(null); + $this->server->httpRequest = new Sabre_HTTP_Request($headers); $length = $this->plugin->getLength(); $this->assertEquals($expected, $length); @@ -37,9 +38,8 @@ class Test_OC_Connector_Sabre_AbortedUploadDetectionPlugin extends PHPUnit_Frame /** * @dataProvider verifyContentLengthProvider */ - public function testVerifyContentLength($method, $fileSize, $headers) - { - $this->plugin->fileView = $this->buildFileViewMock($fileSize); + public function testVerifyContentLength($method, $fileSize, $headers) { + $this->init($this->buildFileViewMock($fileSize)); $headers['REQUEST_METHOD'] = $method; $this->server->httpRequest = new Sabre_HTTP_Request($headers); @@ -51,12 +51,11 @@ class Test_OC_Connector_Sabre_AbortedUploadDetectionPlugin extends PHPUnit_Frame * @dataProvider verifyContentLengthFailedProvider * @expectedException Sabre_DAV_Exception_BadRequest */ - public function testVerifyContentLengthFailed($method, $fileSize, $headers) - { - $this->plugin->fileView = $this->buildFileViewMock($fileSize); - + public function testVerifyContentLengthFailed($method, $fileSize, $headers) { + $view = $this->buildFileViewMock($fileSize); + $this->init($view); // we expect unlink to be called - $this->plugin->fileView->expects($this->once())->method('unlink'); + $view->expects($this->once())->method('unlink'); $headers['REQUEST_METHOD'] = $method; $this->server->httpRequest = new Sabre_HTTP_Request($headers); @@ -92,7 +91,7 @@ class Test_OC_Connector_Sabre_AbortedUploadDetectionPlugin extends PHPUnit_Frame private function buildFileViewMock($fileSize) { // mock filesystem - $view = $this->getMock('\OC\Files\View', array('filesize', 'unlink'), array(), '', FALSE); + $view = $this->getMock('\OC\Files\View', array('filesize', 'unlink'), array(), '', false); $view->expects($this->any())->method('filesize')->withAnyParameters()->will($this->returnValue($fileSize)); return $view; diff --git a/tests/lib/connector/sabre/directory.php b/tests/lib/connector/sabre/directory.php index c501521b60..b2bf0d4a6d 100644 --- a/tests/lib/connector/sabre/directory.php +++ b/tests/lib/connector/sabre/directory.php @@ -1,18 +1,32 @@ * This file is licensed under the Affero General Public License version 3 or * later. * See the COPYING-README file. */ - class Test_OC_Connector_Sabre_Directory extends PHPUnit_Framework_TestCase { + private function getRootDir() { + $view = $this->getMock('OC\Files\View', array(), array(), '', false); + $view->expects($this->once()) + ->method('getRelativePath') + ->will($this->returnValue('')); + + $info = $this->getMock('OC\Files\FileInfo', array(), array(), '', false); + $info->expects($this->once()) + ->method('getPath') + ->will($this->returnValue('')); + + return new OC_Connector_Sabre_Directory($view, $info); + } + /** * @expectedException Sabre_DAV_Exception_Forbidden */ public function testCreateSharedFileFails() { - $dir = new OC_Connector_Sabre_Directory(''); + $dir = $this->getRootDir(); $dir->createFile('Shared'); } @@ -20,7 +34,7 @@ class Test_OC_Connector_Sabre_Directory extends PHPUnit_Framework_TestCase { * @expectedException Sabre_DAV_Exception_Forbidden */ public function testCreateSharedFolderFails() { - $dir = new OC_Connector_Sabre_Directory(''); + $dir = $this->getRootDir(); $dir->createDirectory('Shared'); } @@ -28,7 +42,7 @@ class Test_OC_Connector_Sabre_Directory extends PHPUnit_Framework_TestCase { * @expectedException Sabre_DAV_Exception_Forbidden */ public function testDeleteSharedFolderFails() { - $dir = new OC_Connector_Sabre_Directory('Shared'); + $dir = $this->getRootDir(); $dir->delete(); } } diff --git a/tests/lib/connector/sabre/file.php b/tests/lib/connector/sabre/file.php index c2f0ffa12d..011f8ffb6e 100644 --- a/tests/lib/connector/sabre/file.php +++ b/tests/lib/connector/sabre/file.php @@ -13,9 +13,20 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase { */ public function testSimplePutFails() { // setup - $file = new OC_Connector_Sabre_File('/test.txt'); - $file->fileView = $this->getMock('\OC\Files\View', array('file_put_contents'), array(), '', FALSE); - $file->fileView->expects($this->any())->method('file_put_contents')->withAnyParameters()->will($this->returnValue(false)); + $view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath'), array(), '', false); + $view->expects($this->any()) + ->method('file_put_contents') + ->will($this->returnValue(false)); + + $view->expects($this->any()) + ->method('getRelativePath') + ->will($this->returnValue('/test.txt')); + + $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + 'permissions'=>\OCP\PERMISSION_ALL + )); + + $file = new OC_Connector_Sabre_File($view, $info); // action $etag = $file->put('test data'); @@ -26,10 +37,25 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase { */ public function testSimplePutFailsOnRename() { // setup - $file = new OC_Connector_Sabre_File('/test.txt'); - $file->fileView = $this->getMock('\OC\Files\View', array('file_put_contents', 'rename'), array(), '', FALSE); - $file->fileView->expects($this->any())->method('file_put_contents')->withAnyParameters()->will($this->returnValue(true)); - $file->fileView->expects($this->any())->method('rename')->withAnyParameters()->will($this->returnValue(false)); + $view = $this->getMock('\OC\Files\View', array('file_put_contents', 'rename', 'getRelativePath'), array(), '', false); + $view->expects($this->any()) + ->method('file_put_contents') + ->withAnyParameters() + ->will($this->returnValue(true)); + $view->expects($this->any()) + ->method('rename') + ->withAnyParameters() + ->will($this->returnValue(false)); + + $view->expects($this->any()) + ->method('getRelativePath') + ->will($this->returnValue('/test.txt')); + + $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + 'permissions' => \OCP\PERMISSION_ALL + )); + + $file = new OC_Connector_Sabre_File($view, $info); // action $etag = $file->put('test data'); @@ -40,9 +66,19 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase { */ public function testSimplePutInvalidChars() { // setup - $file = new OC_Connector_Sabre_File('/super*star.txt'); - $file->fileView = $this->getMock('\OC\Files\View', array('file_put_contents'), array(), '', FALSE); - $file->fileView->expects($this->any())->method('file_put_contents')->withAnyParameters()->will($this->returnValue(false)); + $view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath'), array(), '', false); + $view->expects($this->any()) + ->method('file_put_contents') + ->will($this->returnValue(false)); + + $view->expects($this->any()) + ->method('getRelativePath') + ->will($this->returnValue('/super*star.txt')); + + $info = new \OC\Files\FileInfo('/super*star.txt', null, null, array( + 'permissions' => \OCP\PERMISSION_ALL + )); + $file = new OC_Connector_Sabre_File($view, $info); // action $etag = $file->put('test data'); @@ -54,9 +90,16 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase { */ public function testSetNameInvalidChars() { // setup - $file = new OC_Connector_Sabre_File('/test.txt'); - $file->fileView = $this->getMock('\OC\Files\View', array('isUpdatable'), array(), '', FALSE); - $file->fileView->expects($this->any())->method('isUpdatable')->withAnyParameters()->will($this->returnValue(true)); + $view = $this->getMock('\OC\Files\View', array('getRelativePath'), array(), '', false); + + $view->expects($this->any()) + ->method('getRelativePath') + ->will($this->returnValue('/super*star.txt')); + + $info = new \OC\Files\FileInfo('/super*star.txt', null, null, array( + 'permissions' => \OCP\PERMISSION_ALL + )); + $file = new OC_Connector_Sabre_File($view, $info); $file->setName('/super*star.txt'); } @@ -64,7 +107,17 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase { * @expectedException Sabre_DAV_Exception_Forbidden */ public function testDeleteSharedFails() { - $file = new OC_Connector_Sabre_File('Shared'); + $view = $this->getMock('\OC\Files\View', array('getRelativePath'), array(), '', false); + + $view->expects($this->any()) + ->method('getRelativePath') + ->will($this->returnValue('Shared')); + + $info = new \OC\Files\FileInfo('/Shared', null, null, array( + 'permissions' => \OCP\PERMISSION_ALL + )); + + $file = new OC_Connector_Sabre_File($view, $info); $file->delete(); } } diff --git a/tests/lib/connector/sabre/objecttree.php b/tests/lib/connector/sabre/objecttree.php index fb50c736ed..876334cf68 100644 --- a/tests/lib/connector/sabre/objecttree.php +++ b/tests/lib/connector/sabre/objecttree.php @@ -9,6 +9,7 @@ namespace Test\OC\Connector\Sabre; +use OC\Files\FileInfo; use OC_Connector_Sabre_Directory; use PHPUnit_Framework_TestCase; use Sabre_DAV_Exception_Forbidden; @@ -32,6 +33,10 @@ class TestDoubleFileView extends \OC\Files\View{ public function rename($path1, $path2) { return $this->canRename; } + + public function getRelativePath($path){ + return $path; + } } class ObjectTree extends PHPUnit_Framework_TestCase { @@ -91,10 +96,14 @@ class ObjectTree extends PHPUnit_Framework_TestCase { * @param $updatables */ private function moveTest($source, $dest, $updatables, $deletables) { - $rootDir = new OC_Connector_Sabre_Directory(''); + $view = new TestDoubleFileView($updatables, $deletables); + + $info = new FileInfo('', null, null, array()); + + $rootDir = new OC_Connector_Sabre_Directory($view, $info); $objectTree = $this->getMock('\OC\Connector\Sabre\ObjectTree', array('nodeExists', 'getNodeForPath'), - array($rootDir)); + array($rootDir, $view)); $objectTree->expects($this->once()) ->method('getNodeForPath') @@ -102,7 +111,6 @@ class ObjectTree extends PHPUnit_Framework_TestCase { ->will($this->returnValue(false)); /** @var $objectTree \OC\Connector\Sabre\ObjectTree */ - $objectTree->fileView = new TestDoubleFileView($updatables, $deletables); $objectTree->move($source, $dest); } diff --git a/tests/lib/connector/sabre/quotaplugin.php b/tests/lib/connector/sabre/quotaplugin.php index 1186de2874..6781b970a4 100644 --- a/tests/lib/connector/sabre/quotaplugin.php +++ b/tests/lib/connector/sabre/quotaplugin.php @@ -1,11 +1,11 @@ * This file is licensed under the Affero General Public License version 3 or * later. * See the COPYING-README file. */ - class Test_OC_Connector_Sabre_QuotaPlugin extends PHPUnit_Framework_TestCase { /** @@ -18,17 +18,18 @@ class Test_OC_Connector_Sabre_QuotaPlugin extends PHPUnit_Framework_TestCase { */ private $plugin; - public function setUp() { + private function init($quota) { + $view = $this->buildFileViewMock($quota); $this->server = new Sabre_DAV_Server(); - $this->plugin = new OC_Connector_Sabre_QuotaPlugin(); + $this->plugin = new OC_Connector_Sabre_QuotaPlugin($view); $this->plugin->initialize($this->server); } /** * @dataProvider lengthProvider */ - public function testLength($expected, $headers) - { + public function testLength($expected, $headers) { + $this->init(0); $this->server->httpRequest = new Sabre_HTTP_Request($headers); $length = $this->plugin->getLength(); $this->assertEquals($expected, $length); @@ -37,9 +38,8 @@ class Test_OC_Connector_Sabre_QuotaPlugin extends PHPUnit_Framework_TestCase { /** * @dataProvider quotaOkayProvider */ - public function testCheckQuota($quota, $headers) - { - $this->plugin->fileView = $this->buildFileViewMock($quota); + public function testCheckQuota($quota, $headers) { + $this->init($quota); $this->server->httpRequest = new Sabre_HTTP_Request($headers); $result = $this->plugin->checkQuota(''); @@ -50,9 +50,8 @@ class Test_OC_Connector_Sabre_QuotaPlugin extends PHPUnit_Framework_TestCase { * @expectedException Sabre_DAV_Exception_InsufficientStorage * @dataProvider quotaExceededProvider */ - public function testCheckExceededQuota($quota, $headers) - { - $this->plugin->fileView = $this->buildFileViewMock($quota); + public function testCheckExceededQuota($quota, $headers) { + $this->init($quota); $this->server->httpRequest = new Sabre_HTTP_Request($headers); $this->plugin->checkQuota(''); @@ -92,7 +91,7 @@ class Test_OC_Connector_Sabre_QuotaPlugin extends PHPUnit_Framework_TestCase { private function buildFileViewMock($quota) { // mock filesysten - $view = $this->getMock('\OC\Files\View', array('free_space'), array(), '', FALSE); + $view = $this->getMock('\OC\Files\View', array('free_space'), array(), '', false); $view->expects($this->any())->method('free_space')->withAnyParameters()->will($this->returnValue($quota)); return $view; From 5ef37c28d1dc73c1fc16a731954b84b46c1943f4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 4 Mar 2014 13:28:48 +0100 Subject: [PATCH 05/14] Remove the need for a custom SabreDav server constructor --- apps/files/appinfo/remote.php | 8 ++++---- lib/private/connector/sabre/objecttree.php | 19 ++++++++++++++++--- lib/private/connector/sabre/server.php | 11 ----------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/apps/files/appinfo/remote.php b/apps/files/appinfo/remote.php index 1922bc4fcd..d9c667c010 100644 --- a/apps/files/appinfo/remote.php +++ b/apps/files/appinfo/remote.php @@ -35,7 +35,8 @@ $lockBackend = new OC_Connector_Sabre_Locks(); $requestBackend = new OC_Connector_Sabre_Request(); // Fire up server -$server = new OC_Connector_Sabre_Server(); +$objectTree = new \OC\Connector\Sabre\ObjectTree(); +$server = new OC_Connector_Sabre_Server($objectTree); $server->httpRequest = $requestBackend; $server->setBaseUri($baseuri); @@ -49,14 +50,13 @@ $server->addPlugin(new OC_Connector_Sabre_MaintenancePlugin()); $server->addPlugin(new OC_Connector_Sabre_ExceptionLoggerPlugin('webdav')); // wait with registering these until auth is handled and the filesystem is setup -$server->subscribeEvent('beforeMethod', function () use ($server) { +$server->subscribeEvent('beforeMethod', function () use ($server, $objectTree) { $view = \OC\Files\Filesystem::getView(); $rootInfo = $view->getFileInfo(''); // Create ownCloud Dir $rootDir = new OC_Connector_Sabre_Directory($view, $rootInfo); - $objectTree = new \OC\Connector\Sabre\ObjectTree($rootDir, $view); - $server->setObjectTree($objectTree); + $objectTree->init($rootDir, $view); $server->addPlugin(new OC_Connector_Sabre_AbortedUploadDetectionPlugin($view)); $server->addPlugin(new OC_Connector_Sabre_QuotaPlugin($view)); diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index d176d3333f..76f4817d3b 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -22,12 +22,16 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { * Creates the object * * This method expects the rootObject to be passed as a parameter - * + */ + public function __construct() { + } + + /** * @param \Sabre_DAV_ICollection $rootNode * @param \OC\Files\View $view */ - public function __construct(\Sabre_DAV_ICollection $rootNode, $view) { - parent::__construct($rootNode); + public function init(\Sabre_DAV_ICollection $rootNode, \OC\Files\View $view) { + $this->rootNode = $rootNode; $this->fileView = $view; } @@ -39,6 +43,9 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { * @return \Sabre_DAV_INode */ public function getNodeForPath($path) { + if (!$this->fileView) { + throw new \Sabre_DAV_Exception_ServiceUnavailable('filesystem not setup'); + } $path = trim($path, '/'); if (isset($this->cache[$path])) { @@ -94,6 +101,9 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { * @return int */ public function move($sourcePath, $destinationPath) { + if (!$this->fileView) { + throw new \Sabre_DAV_Exception_ServiceUnavailable('filesystem not setup'); + } $sourceNode = $this->getNodeForPath($sourcePath); if ($sourceNode instanceof \Sabre_DAV_ICollection and $this->nodeExists($destinationPath)) { @@ -150,6 +160,9 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { * @return void */ public function copy($source, $destination) { + if (!$this->fileView) { + throw new \Sabre_DAV_Exception_ServiceUnavailable('filesystem not setup'); + } if ($this->fileView->is_file($source)) { $this->fileView->copy($source, $destination); diff --git a/lib/private/connector/sabre/server.php b/lib/private/connector/sabre/server.php index bb7a7171d1..b09237fb3c 100644 --- a/lib/private/connector/sabre/server.php +++ b/lib/private/connector/sabre/server.php @@ -27,17 +27,6 @@ * @see Sabre_DAV_Server */ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { - /** - * Sets up the server - * - * Unlike Sabre_DAV_Server's constructor this does not take an INode or ObjectTree as argument, - * the object tree needs to be set later with setObjectTree - */ - public function __construct() { - $this->httpResponse = new Sabre_HTTP_Response(); - $this->httpRequest = new Sabre_HTTP_Request(); - - } public function setObjectTree($tree) { $this->tree = $tree; From 52115662057408d095249a237a78c2207ab1b821 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 4 Mar 2014 16:36:03 +0100 Subject: [PATCH 06/14] update test case --- tests/lib/connector/sabre/objecttree.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/connector/sabre/objecttree.php b/tests/lib/connector/sabre/objecttree.php index 876334cf68..bc8ec98fae 100644 --- a/tests/lib/connector/sabre/objecttree.php +++ b/tests/lib/connector/sabre/objecttree.php @@ -111,6 +111,7 @@ class ObjectTree extends PHPUnit_Framework_TestCase { ->will($this->returnValue(false)); /** @var $objectTree \OC\Connector\Sabre\ObjectTree */ + $objectTree->init($rootDir, $view); $objectTree->move($source, $dest); } From a687498547448d6ff11eccc0acddcf0647873ce8 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 5 Mar 2014 13:19:08 +0100 Subject: [PATCH 07/14] Fix encryption webdav tests --- apps/files_encryption/tests/util.php | 6 +++++- apps/files_encryption/tests/webdav.php | 19 ++++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php index f70e30c4d7..5fa96f6ad6 100755 --- a/apps/files_encryption/tests/util.php +++ b/apps/files_encryption/tests/util.php @@ -510,7 +510,11 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { */ public static function loginHelper($user, $create = false, $password = false) { if ($create) { - \OC_User::createUser($user, $user); + try { + \OC_User::createUser($user, $user); + } catch(\Exception $e) { // catch username is already being used from previous aborted runs + + } } if ($password === false) { diff --git a/apps/files_encryption/tests/webdav.php b/apps/files_encryption/tests/webdav.php index 8e8b9c53ce..1f23be3e79 100755 --- a/apps/files_encryption/tests/webdav.php +++ b/apps/files_encryption/tests/webdav.php @@ -33,6 +33,7 @@ use OCA\Encryption; /** * Class Test_Encryption_Webdav + * * @brief this class provide basic webdav tests for PUT,GET and DELETE */ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase { @@ -48,6 +49,8 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase { public $dataShort; public $stateFilesTrashbin; + private static $storage; + public static function setUpBeforeClass() { // reset backend \OC_User::clearBackends(); @@ -65,6 +68,8 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase { // create test user \Test_Encryption_Util::loginHelper(\Test_Encryption_Webdav::TEST_ENCRYPTION_WEBDAV_USER1, true); + + self::$storage = new \OC\Files\Storage\Temporary(array()); } function setUp() { @@ -96,8 +101,7 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase { // reset app files_trashbin if ($this->stateFilesTrashbin) { OC_App::enable('files_trashbin'); - } - else { + } else { OC_App::disable('files_trashbin'); } } @@ -153,7 +157,7 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase { $this->assertTrue(Encryption\Crypt::isCatfileContent($encryptedContent)); // get decrypted file contents - $decrypt = file_get_contents('crypt:///' . $this->userId . '/files'. $filename); + $decrypt = file_get_contents('crypt:///' . $this->userId . '/files' . $filename); // check if file content match with the written content $this->assertEquals($this->dataShort, $decrypt); @@ -225,7 +229,12 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase { $requestBackend = new OC_Connector_Sabre_Request(); // Create ownCloud Dir - $publicDir = new OC_Connector_Sabre_Directory(''); + $root = '/' . $this->userId . '/files'; + \OC\Files\Filesystem::mount(self::$storage, array(), $root); + $view = new \OC\Files\View($root); + $publicDir = new OC_Connector_Sabre_Directory($view, $view->getFileInfo('')); + $objectTree = new \OC\Connector\Sabre\ObjectTree(); + $objectTree->init($publicDir, $view); // Fire up server $server = new Sabre_DAV_Server($publicDir); @@ -236,7 +245,7 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase { $server->addPlugin(new Sabre_DAV_Auth_Plugin($authBackend, 'ownCloud')); $server->addPlugin(new Sabre_DAV_Locks_Plugin($lockBackend)); $server->addPlugin(new Sabre_DAV_Browser_Plugin(false)); // Show something in the Browser, but no upload - $server->addPlugin(new OC_Connector_Sabre_QuotaPlugin()); + $server->addPlugin(new OC_Connector_Sabre_QuotaPlugin($view)); $server->addPlugin(new OC_Connector_Sabre_MaintenancePlugin()); // And off we go! From db6fb198fe7b8fa854050730b32d58d3787505b3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 7 Mar 2014 14:27:23 +0100 Subject: [PATCH 08/14] don't throw errors in getType --- lib/private/files/fileinfo.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/private/files/fileinfo.php b/lib/private/files/fileinfo.php index 916346b608..26ea0aab56 100644 --- a/lib/private/files/fileinfo.php +++ b/lib/private/files/fileinfo.php @@ -53,6 +53,9 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { } public function offsetGet($offset) { + if ($offset === 'type') { + return $this->getType(); + } return $this->data[$offset]; } @@ -144,7 +147,7 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { * @return \OCP\Files\FileInfo::TYPE_FILE | \OCP\Files\FileInfo::TYPE_FOLDER */ public function getType() { - if ($this->data['type']) { + if (isset($this->data['type'])) { return $this->data['type']; } else { return $this->getMimetype() === 'httpd/unix-directory' ? self::TYPE_FOLDER : self::TYPE_FILE; From b42ce6c30a214c7089a84eb9c22c0bd576a15fe7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 22 Apr 2014 15:24:38 +0200 Subject: [PATCH 09/14] Prevent error if previously cached data doesn't have an etag --- apps/files_encryption/tests/webdav.php | 1 + lib/private/files/cache/scanner.php | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/files_encryption/tests/webdav.php b/apps/files_encryption/tests/webdav.php index 1f23be3e79..1fe4c13d59 100755 --- a/apps/files_encryption/tests/webdav.php +++ b/apps/files_encryption/tests/webdav.php @@ -247,6 +247,7 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase { $server->addPlugin(new Sabre_DAV_Browser_Plugin(false)); // Show something in the Browser, but no upload $server->addPlugin(new OC_Connector_Sabre_QuotaPlugin($view)); $server->addPlugin(new OC_Connector_Sabre_MaintenancePlugin()); + $server->debugExceptions = true; // And off we go! if ($body) { diff --git a/lib/private/files/cache/scanner.php b/lib/private/files/cache/scanner.php index 79159724d1..c0bdde06a7 100644 --- a/lib/private/files/cache/scanner.php +++ b/lib/private/files/cache/scanner.php @@ -115,11 +115,12 @@ class Scanner extends BasicEmitter { } if ($reuseExisting) { // prevent empty etag - $etag = $cacheData['etag']; - $propagateETagChange = false; - if (empty($etag)) { + if (empty($cacheData['etag'])) { $etag = $data['etag']; $propagateETagChange = true; + } else { + $etag = $cacheData['etag']; + $propagateETagChange = false; } // only reuse data if the file hasn't explicitly changed if (isset($data['storage_mtime']) && isset($cacheData['storage_mtime']) && $data['storage_mtime'] === $cacheData['storage_mtime']) { From e77d89fc4b8801ff426f3759438cae548d3efcc4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 23 Apr 2014 14:21:09 +0200 Subject: [PATCH 10/14] Revert changes to OC_Connector_Sabre_Server --- lib/private/connector/sabre/server.php | 69 +++++++++++--------------- 1 file changed, 28 insertions(+), 41 deletions(-) diff --git a/lib/private/connector/sabre/server.php b/lib/private/connector/sabre/server.php index b09237fb3c..2660b043f4 100644 --- a/lib/private/connector/sabre/server.php +++ b/lib/private/connector/sabre/server.php @@ -28,11 +28,6 @@ */ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { - public function setObjectTree($tree) { - $this->tree = $tree; - } - - /** * @see Sabre_DAV_Server */ @@ -45,22 +40,22 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // The only two options for the depth of a propfind is 0 or 1 // if ($depth!=0) $depth = 1; - $newProperties = $this->getPropertiesForPath($uri, $requestedProperties, $depth); + $newProperties = $this->getPropertiesForPath($uri,$requestedProperties,$depth); // This is a multi-status response $this->httpResponse->sendStatus(207); - $this->httpResponse->setHeader('Content-Type', 'application/xml; charset=utf-8'); - $this->httpResponse->setHeader('Vary', 'Brief,Prefer'); + $this->httpResponse->setHeader('Content-Type','application/xml; charset=utf-8'); + $this->httpResponse->setHeader('Vary','Brief,Prefer'); // Normally this header is only needed for OPTIONS responses, however.. // iCal seems to also depend on these being set for PROPFIND. Since // this is not harmful, we'll add it. - $features = array('1', '3', 'extended-mkcol'); - foreach ($this->plugins as $plugin) { - $features = array_merge($features, $plugin->getFeatures()); + $features = array('1','3', 'extended-mkcol'); + foreach($this->plugins as $plugin) { + $features = array_merge($features,$plugin->getFeatures()); } - $this->httpResponse->setHeader('DAV', implode(', ', $features)); + $this->httpResponse->setHeader('DAV',implode(', ',$features)); $prefer = $this->getHTTPPrefer(); $minimal = $prefer['return-minimal']; @@ -72,11 +67,10 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { /** * Small helper to support PROPFIND with DEPTH_INFINITY. - * * @param string $path */ private function addPathNodesRecursively(&$nodes, $path) { - foreach ($this->tree->getChildren($path) as $childNode) { + foreach($this->tree->getChildren($path) as $childNode) { $nodes[$path . '/' . $childNode->getName()] = $childNode; if ($childNode instanceof Sabre_DAV_ICollection) $this->addPathNodesRecursively($nodes, $path . '/' . $childNode->getName()); @@ -87,7 +81,7 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // if ($depth!=0) $depth = 1; - $path = rtrim($path, '/'); + $path = rtrim($path,'/'); $returnPropertyList = array(); @@ -95,10 +89,9 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { $nodes = array( $path => $parentNode ); - if ($depth == 1 && $parentNode instanceof Sabre_DAV_ICollection) { - foreach ($this->tree->getChildren($path) as $childNode) { + if ($depth==1 && $parentNode instanceof Sabre_DAV_ICollection) { + foreach($this->tree->getChildren($path) as $childNode) $nodes[$path . '/' . $childNode->getName()] = $childNode; - } } else if ($depth == self::DEPTH_INFINITY && $parentNode instanceof Sabre_DAV_ICollection) { $this->addPathNodesRecursively($nodes, $path); } @@ -106,9 +99,9 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // If the propertyNames array is empty, it means all properties are requested. // We shouldn't actually return everything we know though, and only return a // sensible list. - $allProperties = count($propertyNames) == 0; + $allProperties = count($propertyNames)==0; - foreach ($nodes as $myPath => $node) { + foreach($nodes as $myPath=>$node) { $currentPropertyNames = $propertyNames; @@ -135,15 +128,15 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // to make certain decisions about the entry. // WebDAV dictates we should add a / and the end of href's for collections $removeRT = false; - if (!in_array('{DAV:}resourcetype', $currentPropertyNames)) { + if (!in_array('{DAV:}resourcetype',$currentPropertyNames)) { $currentPropertyNames[] = '{DAV:}resourcetype'; $removeRT = true; } - $result = $this->broadcastEvent('beforeGetProperties', array($myPath, $node, &$currentPropertyNames, &$newProperties)); + $result = $this->broadcastEvent('beforeGetProperties',array($myPath, $node, &$currentPropertyNames, &$newProperties)); // If this method explicitly returned false, we must ignore this // node as it is inaccessible. - if ($result === false) continue; + if ($result===false) continue; if (count($currentPropertyNames) > 0) { @@ -156,7 +149,7 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { // So as we loop through this list, we will only take the // properties that were actually requested and discard the // rest. - foreach ($currentPropertyNames as $k => $currentPropertyName) { + foreach($currentPropertyNames as $k=>$currentPropertyName) { if (isset($nodeProperties[$currentPropertyName])) { unset($currentPropertyNames[$k]); $newProperties[200][$currentPropertyName] = $nodeProperties[$currentPropertyName]; @@ -167,14 +160,12 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { } - foreach ($currentPropertyNames as $prop) { + foreach($currentPropertyNames as $prop) { if (isset($newProperties[200][$prop])) continue; - switch ($prop) { - case '{DAV:}getlastmodified' : - if ($node->getLastModified()) $newProperties[200][$prop] = new Sabre_DAV_Property_GetLastModified($node->getLastModified()); - break; + switch($prop) { + case '{DAV:}getlastmodified' : if ($node->getLastModified()) $newProperties[200][$prop] = new Sabre_DAV_Property_GetLastModified($node->getLastModified()); break; case '{DAV:}getcontentlength' : if ($node instanceof Sabre_DAV_IFile) { $size = $node->getSize(); @@ -195,22 +186,18 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { $newProperties[200][$prop] = $quotaInfo[1]; } break; - case '{DAV:}getetag' : - if ($node instanceof Sabre_DAV_IFile && $etag = $node->getETag()) $newProperties[200][$prop] = $etag; - break; - case '{DAV:}getcontenttype' : - if ($node instanceof Sabre_DAV_IFile && $ct = $node->getContentType()) $newProperties[200][$prop] = $ct; - break; + case '{DAV:}getetag' : if ($node instanceof Sabre_DAV_IFile && $etag = $node->getETag()) $newProperties[200][$prop] = $etag; break; + case '{DAV:}getcontenttype' : if ($node instanceof Sabre_DAV_IFile && $ct = $node->getContentType()) $newProperties[200][$prop] = $ct; break; case '{DAV:}supported-report-set' : $reports = array(); - foreach ($this->plugins as $plugin) { + foreach($this->plugins as $plugin) { $reports = array_merge($reports, $plugin->getSupportedReportSet($myPath)); } $newProperties[200][$prop] = new Sabre_DAV_Property_SupportedReportSet($reports); break; case '{DAV:}resourcetype' : $newProperties[200]['{DAV:}resourcetype'] = new Sabre_DAV_Property_ResourceType(); - foreach ($this->resourceTypeMapping as $className => $resourceType) { + foreach($this->resourceTypeMapping as $className => $resourceType) { if ($node instanceof $className) $newProperties[200]['{DAV:}resourcetype']->add($resourceType); } break; @@ -222,16 +209,16 @@ class OC_Connector_Sabre_Server extends Sabre_DAV_Server { } - $this->broadcastEvent('afterGetProperties', array(trim($myPath, '/'), &$newProperties, $node)); + $this->broadcastEvent('afterGetProperties',array(trim($myPath,'/'),&$newProperties, $node)); - $newProperties['href'] = trim($myPath, '/'); + $newProperties['href'] = trim($myPath,'/'); // Its is a WebDAV recommendation to add a trailing slash to collectionnames. // Apple's iCal also requires a trailing slash for principals (rfc 3744), though this is non-standard. - if ($myPath != '' && isset($newProperties[200]['{DAV:}resourcetype'])) { + if ($myPath!='' && isset($newProperties[200]['{DAV:}resourcetype'])) { $rt = $newProperties[200]['{DAV:}resourcetype']; if ($rt->is('{DAV:}collection') || $rt->is('{DAV:}principal')) { - $newProperties['href'] .= '/'; + $newProperties['href'] .='/'; } } From 4ba6f4839d40cea3360637b236eba9371fa390f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 23 Apr 2014 15:34:04 +0200 Subject: [PATCH 11/14] fixing typos and PHPDoc --- lib/private/connector/sabre/directory.php | 6 +++--- lib/private/connector/sabre/file.php | 6 ++++++ lib/private/connector/sabre/node.php | 3 +++ lib/private/connector/sabre/objecttree.php | 10 +++++++--- lib/private/connector/sabre/quotaplugin.php | 5 +++-- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index 5a2145c8aa..545c1f95ac 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -29,7 +29,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa * Data will either be supplied as a stream resource, or in certain cases * as a string. Keep in mind that you may have to support either. * - * After succesful creation of the file, you may choose to return the ETag + * After successful creation of the file, you may choose to return the ETag * of the new file here. * * The returned ETag must be surrounded by double-quotes (The quotes should @@ -55,7 +55,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa } // for chunked upload also updating a existing file is a "createFile" - // because we create all the chunks before reasamble them to the existing file. + // because we create all the chunks before re-assemble them to the existing file. if (isset($_SERVER['HTTP_OC_CHUNKED'])) { // exit if we can't create a new file and we don't updatable existing file @@ -108,7 +108,7 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa * * @param string $name * @param \OCP\Files\FileInfo $info - * @throws Sabre_DAV_Exception_FileNotFound + * @throws Sabre_DAV_Exception_NotFound * @return Sabre_DAV_INode */ public function getChild($name, $info = null) { diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index a9de41adf4..641c69dd1f 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -42,6 +42,11 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D * * @param resource $data * @throws Sabre_DAV_Exception_Forbidden + * @throws OC_Connector_Sabre_Exception_UnsupportedMediaType + * @throws Sabre_DAV_Exception_BadRequest + * @throws Sabre_DAV_Exception + * @throws OC_Connector_Sabre_Exception_EntityTooLarge + * @throws Sabre_DAV_Exception_ServiceUnavailable * @return string|null */ public function put($data) { @@ -198,6 +203,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D /** * @param resource $data + * @return null|string */ private function createFileChunked($data) { diff --git a/lib/private/connector/sabre/node.php b/lib/private/connector/sabre/node.php index 55e626c4c8..eede39cba8 100644 --- a/lib/private/connector/sabre/node.php +++ b/lib/private/connector/sabre/node.php @@ -81,6 +81,8 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr /** * @brief Renames the node * @param string $name The new name + * @throws Sabre_DAV_Exception_BadRequest + * @throws Sabre_DAV_Exception_Forbidden */ public function setName($name) { @@ -138,6 +140,7 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr /** * @brief Updates properties on this node, * @see Sabre_DAV_IProperties::updateProperties + * @param array $properties * @return boolean */ public function updateProperties($properties) { diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index fa12285878..71a35e87ee 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -39,6 +39,7 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { * Returns the INode object for the requested path * * @param string $path + * @throws \Sabre_DAV_Exception_ServiceUnavailable * @throws \Sabre_DAV_Exception_NotFound * @return \Sabre_DAV_INode */ @@ -97,6 +98,8 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { * * @param string $sourcePath The path to the file which should be moved * @param string $destinationPath The full destination path, so not just the destination parent node + * @throws \Sabre_DAV_Exception_BadRequest + * @throws \Sabre_DAV_Exception_ServiceUnavailable * @throws \Sabre_DAV_Exception_Forbidden * @return int */ @@ -160,6 +163,7 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { * * @param string $source * @param string $destination + * @throws \Sabre_DAV_Exception_ServiceUnavailable * @return void */ public function copy($source, $destination) { @@ -173,10 +177,10 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { $this->fileView->mkdir($destination); $dh = $this->fileView->opendir($source); if (is_resource($dh)) { - while (($subnode = readdir($dh)) !== false) { + while (($subNode = readdir($dh)) !== false) { - if ($subnode == '.' || $subnode == '..') continue; - $this->copy($source . '/' . $subnode, $destination . '/' . $subnode); + if ($subNode == '.' || $subNode == '..') continue; + $this->copy($source . '/' . $subNode, $destination . '/' . $subNode); } } diff --git a/lib/private/connector/sabre/quotaplugin.php b/lib/private/connector/sabre/quotaplugin.php index 78d3172725..1e73e1645c 100644 --- a/lib/private/connector/sabre/quotaplugin.php +++ b/lib/private/connector/sabre/quotaplugin.php @@ -50,8 +50,9 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { /** * This method is called before any HTTP method and validates there is enough free space to store the file * - * @throws Sabre_DAV_Exception * @param string $uri + * @param null $data + * @throws Sabre_DAV_Exception_InsufficientStorage * @return bool */ public function checkQuota($uri, $data = null) { @@ -65,7 +66,7 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { if ($req->getHeader('OC-Chunked')) { $info = OC_FileChunking::decodeName($newName); $chunkHandler = new OC_FileChunking($info); - // substract the already uploaded size to see whether + // subtract the already uploaded size to see whether // there is still enough space for the remaining chunks $length -= $chunkHandler->getCurrentSize(); } From 49b44e7e228855a42771bce4b2bc10b2235557cb Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 23 Apr 2014 16:25:29 +0200 Subject: [PATCH 12/14] Normalize paths when moving properties --- lib/private/connector/sabre/objecttree.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index 71a35e87ee..605684a779 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -148,7 +148,7 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { // update properties $query = \OC_DB::prepare('UPDATE `*PREFIX*properties` SET `propertypath` = ?' . ' WHERE `userid` = ? AND `propertypath` = ?'); - $query->execute(array($destinationPath, \OC_User::getUser(), $sourcePath)); + $query->execute(array(\OC\Files\Filesystem::normalizePath($destinationPath), \OC_User::getUser(), \OC\Files\Filesystem::normalizePath($sourcePath))); $this->markDirty($sourceDir); $this->markDirty($destinationDir); From cd0c5990f895bcdce47acf2dbf11ebadd920a404 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 24 Apr 2014 11:10:46 +0200 Subject: [PATCH 13/14] properly quote etags --- lib/private/connector/sabre/file.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 641c69dd1f..1d5b3fce32 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -185,7 +185,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D * @return mixed */ public function getETag() { - return $this->info->getEtag(); + return '"' . $this->info->getEtag() . '"'; } /** From 4109521ccef38ff2cf16bf0fab7ff4d142d26713 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Apr 2014 12:24:18 +0200 Subject: [PATCH 14/14] remove outdated test --- tests/lib/connector/sabre/file.php | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/lib/connector/sabre/file.php b/tests/lib/connector/sabre/file.php index 011f8ffb6e..a9056460a5 100644 --- a/tests/lib/connector/sabre/file.php +++ b/tests/lib/connector/sabre/file.php @@ -102,22 +102,4 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase { $file = new OC_Connector_Sabre_File($view, $info); $file->setName('/super*star.txt'); } - - /** - * @expectedException Sabre_DAV_Exception_Forbidden - */ - public function testDeleteSharedFails() { - $view = $this->getMock('\OC\Files\View', array('getRelativePath'), array(), '', false); - - $view->expects($this->any()) - ->method('getRelativePath') - ->will($this->returnValue('Shared')); - - $info = new \OC\Files\FileInfo('/Shared', null, null, array( - 'permissions' => \OCP\PERMISSION_ALL - )); - - $file = new OC_Connector_Sabre_File($view, $info); - $file->delete(); - } }