diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index f172bde5f1..32cc8b7ade 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -36,13 +36,16 @@ namespace OCA\DAV\Connector\Sabre; +use OC\AppFramework\Http\Request; use OC\Files\Filesystem; +use OC\Files\View; use OCA\DAV\Connector\Sabre\Exception\EntityTooLarge; use OCA\DAV\Connector\Sabre\Exception\FileLocked; use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException; use OCA\DAV\Connector\Sabre\Exception\UnsupportedMediaType; use OCP\Encryption\Exceptions\GenericEncryptionException; use OCP\Files\EntityTooLargeException; +use OCP\Files\FileInfo; use OCP\Files\ForbiddenException; use OCP\Files\InvalidContentException; use OCP\Files\InvalidPathException; @@ -51,6 +54,7 @@ use OCP\Files\NotPermittedException; use OCP\Files\StorageNotAvailableException; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; +use OCP\Share\IManager; use Sabre\DAV\Exception; use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Forbidden; @@ -61,6 +65,26 @@ use Sabre\DAV\Exception\NotFound; class File extends Node implements IFile { + protected $request; + + /** + * Sets up the node, expects a full path name + * + * @param \OC\Files\View $view + * @param \OCP\Files\FileInfo $info + * @param \OCP\Share\IManager $shareManager + * @param \OC\AppFramework\Http\Request $request + */ + public function __construct(View $view, FileInfo $info, IManager $shareManager = null, Request $request = null) { + parent::__construct($view, $info, $shareManager); + + if (isset($request)) { + $this->request = $request; + } else { + $this->request = \OC::$server->getRequest(); + } + } + /** * Updates the data * @@ -208,15 +232,10 @@ class File extends Node implements IFile { } // allow sync clients to send the mtime along in a header - $request = \OC::$server->getRequest(); - if (isset($request->server['HTTP_X_OC_MTIME'])) { - $mtimeStr = $request->server['HTTP_X_OC_MTIME']; - if (!is_numeric($mtimeStr)) { - throw new \InvalidArgumentException('X-OC-Mtime header must be an integer (unix timestamp).'); - } - $mtime = intval($mtimeStr); + if (isset($this->request->server['HTTP_X_OC_MTIME'])) { + $mtime = $this->sanitizeMtime($this->request->server['HTTP_X_OC_MTIME']); if ($this->fileView->touch($this->path, $mtime)) { - header('X-OC-MTime: accepted'); + $this->header('X-OC-MTime: accepted'); } } @@ -226,8 +245,8 @@ class File extends Node implements IFile { $this->refreshInfo(); - if (isset($request->server['HTTP_OC_CHECKSUM'])) { - $checksum = trim($request->server['HTTP_OC_CHECKSUM']); + if (isset($this->request->server['HTTP_OC_CHECKSUM'])) { + $checksum = trim($this->request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($this->path, ['checksum' => $checksum]); $this->refreshInfo(); } else if ($this->getChecksum() !== null && $this->getChecksum() !== '') { @@ -470,10 +489,10 @@ class File extends Node implements IFile { } // allow sync clients to send the mtime along in a header - $request = \OC::$server->getRequest(); - if (isset($request->server['HTTP_X_OC_MTIME'])) { - if ($targetStorage->touch($targetInternalPath, $request->server['HTTP_X_OC_MTIME'])) { - header('X-OC-MTime: accepted'); + if (isset($this->request->server['HTTP_X_OC_MTIME'])) { + $mtime = $this->sanitizeMtime($this->request->server['HTTP_X_OC_MTIME']); + if ($targetStorage->touch($targetInternalPath, $mtime)) { + $this->header('X-OC-MTime: accepted'); } } @@ -487,8 +506,8 @@ class File extends Node implements IFile { // FIXME: should call refreshInfo but can't because $this->path is not the of the final file $info = $this->fileView->getFileInfo($targetPath); - if (isset($request->server['HTTP_OC_CHECKSUM'])) { - $checksum = trim($request->server['HTTP_OC_CHECKSUM']); + if (isset($this->request->server['HTTP_OC_CHECKSUM'])) { + $checksum = trim($this->request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]); } else if ($info->getChecksum() !== null && $info->getChecksum() !== '') { $this->fileView->putFileInfo($this->path, ['checksum' => '']); @@ -570,6 +589,18 @@ class File extends Node implements IFile { throw new \Sabre\DAV\Exception($e->getMessage(), 0, $e); } + private function sanitizeMtime($mtimeFromRequest) { + // In PHP 5.X "is_numeric" returns true for strings in hexadecimal + // notation. This is no longer the case in PHP 7.X, so this check + // ensures that strings with hexadecimal notations fail too in PHP 5.X. + $isHexadecimal = is_string($mtimeFromRequest) && preg_match('/^\s*0[xX]/', $mtimeFromRequest); + if ($isHexadecimal || !is_numeric($mtimeFromRequest)) { + throw new \InvalidArgumentException('X-OC-MTime header must be an integer (unix timestamp).'); + } + + return intval($mtimeFromRequest); + } + /** * Get the checksum for this file * @@ -578,4 +609,8 @@ class File extends Node implements IFile { public function getChecksum() { return $this->info->getChecksum(); } + + protected function header($string) { + \header($string); + } } diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index 4d106842cf..1db9b7948e 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -31,6 +31,7 @@ use OC\Files\Storage\Local; use OC\Files\View; use OCP\Files\ForbiddenException; use OCP\Files\Storage; +use OCP\IConfig; use Test\HookHelper; use OC\Files\Filesystem; use OCP\Lock\ILockingProvider; @@ -49,6 +50,9 @@ class FileTest extends \Test\TestCase { */ private $user; + /** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */ + protected $config; + public function setUp() { parent::setUp(); unset($_SERVER['HTTP_OC_CHUNKED']); @@ -62,6 +66,8 @@ class FileTest extends \Test\TestCase { $userManager->createUser($this->user, 'pass'); $this->loginAsUser($this->user); + + $this->config = $this->getMockBuilder('\OCP\IConfig')->getMock(); } public function tearDown() { @@ -284,10 +290,11 @@ class FileTest extends \Test\TestCase { * * @param string $path path to put the file into * @param string $viewRoot root to use for the view + * @param null|\OC\AppFramework\Http\Request $request the HTTP request * * @return null|string of the PUT operaiton which is usually the etag */ - private function doPut($path, $viewRoot = null) { + private function doPut($path, $viewRoot = null, \OC\AppFramework\Http\Request $request = null) { $view = \OC\Files\Filesystem::getView(); if (!is_null($viewRoot)) { $view = new \OC\Files\View($viewRoot); @@ -303,7 +310,11 @@ class FileTest extends \Test\TestCase { null ); - $file = new \OCA\DAV\Connector\Sabre\File($view, $info); + /** @var \OCA\DAV\Connector\Sabre\File | \PHPUnit_Framework_MockObject_MockObject $file */ + $file = $this->getMockBuilder(\OCA\DAV\Connector\Sabre\File::class) + ->setConstructorArgs([$view, $info, null, $request]) + ->setMethods(['header']) + ->getMock(); // beforeMethod locks $view->lockFile($path, ILockingProvider::LOCK_SHARED); @@ -323,6 +334,110 @@ class FileTest extends \Test\TestCase { $this->assertNotEmpty($this->doPut('/foo.txt')); } + public function legalMtimeProvider() { + return [ + "string" => [ + 'HTTP_X_OC_MTIME' => "string", + 'expected result' => null + ], + "castable string (int)" => [ + 'HTTP_X_OC_MTIME' => "34", + 'expected result' => 34 + ], + "castable string (float)" => [ + 'HTTP_X_OC_MTIME' => "34.56", + 'expected result' => 34 + ], + "float" => [ + 'HTTP_X_OC_MTIME' => 34.56, + 'expected result' => 34 + ], + "zero" => [ + 'HTTP_X_OC_MTIME' => 0, + 'expected result' => 0 + ], + "zero string" => [ + 'HTTP_X_OC_MTIME' => "0", + 'expected result' => 0 + ], + "negative zero string" => [ + 'HTTP_X_OC_MTIME' => "-0", + 'expected result' => 0 + ], + "string starting with number following by char" => [ + 'HTTP_X_OC_MTIME' => "2345asdf", + 'expected result' => null + ], + "string castable hex int" => [ + 'HTTP_X_OC_MTIME' => "0x45adf", + 'expected result' => null + ], + "string that looks like invalid hex int" => [ + 'HTTP_X_OC_MTIME' => "0x123g", + 'expected result' => null + ], + "negative int" => [ + 'HTTP_X_OC_MTIME' => -34, + 'expected result' => -34 + ], + "negative float" => [ + 'HTTP_X_OC_MTIME' => -34.43, + 'expected result' => -34 + ], + ]; + } + + /** + * Test putting a file with string Mtime + * @dataProvider legalMtimeProvider + */ + public function testPutSingleFileLegalMtime($requestMtime, $resultMtime) { + $request = new \OC\AppFramework\Http\Request([ + 'server' => [ + 'HTTP_X_OC_MTIME' => $requestMtime, + ] + ], null, $this->config, null); + $file = 'foo.txt'; + + if ($resultMtime === null) { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage("X-OC-MTime header must be an integer (unix timestamp)."); + } + + $this->doPut($file, null, $request); + + if ($resultMtime !== null) { + $this->assertEquals($resultMtime, $this->getFileInfos($file)['mtime']); + } + } + + /** + * Test putting a file with string Mtime using chunking + * @dataProvider legalMtimeProvider + */ + public function testChunkedPutLegalMtime($requestMtime, $resultMtime) { + $request = new \OC\AppFramework\Http\Request([ + 'server' => [ + 'HTTP_X_OC_MTIME' => $requestMtime, + ] + ], null, $this->config, null); + + $_SERVER['HTTP_OC_CHUNKED'] = true; + $file = 'foo.txt'; + + if ($resultMtime === null) { + $this->expectException(\Sabre\DAV\Exception::class); + $this->expectExceptionMessage("X-OC-MTime header must be an integer (unix timestamp)."); + } + + $this->doPut($file.'-chunking-12345-2-0', null, $request); + $this->doPut($file.'-chunking-12345-2-1', null, $request); + + if ($resultMtime !== null) { + $this->assertEquals($resultMtime, $this->getFileInfos($file)['mtime']); + } + } + /** * Test putting a file using chunking */ @@ -967,6 +1082,25 @@ class FileTest extends \Test\TestCase { return $files; } + /** + * returns an array of file information filesize, mtime, filetype, mimetype + * + * @param string $path + * @param View $userView + * @return array + */ + private function getFileInfos($path = '', View $userView = null) { + if ($userView === null) { + $userView = Filesystem::getView(); + } + return [ + "filesize" => $userView->filesize($path), + "mtime" => $userView->filemtime($path), + "filetype" => $userView->filetype($path), + "mimetype" => $userView->getMimeType($path) + ]; + } + /** * @expectedException \Sabre\DAV\Exception\ServiceUnavailable */