Merge pull request #16452 from owncloud/enc_ftp_upload

always write file, if fseek doesn't work we write the whole file

3 files changed, 114 insertions, 12 deletions

diff --git a/lib/private/files/stream/encryption.php b/lib/private/files/stream/encryption.php
index f2f5b9c9af7..22d230e7c86 100644
--- a/lib/private/files/stream/encryption.php
+++ b/lib/private/files/stream/encryption.php
@@ -130,6 +130,7 @@ class Encryption extends Wrapper {
 	 * @param int $size
 	 * @param int $unencryptedSize
 	 * @param int $headerSize
+	 * @param string $wrapper stream wrapper class
 	 * @return resource
 	 *
 	 * @throws \BadMethodCallException
@@ -144,7 +145,8 @@ class Encryption extends Wrapper {
 								$mode,
 								$size,
 								$unencryptedSize,
-								$headerSize) {
+								$headerSize,
+								$wrapper =  'OC\Files\Stream\Encryption') {
 
 		$context = stream_context_create(array(
 			'ocencryption' => array(
@@ -164,7 +166,7 @@ class Encryption extends Wrapper {
 			)
 		));
 
-		return self::wrapSource($source, $mode, $context, 'ocencryption', 'OC\Files\Stream\Encryption');
+		return self::wrapSource($source, $mode, $context, 'ocencryption', $wrapper);
 	}
 
 	/**
@@ -271,7 +273,7 @@ class Encryption extends Wrapper {
 
 		$result = '';
 
-//		$count = min($count, $this->unencryptedSize - $this->position);
+		$count = min($count, $this->unencryptedSize - $this->position);
 		while ($count > 0) {
 			$remainingLength = $count;
 			// update the cache of the current block
@@ -309,7 +311,7 @@ class Encryption extends Wrapper {
 			// flush will start writing there when the position moves to another block
 			$positionInFile = (int)floor($this->position / $this->unencryptedBlockSize) *
 				$this->util->getBlockSize() + $this->headerSize;
-			$resultFseek = parent::stream_seek($positionInFile);
+			$resultFseek = $this->parentStreamSeek($positionInFile);
 
 			// only allow writes on seekable streams, or at the end of the encrypted stream
 			if (!($this->readOnly) && ($resultFseek || $positionInFile === $this->size)) {
@@ -376,10 +378,10 @@ class Encryption extends Wrapper {
 			* $this->util->getBlockSize() + $this->headerSize;
 
 		$oldFilePosition = parent::stream_tell();
-		if (parent::stream_seek($newFilePosition)) {
-			parent::stream_seek($oldFilePosition);
+		if ($this->parentStreamSeek($newFilePosition)) {
+			$this->parentStreamSeek($oldFilePosition);
 			$this->flush();
-			parent::stream_seek($newFilePosition);
+			$this->parentStreamSeek($newFilePosition);
 			$this->position = $newPosition;
 			$return = true;
 		}
@@ -410,9 +412,18 @@ class Encryption extends Wrapper {
 			// we are handling that separately here and we don't want to
 			// get into an infinite loop
 			$encrypted = $this->encryptionModule->encrypt($this->cache);
-			parent::stream_write($encrypted);
+			$bytesWritten = parent::stream_write($encrypted);
 			$this->writeFlag = false;
-			$this->size = max($this->size, parent::stream_tell());
+			// Check whether the write concerns the last block
+			// If so then update the encrypted filesize
+			// Note that the unencrypted pointer and filesize are NOT yet updated when flush() is called
+			// We recalculate the encrypted filesize as we do not know the context of calling flush()
+			$completeBlocksInFile=(int)floor($this->unencryptedSize/$this->unencryptedBlockSize);
+			if ($completeBlocksInFile === (int)floor($this->position/$this->unencryptedBlockSize)) {
+				$this->size = $this->util->getBlockSize() * $completeBlocksInFile;
+				$this->size += $bytesWritten;
+				$this->size += $this->headerSize;
+			}
 		}
 		// always empty the cache (otherwise readCache() will not fill it with the new block)
 		$this->cache = '';
@@ -449,4 +460,14 @@ class Encryption extends Wrapper {
 		parent::stream_read($this->headerSize);
 	}
 
+	/**
+	 * call stream_seek() from parent class
+	 *
+	 * @param integer $position
+	 * @return bool
+	 */
+	protected function parentStreamSeek($position) {
+		return parent::stream_seek($position);
+	}
+
 }
diff --git a/tests/lib/files/stream/dummyencryptionwrapper.php b/tests/lib/files/stream/dummyencryptionwrapper.php
new file mode 100644
index 00000000000..bb512d99c66
--- /dev/null
+++ b/tests/lib/files/stream/dummyencryptionwrapper.php
@@ -0,0 +1,37 @@
+<?php
+/**
+ * @author Björn Schießle <schiessle@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+
+namespace Test\Files\Stream;
+
+class DummyEncryptionWrapper extends \OC\Files\Stream\Encryption {
+
+	/**
+	 * simulate a non-seekable stream wrapper by always return false
+	 *
+	 * @param int $position
+	 * @return bool
+	 */
+	protected function parentStreamSeek($position) {
+		return false;
+	}
+
+}
diff --git a/tests/lib/files/stream/encryption.php b/tests/lib/files/stream/encryption.php
index 040be7256a6..281ec0a14a0 100644
--- a/tests/lib/files/stream/encryption.php
+++ b/tests/lib/files/stream/encryption.php
@@ -15,7 +15,7 @@ class Encryption extends \Test\TestCase {
 	 * @param integer $unencryptedSize
 	 * @return resource
 	 */
-	protected function getStream($fileName, $mode, $unencryptedSize) {
+	protected function getStream($fileName, $mode, $unencryptedSize, $wrapper = '\OC\Files\Stream\Encryption') {
 		clearstatcache();
 		$size = filesize($fileName);
 		$source = fopen($fileName, $mode);
@@ -44,9 +44,10 @@ class Encryption extends \Test\TestCase {
 			->method('getUidAndFilename')
 			->willReturn(['user1', $internalPath]);
 
-		return \OC\Files\Stream\Encryption::wrap($source, $internalPath,
+
+		return $wrapper::wrap($source, $internalPath,
 			$fullPath, $header, $uid, $this->encryptionModule, $storage, $encStorage,
-			$util, $file, $mode, $size, $unencryptedSize, 8192);
+			$util, $file, $mode, $size, $unencryptedSize, 8192, $wrapper);
 	}
 
 	/**
@@ -256,6 +257,49 @@ class Encryption extends \Test\TestCase {
 	}
 
 	/**
+	 * simulate a non-seekable storage
+	 *
+	 * @dataProvider dataFilesProvider
+	 */
+	public function testWriteToNonSeekableStorage($testFile) {
+
+		$wrapper = $this->getMockBuilder('\OC\Files\Stream\Encryption')
+			->setMethods(['parentSeekStream'])->getMock();
+		$wrapper->expects($this->any())->method('parentSeekStream')->willReturn(false);
+
+		$expectedData = file_get_contents(\OC::$SERVERROOT . '/tests/data/' . $testFile);
+		// write it
+		$fileName = tempnam("/tmp", "FOO");
+		$stream = $this->getStream($fileName, 'w+', 0, '\Test\Files\Stream\DummyEncryptionWrapper');
+		// while writing the file from the beginning to the end we should never try
+		// to read parts of the file. This should only happen for write operations
+		// in the middle of a file
+		$this->encryptionModule->expects($this->never())->method('decrypt');
+		fwrite($stream, $expectedData);
+		fclose($stream);
+
+		// read it all
+		$stream = $this->getStream($fileName, 'r', strlen($expectedData), '\Test\Files\Stream\DummyEncryptionWrapper');
+		$data = stream_get_contents($stream);
+		fclose($stream);
+
+		$this->assertEquals($expectedData, $data);
+
+		// another read test with a loop like we do in several places:
+		$stream = $this->getStream($fileName, 'r', strlen($expectedData));
+		$data = '';
+		while (!feof($stream)) {
+			$data .= fread($stream, 8192);
+		}
+		fclose($stream);
+
+		$this->assertEquals($expectedData, $data);
+
+		unlink($fileName);
+
+	}
+
+	/**
 	 * @return \PHPUnit_Framework_MockObject_MockObject
 	 */
 	protected function buildMockModule() {