Merge pull request #7027 from owncloud/extstorage-webdavurlencodingfixes

Fixed ext storage webdav path encoding

4 files changed, 74 insertions, 17 deletions

diff --git a/apps/files_external/lib/webdav.php b/apps/files_external/lib/webdav.php
index 7611316a26a..9afe73aebd7 100644
--- 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';
@@ -346,6 +367,16 @@ class DAV extends \OC\Files\Storage\Common{
 	}
 
 	/**
+	 * 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
 	 * @param integer $expected
@@ -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 e296bfcb5b2..767c0adf58e 100644
--- 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 1f9b767eca6..74e905ccc89 100644
--- 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 182c014d999..f9291758606 100644
--- 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'));