Fixed ext storage webdav path encoding

- Some WebDAV servers like lighttpd need paths in URLs to be properly
encoded
- Added error log output when curl connection failed
- Added check for 'resourcetype' in case the WebDAV server doesn't
  support/return it
- Fixed touch() to return false if the server doesn't implement
  PROPPATCH
- Added optional delay in WebDAV unit tests to use when testing against
  lighttpd's WebDAV

diff --git a/apps/files_external/lib/webdav.php b/apps/files_external/lib/webdav.php
index 7611316a26a5e2f36d64a30ded3fd10d57eac220..9afe73aebd7e9020172ea3639a4a568d8b55b89e 100644 (file)
--- a/apps/files_external/lib/webdav.php
+++ b/apps/files_external/lib/webdav.php
@@ -99,7 +99,9 @@ class DAV extends \OC\Files\Storage\Common{
 
        public function rmdir($path) {
                $this->init();
-               $path=$this->cleanPath($path);
+               $path=$this->cleanPath($path) . '/';
+               // FIXME: some WebDAV impl return 403 when trying to DELETE
+               // a non-empty folder
                return $this->simpleResponse('DELETE', $path, null, 204);
        }
 
@@ -107,7 +109,7 @@ class DAV extends \OC\Files\Storage\Common{
                $this->init();
                $path=$this->cleanPath($path);
                try {
-                       $response=$this->client->propfind($path, array(), 1);
+                       $response=$this->client->propfind($this->encodePath($path), array(), 1);
                        $id=md5('webdav'.$this->root.$path);
                        $content = array();
                        $files=array_keys($response);
@@ -127,8 +129,11 @@ class DAV extends \OC\Files\Storage\Common{
                $this->init();
                $path=$this->cleanPath($path);
                try {
-                       $response=$this->client->propfind($path, array('{DAV:}resourcetype'));
-                       $responseType=$response["{DAV:}resourcetype"]->resourceType;
+                       $response=$this->client->propfind($this->encodePath($path), array('{DAV:}resourcetype'));
+                       $responseType = array();
+                       if (isset($response["{DAV:}resourcetype"])) {
+                               $responseType=$response["{DAV:}resourcetype"]->resourceType;
+                       }
                        return (count($responseType)>0 and $responseType[0]=="{DAV:}collection")?'dir':'file';
                } catch(\Exception $e) {
                        error_log($e->getMessage());
@@ -141,7 +146,7 @@ class DAV extends \OC\Files\Storage\Common{
                $this->init();
                $path=$this->cleanPath($path);
                try {
-                       $this->client->propfind($path, array('{DAV:}resourcetype'));
+                       $this->client->propfind($this->encodePath($path), array('{DAV:}resourcetype'));
                        return true;//no 404 exception
                } catch(\Exception $e) {
                        return false;
@@ -166,7 +171,7 @@ class DAV extends \OC\Files\Storage\Common{
                                $curl = curl_init();
                                $fp = fopen('php://temp', 'r+');
                                curl_setopt($curl, CURLOPT_USERPWD, $this->user.':'.$this->password);
-                               curl_setopt($curl, CURLOPT_URL, $this->createBaseUri().str_replace(' ', '%20', $path));
+                               curl_setopt($curl, CURLOPT_URL, $this->createBaseUri().$this->encodePath($path));
                                curl_setopt($curl, CURLOPT_FILE, $fp);
                                curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
                                if ($this->secure === true) {
@@ -178,6 +183,10 @@ class DAV extends \OC\Files\Storage\Common{
                                }
                                
                                curl_exec ($curl);
+                               $statusCode = curl_getinfo($curl, CURLINFO_HTTP_CODE);
+                               if ($statusCode !== 200) {
+                                       \OCP\Util::writeLog("webdav client", 'curl GET ' . curl_getinfo($curl, CURLINFO_EFFECTIVE_URL) . ' returned status code ' . $statusCode, \OCP\Util::ERROR);
+                               }
                                curl_close ($curl);
                                rewind($fp);
                                return $fp;
@@ -220,7 +229,7 @@ class DAV extends \OC\Files\Storage\Common{
                $this->init();
                $path=$this->cleanPath($path);
                try {
-                       $response=$this->client->propfind($path, array('{DAV:}quota-available-bytes'));
+                       $response=$this->client->propfind($this->encodePath($path), array('{DAV:}quota-available-bytes'));
                        if (isset($response['{DAV:}quota-available-bytes'])) {
                                return (int)$response['{DAV:}quota-available-bytes'];
                        } else {
@@ -240,7 +249,12 @@ class DAV extends \OC\Files\Storage\Common{
 
                // if file exists, update the mtime, else create a new empty file
                if ($this->file_exists($path)) {
-                       $this->client->proppatch($path, array('{DAV:}lastmodified' => $mtime));
+                       try {
+                               $this->client->proppatch($this->encodePath($path), array('{DAV:}lastmodified' => $mtime));
+                       }
+                       catch (\Sabre_DAV_Exception_NotImplemented $e) {
+                               return false;
+                       }
                } else {
                        $this->file_put_contents($path, '');
                }
@@ -276,13 +290,17 @@ class DAV extends \OC\Files\Storage\Common{
                        }
                }
                curl_exec ($curl);
+               $statusCode = curl_getinfo($curl, CURLINFO_HTTP_CODE);
+               if ($statusCode !== 200) {
+                       \OCP\Util::writeLog("webdav client", 'curl GET ' . curl_getinfo($curl, CURLINFO_EFFECTIVE_URL) . ' returned status code ' . $statusCode, \OCP\Util::ERROR);
+               }
                curl_close ($curl);
        }
 
        public function rename($path1, $path2) {
                $this->init();
-               $path1=$this->cleanPath($path1);
-               $path2=$this->createBaseUri().$this->cleanPath($path2);
+               $path1 = $this->encodePath($this->cleanPath($path1));
+               $path2 = $this->createBaseUri().$this->encodePath($this->cleanPath($path2));
                try {
                        $this->client->request('MOVE', $path1, null, array('Destination'=>$path2));
                        return true;
@@ -293,8 +311,8 @@ class DAV extends \OC\Files\Storage\Common{
 
        public function copy($path1, $path2) {
                $this->init();
-               $path1=$this->cleanPath($path1);
-               $path2=$this->createBaseUri().$this->cleanPath($path2);
+               $path1 = $this->encodePath($this->cleanPath($path1));
+               $path2 = $this->createBaseUri().$this->encodePath($this->cleanPath($path2));
                try {
                        $this->client->request('COPY', $path1, null, array('Destination'=>$path2));
                        return true;
@@ -307,7 +325,7 @@ class DAV extends \OC\Files\Storage\Common{
                $this->init();
                $path=$this->cleanPath($path);
                try {
-                       $response=$this->client->propfind($path, array('{DAV:}getlastmodified', '{DAV:}getcontentlength'));
+                       $response = $this->client->propfind($this->encodePath($path), array('{DAV:}getlastmodified', '{DAV:}getcontentlength'));
                        return array(
                                'mtime'=>strtotime($response['{DAV:}getlastmodified']),
                                'size'=>(int)isset($response['{DAV:}getcontentlength']) ? $response['{DAV:}getcontentlength'] : 0,
@@ -321,8 +339,11 @@ class DAV extends \OC\Files\Storage\Common{
                $this->init();
                $path=$this->cleanPath($path);
                try {
-                       $response=$this->client->propfind($path, array('{DAV:}getcontenttype', '{DAV:}resourcetype'));
-                       $responseType=$response["{DAV:}resourcetype"]->resourceType;
+                       $response=$this->client->propfind($this->encodePath($path), array('{DAV:}getcontenttype', '{DAV:}resourcetype'));
+                       $responseType = array();
+                       if (isset($response["{DAV:}resourcetype"])) {
+                               $responseType=$response["{DAV:}resourcetype"]->resourceType;
+                       }
                        $type=(count($responseType)>0 and $responseType[0]=="{DAV:}collection")?'dir':'file';
                        if ($type=='dir') {
                                return 'httpd/unix-directory';
@@ -345,6 +366,16 @@ class DAV extends \OC\Files\Storage\Common{
                return substr($path, 1);
        }
 
+       /**
+        * URL encodes the given path but keeps the slashes
+        * @param string $path to encode
+        * @return string encoded path
+        */
+       private function encodePath($path) {
+               // slashes need to stay
+               return str_replace('%2F', '/', rawurlencode($path));
+       }
+
        /**
         * @param string $method
         * @param string $path
@@ -353,7 +384,7 @@ class DAV extends \OC\Files\Storage\Common{
        private function simpleResponse($method, $path, $body, $expected) {
                $path=$this->cleanPath($path);
                try {
-                       $response=$this->client->request($method, $path, $body);
+                       $response=$this->client->request($method, $this->encodePath($path), $body);
                        return $response['statusCode']==$expected;
                } catch(\Exception $e) {
                        return false;

diff --git a/apps/files_external/tests/config.php b/apps/files_external/tests/config.php
index e296bfcb5b243d199ceb8f5475711c4f77e62821..767c0adf58e17bbeabd4ce724b9226392b470258 100644 (file)
--- a/apps/files_external/tests/config.php
+++ b/apps/files_external/tests/config.php
@@ -21,7 +21,11 @@ return array(
                'host'=>'localhost',
                'user'=>'test',
                'password'=>'test',
-               'root'=>'/owncloud/files/webdav.php',
+               'root'=>'',
+               // wait delay in seconds after write operations
+               // (only in tests)
+               // set to higher value for lighttpd webdav
+               'wait'=> 0
        ),
        'owncloud'=>array(
                'run'=>true,

diff --git a/apps/files_external/tests/webdav.php b/apps/files_external/tests/webdav.php
index 1f9b767eca69d24ca1fff865471972f7c6a79eb6..74e905ccc89eb5704d0651534402b9d13fcb2015 100644 (file)
--- a/apps/files_external/tests/webdav.php
+++ b/apps/files_external/tests/webdav.php
@@ -18,6 +18,9 @@ class DAV extends Storage {
                if ( ! is_array($this->config) or ! isset($this->config['webdav']) or ! $this->config['webdav']['run']) {
                        $this->markTestSkipped('WebDAV backend not configured');
                }
+               if (isset($this->config['webdav']['wait'])) {
+                       $this->waitDelay = $this->config['webdav']['wait'];
+               }
                $this->config['webdav']['root'] .= '/' . $id; //make sure we have an new empty folder to work in
                $this->instance = new \OC\Files\Storage\DAV($this->config['webdav']);
                $this->instance->mkdir('/');

diff --git a/tests/lib/files/storage/storage.php b/tests/lib/files/storage/storage.php
index 182c014d9996690a6891cd6a62df7011f41dccc7..f9291758606dc3cb327ac459ef264c9f1ca822d5 100644 (file)
--- a/tests/lib/files/storage/storage.php
+++ b/tests/lib/files/storage/storage.php
@@ -27,6 +27,17 @@ abstract class Storage extends \PHPUnit_Framework_TestCase {
         * @var \OC\Files\Storage\Storage instance
         */
        protected $instance;
+       protected $waitDelay = 0;
+
+       /**
+        * Sleep for the number of seconds specified in the
+        * $waitDelay attribute
+        */
+       protected function wait() {
+               if ($this->waitDelay > 0) {
+                       sleep($this->waitDelay);
+               }
+       }
 
        /**
         * the root folder of the storage should always exist, be readable and be recognized as a directory
@@ -77,6 +88,7 @@ abstract class Storage extends \PHPUnit_Framework_TestCase {
                $this->assertFalse($this->instance->mkdir('/'.$directory)); //cant create existing folders
                $this->assertTrue($this->instance->rmdir('/'.$directory));
 
+               $this->wait();
                $this->assertFalse($this->instance->file_exists('/'.$directory));
 
                $this->assertFalse($this->instance->rmdir('/'.$directory)); //cant remove non existing folders
@@ -97,6 +109,8 @@ abstract class Storage extends \PHPUnit_Framework_TestCase {
                        array('folder'),
                        array(' folder'),
                        array('folder '),
+                       array('folder with space'),
+                       array('spéciäl földer'),
                );
        }
        /**
@@ -144,6 +158,7 @@ abstract class Storage extends \PHPUnit_Framework_TestCase {
                $this->assertEquals($this->instance->file_get_contents('/source.txt'), $this->instance->file_get_contents('/target.txt'));
 
                $this->instance->rename('/source.txt', '/target2.txt');
+               $this->wait();
                $this->assertTrue($this->instance->file_exists('/target2.txt'));
                $this->assertFalse($this->instance->file_exists('/source.txt'));
                $this->assertEquals(file_get_contents($textFile), $this->instance->file_get_contents('/target2.txt'));
@@ -225,6 +240,7 @@ abstract class Storage extends \PHPUnit_Framework_TestCase {
                $this->assertTrue($this->instance->file_exists('/lorem.txt'));
 
                $this->assertTrue($this->instance->unlink('/lorem.txt'));
+               $this->wait();
 
                $this->assertFalse($this->instance->file_exists('/lorem.txt'));
        }
@@ -259,9 +275,11 @@ abstract class Storage extends \PHPUnit_Framework_TestCase {
        public function testRecursiveRmdir() {
                $this->instance->mkdir('folder');
                $this->instance->mkdir('folder/bar');
+               $this->wait();
                $this->instance->file_put_contents('folder/asd.txt', 'foobar');
                $this->instance->file_put_contents('folder/bar/foo.txt', 'asd');
                $this->assertTrue($this->instance->rmdir('folder'));
+               $this->wait();
                $this->assertFalse($this->instance->file_exists('folder/asd.txt'));
                $this->assertFalse($this->instance->file_exists('folder/bar/foo.txt'));
                $this->assertFalse($this->instance->file_exists('folder/bar'));
@@ -274,6 +292,7 @@ abstract class Storage extends \PHPUnit_Framework_TestCase {
                $this->instance->file_put_contents('folder/asd.txt', 'foobar');
                $this->instance->file_put_contents('folder/bar/foo.txt', 'asd');
                $this->assertTrue($this->instance->unlink('folder'));
+               $this->wait();
                $this->assertFalse($this->instance->file_exists('folder/asd.txt'));
                $this->assertFalse($this->instance->file_exists('folder/bar/foo.txt'));
                $this->assertFalse($this->instance->file_exists('folder/bar'));