Merge pull request #7313 from nextcloud/ensure-that-x-oc-mtime-header-is-an-integer-with-chunked-uploads

Ensure that X-OC-MTime header is an integer with chunked uploads

1 files changed, 136 insertions, 2 deletions

diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php
index 4d106842cf0..1db9b7948e3 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
 	 */
@@ -968,6 +1083,25 @@ class FileTest extends \Test\TestCase {
 	}
 
 	/**
+	 * 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
 	 */
 	public function testGetFopenFails() {