From e55f25b64df09a3ba6535274eaf738e82910e1f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 23 Sep 2013 22:04:37 +0200 Subject: [PATCH 01/14] handle error situation of rename proper --- lib/connector/sabre/directory.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/connector/sabre/directory.php b/lib/connector/sabre/directory.php index 3181a4b310f..9e0fe5e64e7 100644 --- a/lib/connector/sabre/directory.php +++ b/lib/connector/sabre/directory.php @@ -88,7 +88,12 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa } // rename to correct path - \OC\Files\Filesystem::rename($partpath, $newPath); + $renameOkay = \OC\Files\Filesystem::rename($partpath, $newPath); + if (!$renameOkay) { + \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR); + \OC\Files\Filesystem::unlink($partpath); + throw new Sabre_DAV_Exception(); + } // allow sync clients to send the mtime along in a header $mtime = OC_Request::hasModificationTime(); From 31d2048eb83d60007183cec43a54d7a112d7a3b2 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Tue, 24 Sep 2013 11:00:08 +0200 Subject: [PATCH 02/14] add blacklist to txt preview backend --- lib/preview/txt.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/preview/txt.php b/lib/preview/txt.php index a487330691e..77e728eb364 100644 --- a/lib/preview/txt.php +++ b/lib/preview/txt.php @@ -9,11 +9,21 @@ namespace OC\Preview; class TXT extends Provider { + private static $blacklist = array( + 'text/calendar', + 'text/vcard', + ); + public function getMimeType() { return '/text\/.*/'; } public function getThumbnail($path, $maxX, $maxY, $scalingup, $fileview) { + $mimetype = $fileview->getMimeType($path); + if(in_array($mimetype, self::$blacklist)) { + return false; + } + $content = $fileview->fopen($path, 'r'); $content = stream_get_contents($content); From ee1f627155cad4153f3da3160ca6040c137841d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 24 Sep 2013 13:26:12 +0200 Subject: [PATCH 03/14] adding privilege check on move and rename operations --- lib/connector/sabre/node.php | 11 +++++++++++ lib/connector/sabre/objecttree.php | 24 +++++++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/connector/sabre/node.php b/lib/connector/sabre/node.php index 0bffa58af78..29b7f9e53a5 100644 --- a/lib/connector/sabre/node.php +++ b/lib/connector/sabre/node.php @@ -78,6 +78,11 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr */ public function setName($name) { + // rename is only allowed if the update privilege is granted + if (!\OC\Files\Filesystem::isUpdatable($this->path)) { + throw new \Sabre_DAV_Exception_Forbidden(); + } + list($parentPath, ) = Sabre_DAV_URLUtil::splitPath($this->path); list(, $newName) = Sabre_DAV_URLUtil::splitPath($name); @@ -135,6 +140,12 @@ 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) { + + // touch is only allowed if the update privilege is granted + if (!\OC\Files\Filesystem::isUpdatable($this->path)) { + throw new \Sabre_DAV_Exception_Forbidden(); + } + \OC\Files\Filesystem::touch($this->path, $mtime); } diff --git a/lib/connector/sabre/objecttree.php b/lib/connector/sabre/objecttree.php index acff45ed5e2..7accf98c8e1 100644 --- a/lib/connector/sabre/objecttree.php +++ b/lib/connector/sabre/objecttree.php @@ -64,7 +64,29 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { list($sourceDir,) = \Sabre_DAV_URLUtil::splitPath($sourcePath); list($destinationDir,) = \Sabre_DAV_URLUtil::splitPath($destinationPath); - Filesystem::rename($sourcePath, $destinationPath); + // check update privileges + if ($sourceDir === $destinationDir) { + // for renaming it's enough to check if the sourcePath can be updated + if (!\OC\Files\Filesystem::isUpdatable($sourcePath)) { + throw new \Sabre_DAV_Exception_Forbidden(); + } + } else { + // for a full move we need update privileges on sourcePath and sourceDir as well as destinationDir + if (!\OC\Files\Filesystem::isUpdatable($sourcePath)) { + throw new \Sabre_DAV_Exception_Forbidden(); + } + if (!\OC\Files\Filesystem::isUpdatable($sourceDir)) { + throw new \Sabre_DAV_Exception_Forbidden(); + } + if (!\OC\Files\Filesystem::isUpdatable($destinationDir)) { + throw new \Sabre_DAV_Exception_Forbidden(); + } + } + + $renameOkay = Filesystem::rename($sourcePath, $destinationPath); + if (!$renameOkay) { + throw new \Sabre_DAV_Exception_Forbidden(''); + } $this->markDirty($sourceDir); $this->markDirty($destinationDir); From 52f1d5856dfaa9186a05d529674c2dbab9cc90b7 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Tue, 24 Sep 2013 13:26:51 +0200 Subject: [PATCH 04/14] add test data for cal and contact preview --- tests/data/testcal.ics | 13 +++++++++++++ tests/data/testcontact.vcf | 6 ++++++ 2 files changed, 19 insertions(+) create mode 100644 tests/data/testcal.ics create mode 100644 tests/data/testcontact.vcf diff --git a/tests/data/testcal.ics b/tests/data/testcal.ics new file mode 100644 index 00000000000..e05f01ba1c2 --- /dev/null +++ b/tests/data/testcal.ics @@ -0,0 +1,13 @@ +BEGIN:VCALENDAR +PRODID:-//some random cal software//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20130102T120000Z +LAST-MODIFIED:20130102T120000Z +DTSTAMP:20130102T120000Z +UID:f106ecdf-c716-43ef-9d94-4e6f19f2fcfb +SUMMARY:a test cal file +DTSTART;VALUE=DATE:20130101 +DTEND;VALUE=DATE:20130102 +END:VEVENT +END:VCALENDAR \ No newline at end of file diff --git a/tests/data/testcontact.vcf b/tests/data/testcontact.vcf new file mode 100644 index 00000000000..2af963d6916 --- /dev/null +++ b/tests/data/testcontact.vcf @@ -0,0 +1,6 @@ +BEGIN:VCARD +VERSION:3.0 +PRODID:-//some random contact software//EN +N:def;abc;;; +FN:abc def +END:VCARD \ No newline at end of file From 6c5466a540340a22e487d71a2605dcda3498a658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 24 Sep 2013 13:53:32 +0200 Subject: [PATCH 05/14] adding file_exists check just to be on the save side --- lib/connector/sabre/directory.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/connector/sabre/directory.php b/lib/connector/sabre/directory.php index 9e0fe5e64e7..a50098df793 100644 --- a/lib/connector/sabre/directory.php +++ b/lib/connector/sabre/directory.php @@ -89,7 +89,8 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa // rename to correct path $renameOkay = \OC\Files\Filesystem::rename($partpath, $newPath); - if (!$renameOkay) { + $fileExists = \OC\Files\Filesystem::file_exists($newPath); + if ($renameOkay === false || $fileExists === false) { \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR); \OC\Files\Filesystem::unlink($partpath); throw new Sabre_DAV_Exception(); From e9eb34f1872a0237b7474496e8c759a4a3ef1156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 24 Sep 2013 13:54:18 +0200 Subject: [PATCH 06/14] duplicate code :sigh: - will fix this in a second pr --- lib/connector/sabre/file.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/connector/sabre/file.php b/lib/connector/sabre/file.php index 61bdcd5e0ae..433b1148552 100644 --- a/lib/connector/sabre/file.php +++ b/lib/connector/sabre/file.php @@ -74,7 +74,14 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D } // rename to correct path - \OC\Files\Filesystem::rename($partpath, $this->path); + $renameOkay = \OC\Files\Filesystem::rename($partpath, $this->path); + $fileExists = \OC\Files\Filesystem::file_exists($this->path); + if ($renameOkay === false || $fileExists === false) { + \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR); + \OC\Files\Filesystem::unlink($partpath); + throw new Sabre_DAV_Exception(); + } + //allow sync clients to send the mtime along in a header $mtime = OC_Request::hasModificationTime(); From 9e4fe103291133ea78427af18693d93bd78d2bd0 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Wed, 25 Sep 2013 10:20:40 +0200 Subject: [PATCH 07/14] add test for txt blacklist --- tests/lib/preview.php | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/lib/preview.php b/tests/lib/preview.php index bebdc12b500..c40b2d03ef9 100644 --- a/tests/lib/preview.php +++ b/tests/lib/preview.php @@ -92,6 +92,44 @@ class Preview extends \PHPUnit_Framework_TestCase { $this->assertEquals($image->height(), $maxY); } + public function testTxtBlacklist() { + $user = $this->initFS(); + + $x = 32; + $y = 32; + + $txt = 'random text file'; + $ics = file_get_contents(__DIR__ . '/../data/testcal.ics'); + $vcf = file_get_contents(__DIR__ . '/../data/testcontact.vcf'); + + $rootView = new \OC\Files\View(''); + $rootView->mkdir('/'.$user); + $rootView->mkdir('/'.$user.'/files'); + + $toTest = array('txt', + 'ics', + 'vcf'); + + foreach($toTest as $test) { + $sample = '/'.$user.'/files/test.'.$test; + $rootView->file_put_contents($sample, ${$test}); + $preview = new \OC\Preview($user, 'files/', 'test.'.$test, $x, $y); + $image = $preview->getPreview(); + $resource = $image->resource(); + + //http://stackoverflow.com/questions/5702953/imagecolorat-and-transparency + $colorIndex = imagecolorat($resource, 1, 1); + $colorInfo = imagecolorsforindex($resource, $colorIndex); + $isTransparent = ($colorInfo['alpha'] === 127); + + if($test === 'txt') { + $this->assertEquals($isTransparent, false); + } else { + $this->assertEquals($isTransparent, true); + } + } + } + private function initFS() { if(\OC\Files\Filesystem::getView()){ $user = \OC_User::getUser(); From b11d8799c17b24e7823de8d8dc171fb78f9b8442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 26 Sep 2013 10:50:15 +0200 Subject: [PATCH 08/14] adding unit tests for ObjectTree::move() --- lib/connector/sabre/objecttree.php | 44 +++++++--- tests/lib/connector/sabre/objecttree.php | 104 +++++++++++++++++++++++ 2 files changed, 134 insertions(+), 14 deletions(-) create mode 100644 tests/lib/connector/sabre/objecttree.php diff --git a/lib/connector/sabre/objecttree.php b/lib/connector/sabre/objecttree.php index 7accf98c8e1..80c3840b99d 100644 --- a/lib/connector/sabre/objecttree.php +++ b/lib/connector/sabre/objecttree.php @@ -11,6 +11,14 @@ namespace OC\Connector\Sabre; 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; + /** * Returns the INode object for the requested path * @@ -21,14 +29,16 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { public function getNodeForPath($path) { $path = trim($path, '/'); - if (isset($this->cache[$path])) return $this->cache[$path]; + if (isset($this->cache[$path])) { + return $this->cache[$path]; + } // Is it the root node? if (!strlen($path)) { return $this->rootNode; } - $info = Filesystem::getFileInfo($path); + $info = $this->getFileView()->getFileInfo($path); if (!$info) { throw new \Sabre_DAV_Exception_NotFound('File with name ' . $path . ' could not be located'); @@ -65,25 +75,21 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { list($destinationDir,) = \Sabre_DAV_URLUtil::splitPath($destinationPath); // check update privileges - if ($sourceDir === $destinationDir) { - // for renaming it's enough to check if the sourcePath can be updated - if (!\OC\Files\Filesystem::isUpdatable($sourcePath)) { - throw new \Sabre_DAV_Exception_Forbidden(); - } - } else { + $fs = $this->getFileView(); + if (!$fs->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 (!\OC\Files\Filesystem::isUpdatable($sourcePath)) { + if (!$fs->isUpdatable($sourceDir)) { throw new \Sabre_DAV_Exception_Forbidden(); } - if (!\OC\Files\Filesystem::isUpdatable($sourceDir)) { - throw new \Sabre_DAV_Exception_Forbidden(); - } - if (!\OC\Files\Filesystem::isUpdatable($destinationDir)) { + if (!$fs->isUpdatable($destinationDir)) { throw new \Sabre_DAV_Exception_Forbidden(); } } - $renameOkay = Filesystem::rename($sourcePath, $destinationPath); + $renameOkay = $fs->rename($sourcePath, $destinationPath); if (!$renameOkay) { throw new \Sabre_DAV_Exception_Forbidden(''); } @@ -123,4 +129,14 @@ 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/tests/lib/connector/sabre/objecttree.php b/tests/lib/connector/sabre/objecttree.php new file mode 100644 index 00000000000..c920441c8b4 --- /dev/null +++ b/tests/lib/connector/sabre/objecttree.php @@ -0,0 +1,104 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test\OC\Connector\Sabre; + + +use OC_Connector_Sabre_Directory; +use PHPUnit_Framework_TestCase; +use Sabre_DAV_Exception_Forbidden; + +class TestDoubleFileView extends \OC\Files\View{ + + public function __construct($updatables, $canRename = true) { + $this->updatables = $updatables; + $this->canRename = $canRename; + } + + public function isUpdatable($path) { + return $this->updatables[$path]; + } + + public function rename($path1, $path2) { + return $this->canRename; + } +} + +class ObjectTree extends PHPUnit_Framework_TestCase { + + /** + * @dataProvider moveFailedProvider + * @expectedException Sabre_DAV_Exception_Forbidden + */ + public function testMoveFailed($source, $dest, $updatables) { + $rootDir = new OC_Connector_Sabre_Directory(''); + $objectTree = $this->getMock('\OC\Connector\Sabre\ObjectTree', + array('nodeExists', 'getNodeForPath'), + array($rootDir)); + + $objectTree->expects($this->once()) + ->method('getNodeForPath') + ->with($this->identicalTo('a/b')) + ->will($this->returnValue(false)); + + /** @var $objectTree \OC\Connector\Sabre\ObjectTree */ + $objectTree->fileView = new TestDoubleFileView($updatables); + $objectTree->move($source, $dest); + } + + /** + * @dataProvider moveSuccessProvider + */ + public function testMoveSuccess($source, $dest, $updatables) { + $rootDir = new OC_Connector_Sabre_Directory(''); + $objectTree = $this->getMock('\OC\Connector\Sabre\ObjectTree', + array('nodeExists', 'getNodeForPath'), + array($rootDir)); + + $objectTree->expects($this->once()) + ->method('getNodeForPath') + ->with($this->identicalTo('a/b')) + ->will($this->returnValue(false)); + + /** @var $objectTree \OC\Connector\Sabre\ObjectTree */ + $objectTree->fileView = new TestDoubleFileView($updatables); + $objectTree->move($source, $dest); + $this->assertTrue(true); + } + + function moveFailedProvider() { + return array( + array('a/b', 'a/c', array('a' => false, 'a/b' => false, 'a/c' => false)), + array('a/b', 'b/b', array('a' => false, 'a/b' => false, 'b' => false, 'b/b' => false)), + array('a/b', 'b/b', array('a' => false, 'a/b' => true, 'b' => false, 'b/b' => false)), + array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => false, 'b/b' => false)), + ); + } + + function moveSuccessProvider() { + return array( + array('a/b', 'a/c', array('a' => false, 'a/b' => true, 'a/c' => false)), + array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => true, 'b/b' => false)), + ); + } + +// private function buildFileViewMock($updatables) { +// // mock filesysten +// $view = $this->getMock('\OC\Files\View', array('isUpdatable'), array(), '', FALSE); +// +// foreach ($updatables as $path => $updatable) { +// $view->expects($this->any()) +// ->method('isUpdatable') +// ->with($this->identicalTo($path)) +// ->will($this->returnValue($updatable)); +// } +// +// return $view; +// } + +} From aa8a145ba8fe5a429698ac9c508704952a454358 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 27 Sep 2013 09:59:04 +0200 Subject: [PATCH 09/14] use dataProvider for txt blacklist test --- tests/lib/preview.php | 47 +++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/tests/lib/preview.php b/tests/lib/preview.php index c40b2d03ef9..5dc4a93acc5 100644 --- a/tests/lib/preview.php +++ b/tests/lib/preview.php @@ -95,9 +95,6 @@ class Preview extends \PHPUnit_Framework_TestCase { public function testTxtBlacklist() { $user = $this->initFS(); - $x = 32; - $y = 32; - $txt = 'random text file'; $ics = file_get_contents(__DIR__ . '/../data/testcal.ics'); $vcf = file_get_contents(__DIR__ . '/../data/testcontact.vcf'); @@ -106,28 +103,34 @@ class Preview extends \PHPUnit_Framework_TestCase { $rootView->mkdir('/'.$user); $rootView->mkdir('/'.$user.'/files'); - $toTest = array('txt', - 'ics', - 'vcf'); + return array( + array('txt', $txt, $user, $rootView, false), + array('ics', $ics, $user, $rootView, true), + array('vcf', $vcf, $user, $rootView, true), + ); + } - foreach($toTest as $test) { - $sample = '/'.$user.'/files/test.'.$test; - $rootView->file_put_contents($sample, ${$test}); - $preview = new \OC\Preview($user, 'files/', 'test.'.$test, $x, $y); - $image = $preview->getPreview(); - $resource = $image->resource(); + /** + * @dataProvider testTxtBlacklist + */ + public function testIsTransparent($test, $data, $user, $rootView, $expectedResult) { + $x = 32; + $y = 32; - //http://stackoverflow.com/questions/5702953/imagecolorat-and-transparency - $colorIndex = imagecolorat($resource, 1, 1); - $colorInfo = imagecolorsforindex($resource, $colorIndex); - $isTransparent = ($colorInfo['alpha'] === 127); + $sample = '/'.$user.'/files/test.'.$test; + $rootView->file_put_contents($sample, $data); + $preview = new \OC\Preview($user, 'files/', 'test.'.$test, $x, $y); + $image = $preview->getPreview(); + $resource = $image->resource(); - if($test === 'txt') { - $this->assertEquals($isTransparent, false); - } else { - $this->assertEquals($isTransparent, true); - } - } + //http://stackoverflow.com/questions/5702953/imagecolorat-and-transparency + $colorIndex = imagecolorat($resource, 1, 1); + $colorInfo = imagecolorsforindex($resource, $colorIndex); + $this->assertEquals( + $expectedResult, + $colorInfo['alpha'] === 127, + 'Failed asserting that only previews for text files are transparent.' + ); } private function initFS() { From 1b13101096493c0c8f02826e373af761fd5cfac6 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 27 Sep 2013 11:01:47 +0200 Subject: [PATCH 10/14] move fileView object initialization to testIsTransparent --- tests/lib/preview.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/lib/preview.php b/tests/lib/preview.php index 5dc4a93acc5..98659a7e887 100644 --- a/tests/lib/preview.php +++ b/tests/lib/preview.php @@ -93,27 +93,27 @@ class Preview extends \PHPUnit_Framework_TestCase { } public function testTxtBlacklist() { - $user = $this->initFS(); - $txt = 'random text file'; $ics = file_get_contents(__DIR__ . '/../data/testcal.ics'); $vcf = file_get_contents(__DIR__ . '/../data/testcontact.vcf'); - $rootView = new \OC\Files\View(''); - $rootView->mkdir('/'.$user); - $rootView->mkdir('/'.$user.'/files'); - return array( - array('txt', $txt, $user, $rootView, false), - array('ics', $ics, $user, $rootView, true), - array('vcf', $vcf, $user, $rootView, true), + array('txt', $txt, false), + array('ics', $ics, true), + array('vcf', $vcf, true), ); } /** * @dataProvider testTxtBlacklist */ - public function testIsTransparent($test, $data, $user, $rootView, $expectedResult) { + public function testIsTransparent($test, $data, $expectedResult) { + $user = $this->initFS(); + + $rootView = new \OC\Files\View(''); + $rootView->mkdir('/'.$user); + $rootView->mkdir('/'.$user.'/files'); + $x = 32; $y = 32; From 4e9296a484fbec0a4c725f32ef47716f9e1b56bc Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 27 Sep 2013 11:33:37 +0200 Subject: [PATCH 11/14] rename testTxtBlacklist to txtBlacklist --- tests/lib/preview.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lib/preview.php b/tests/lib/preview.php index 98659a7e887..01d8a57fc68 100644 --- a/tests/lib/preview.php +++ b/tests/lib/preview.php @@ -92,7 +92,7 @@ class Preview extends \PHPUnit_Framework_TestCase { $this->assertEquals($image->height(), $maxY); } - public function testTxtBlacklist() { + public function txtBlacklist() { $txt = 'random text file'; $ics = file_get_contents(__DIR__ . '/../data/testcal.ics'); $vcf = file_get_contents(__DIR__ . '/../data/testcontact.vcf'); @@ -105,7 +105,7 @@ class Preview extends \PHPUnit_Framework_TestCase { } /** - * @dataProvider testTxtBlacklist + * @dataProvider txtBlacklist */ public function testIsTransparent($test, $data, $expectedResult) { $user = $this->initFS(); @@ -146,4 +146,4 @@ class Preview extends \PHPUnit_Framework_TestCase { return $user; } -} \ No newline at end of file +} From 79da35b698a398bef59f83f222de3055ddbb5a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 27 Sep 2013 13:41:23 +0200 Subject: [PATCH 12/14] code cleanup --- tests/lib/connector/sabre/objecttree.php | 61 +++++++++--------------- 1 file changed, 22 insertions(+), 39 deletions(-) diff --git a/tests/lib/connector/sabre/objecttree.php b/tests/lib/connector/sabre/objecttree.php index c920441c8b4..1d76bb59676 100644 --- a/tests/lib/connector/sabre/objecttree.php +++ b/tests/lib/connector/sabre/objecttree.php @@ -36,38 +36,14 @@ class ObjectTree extends PHPUnit_Framework_TestCase { * @expectedException Sabre_DAV_Exception_Forbidden */ public function testMoveFailed($source, $dest, $updatables) { - $rootDir = new OC_Connector_Sabre_Directory(''); - $objectTree = $this->getMock('\OC\Connector\Sabre\ObjectTree', - array('nodeExists', 'getNodeForPath'), - array($rootDir)); - - $objectTree->expects($this->once()) - ->method('getNodeForPath') - ->with($this->identicalTo('a/b')) - ->will($this->returnValue(false)); - - /** @var $objectTree \OC\Connector\Sabre\ObjectTree */ - $objectTree->fileView = new TestDoubleFileView($updatables); - $objectTree->move($source, $dest); + $this->moveTest($source, $dest, $updatables); } /** * @dataProvider moveSuccessProvider */ public function testMoveSuccess($source, $dest, $updatables) { - $rootDir = new OC_Connector_Sabre_Directory(''); - $objectTree = $this->getMock('\OC\Connector\Sabre\ObjectTree', - array('nodeExists', 'getNodeForPath'), - array($rootDir)); - - $objectTree->expects($this->once()) - ->method('getNodeForPath') - ->with($this->identicalTo('a/b')) - ->will($this->returnValue(false)); - - /** @var $objectTree \OC\Connector\Sabre\ObjectTree */ - $objectTree->fileView = new TestDoubleFileView($updatables); - $objectTree->move($source, $dest); + $this->moveTest($source, $dest, $updatables); $this->assertTrue(true); } @@ -87,18 +63,25 @@ class ObjectTree extends PHPUnit_Framework_TestCase { ); } -// private function buildFileViewMock($updatables) { -// // mock filesysten -// $view = $this->getMock('\OC\Files\View', array('isUpdatable'), array(), '', FALSE); -// -// foreach ($updatables as $path => $updatable) { -// $view->expects($this->any()) -// ->method('isUpdatable') -// ->with($this->identicalTo($path)) -// ->will($this->returnValue($updatable)); -// } -// -// return $view; -// } + /** + * @param $source + * @param $dest + * @param $updatables + */ + private function moveTest($source, $dest, $updatables) { + $rootDir = new OC_Connector_Sabre_Directory(''); + $objectTree = $this->getMock('\OC\Connector\Sabre\ObjectTree', + array('nodeExists', 'getNodeForPath'), + array($rootDir)); + + $objectTree->expects($this->once()) + ->method('getNodeForPath') + ->with($this->identicalTo($source)) + ->will($this->returnValue(false)); + + /** @var $objectTree \OC\Connector\Sabre\ObjectTree */ + $objectTree->fileView = new TestDoubleFileView($updatables); + $objectTree->move($source, $dest); + } } From c5bcefe4dbd5a237693b1a7435a7041c7e85abd4 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 27 Sep 2013 14:55:37 +0200 Subject: [PATCH 13/14] rename variable in testIsTransparent --- tests/lib/preview.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lib/preview.php b/tests/lib/preview.php index 01d8a57fc68..d0cdd2c44fb 100644 --- a/tests/lib/preview.php +++ b/tests/lib/preview.php @@ -107,7 +107,7 @@ class Preview extends \PHPUnit_Framework_TestCase { /** * @dataProvider txtBlacklist */ - public function testIsTransparent($test, $data, $expectedResult) { + public function testIsTransparent($extension, $data, $expectedResult) { $user = $this->initFS(); $rootView = new \OC\Files\View(''); @@ -117,9 +117,9 @@ class Preview extends \PHPUnit_Framework_TestCase { $x = 32; $y = 32; - $sample = '/'.$user.'/files/test.'.$test; + $sample = '/'.$user.'/files/test.'.$extension; $rootView->file_put_contents($sample, $data); - $preview = new \OC\Preview($user, 'files/', 'test.'.$test, $x, $y); + $preview = new \OC\Preview($user, 'files/', 'test.'.$extension, $x, $y); $image = $preview->getPreview(); $resource = $image->resource(); From b6fc143074ae55bc6f9732d825571681f88f46ed Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Fri, 31 May 2013 18:21:31 +0200 Subject: [PATCH 14/14] cURL does not honour default_socket_timeout SabreDAV uses cURL for the requests and as this does not honour default_socket_timeout, setting it is useless and confusing as people will expect the request to timeout faster than it actually will do. One has to use curl_setopt($curl, CURLOPT_TIMEOUT, x) or curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, x) but there is currently no way to make SabreDAV pass this to cURL. --- lib/util.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/util.php b/lib/util.php index 41f5f1d16be..d4f4eed1ca7 100755 --- a/lib/util.php +++ b/lib/util.php @@ -730,12 +730,6 @@ class OC_Util { 'baseUri' => OC_Helper::linkToRemote('webdav'), ); - // save the old timeout so that we can restore it later - $oldTimeout = ini_get("default_socket_timeout"); - - // use a 5 sec timeout for the check. Should be enough for local requests. - ini_set("default_socket_timeout", 5); - $client = new \Sabre_DAV_Client($settings); // for this self test we don't care if the ssl certificate is self signed and the peer cannot be verified. @@ -752,9 +746,6 @@ class OC_Util { $return = false; } - // restore the original timeout - ini_set("default_socket_timeout", $oldTimeout); - return $return; }