Merge pull request #15774 from owncloud/jknockaert-patch-1

fix encryption header error

lib/private/files/stream/encryption.php

@@ -1,7 +1,7 @@
 <?php
 /**
  * @author Björn Schießle <schiessle@owncloud.com>
- * @author jknockaert <jasper@knockaert.nl>
+ * @author Jasper Knockaert <jasper@knockaert.nl>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
  *
  * @copyright Copyright (c) 2015, ownCloud, Inc.
@@ -221,10 +221,9 @@ class Encryption extends Wrapper {
 			|| $mode === 'w+'
 			|| $mode === 'wb'
 			|| $mode === 'wb+'
+			|| $mode === 'r+'
+			|| $mode === 'rb+'
 		) {
-			// We're writing a new file so start write counter with 0 bytes
-			$this->unencryptedSize = 0;
-			$this->size = 0;
 			$this->readOnly = false;
 		} else {
 			$this->readOnly = true;
@@ -238,6 +237,20 @@ class Encryption extends Wrapper {
 		$accessList = $this->file->getAccessList($sharePath);
 		$this->newHeader = $this->encryptionModule->begin($this->fullPath, $this->uid, $this->header, $accessList);
 
+		if (
+			$mode === 'w'
+			|| $mode === 'w+'
+			|| $mode === 'wb'
+			|| $mode === 'wb+'
+		) {
+			// We're writing a new file so start write counter with 0 bytes
+			$this->unencryptedSize = 0;
+			$this->writeHeader();
+			$this->size = $this->util->getHeaderSize();
+		} else {
+			$this->skipHeader();
+		}
+
 		return true;
 
 	}
@@ -250,11 +263,6 @@ class Encryption extends Wrapper {
 
 		$result = '';
 
-		// skip the header if we read the file from the beginning
-		if ($this->position === 0) {
-			parent::stream_read($this->util->getHeaderSize());
-		}
-
 //		$count = min($count, $this->unencryptedSize - $this->position);
 		while ($count > 0) {
 			$remainingLength = $count;
@@ -281,11 +289,6 @@ class Encryption extends Wrapper {
 
 	public function stream_write($data) {
 
-		if ($this->position === 0) {
-			$this->writeHeader();
-			$this->size = $this->util->getHeaderSize();
-		}
-
 		$length = 0;
 		// loop over $data to fit it in 6126 sized unencrypted blocks
 		while (strlen($data) > 0) {
@@ -428,9 +431,16 @@ class Encryption extends Wrapper {
 	 * @return integer
 	 * @throws EncryptionHeaderKeyExistsException if header key is already in use
 	 */
-	private function writeHeader() {
+	protected function writeHeader() {
 		$header = $this->util->createHeader($this->newHeader, $this->encryptionModule);
 		return parent::stream_write($header);
 	}
 
+	/**
+	 * read first block to skip the header
+	 */
+	protected function skipHeader() {
+		parent::stream_read($this->util->getHeaderSize());
+	}
+
 }

tests/lib/files/stream/encryption.php

@@ -55,6 +55,7 @@ class Encryption extends \Test\TestCase {
 								   $fileExists,
 								   $expectedSharePath,
 								   $expectedSize,
+								   $expectedUnencryptedSize,
 								   $expectedReadOnly) {
 
 		// build mocks
@@ -77,9 +78,15 @@ class Encryption extends \Test\TestCase {
 				return array();
 			}));
 
+		$utilMock = $this->getMockBuilder('\OC\Encryption\Util')
+			->disableOriginalConstructor()->getMock();
+		$utilMock->expects($this->any())
+			->method('getHeaderSize')
+			->willReturn(8192);
+
 		// get a instance of the stream wrapper
 		$streamWrapper = $this->getMockBuilder('\OC\Files\Stream\Encryption')
-			->setMethods(['loadContext'])->disableOriginalConstructor()->getMock();
+			->setMethods(['loadContext', 'writeHeader', 'skipHeader'])->disableOriginalConstructor()->getMock();
 
 		// set internal properties of the stream wrapper
 		$stream = new \ReflectionClass('\OC\Files\Stream\Encryption');
@@ -95,6 +102,10 @@ class Encryption extends \Test\TestCase {
 		$file->setAccessible(true);
 		$file->setValue($streamWrapper, $fileMock);
 		$file->setAccessible(false);
+		$util = $stream->getProperty('util');
+		$util->setAccessible(true);
+		$util->setValue($streamWrapper, $utilMock);
+		$util->setAccessible(false);
 		$fullPathP = $stream->getProperty('fullPath');
 		$fullPathP->setAccessible(true);
 		$fullPathP->setValue($streamWrapper, $fullPath);
@@ -118,7 +129,7 @@ class Encryption extends \Test\TestCase {
 
 		$unencryptedSize = $stream->getProperty('unencryptedSize');
 		$unencryptedSize->setAccessible(true);
-		$this->assertSame($expectedSize,
+		$this->assertSame($expectedUnencryptedSize,
 			$unencryptedSize->getValue($streamWrapper)
 		);
 		$unencryptedSize->setAccessible(false);
@@ -133,9 +144,9 @@ class Encryption extends \Test\TestCase {
 
 	public function dataProviderStreamOpen() {
 		return array(
-			array('r', '/foo/bar/test.txt', true, '/foo/bar/test.txt', null, true),
+			array('r', '/foo/bar/test.txt', true, '/foo/bar/test.txt', null, null, true),
-			array('r', '/foo/bar/test.txt', false, '/foo/bar', null, true),
+			array('r', '/foo/bar/test.txt', false, '/foo/bar', null, null, true),
-			array('w', '/foo/bar/test.txt', true, '/foo/bar/test.txt', 0, false),
+			array('w', '/foo/bar/test.txt', true, '/foo/bar/test.txt', 8192, 0, false),
 		);
 	}
 
@@ -152,6 +163,36 @@ class Encryption extends \Test\TestCase {
 		unlink($fileName);
 	}
 
+	public function testWriteWriteRead() {
+		$fileName = tempnam("/tmp", "FOO");
+		$stream = $this->getStream($fileName, 'w+', 0);
+		$this->assertEquals(6, fwrite($stream, 'foobar'));
+		fclose($stream);
+
+		$stream = $this->getStream($fileName, 'r+', 6);
+		$this->assertEquals(3, fwrite($stream, 'bar'));
+		fclose($stream);
+
+		$stream = $this->getStream($fileName, 'r', 6);
+		$this->assertEquals('barbar', fread($stream, 100));
+		fclose($stream);
+	}
+
+	public function testRewind() {
+		$fileName = tempnam("/tmp", "FOO");
+		$stream = $this->getStream($fileName, 'w+', 0);
+		$this->assertEquals(6, fwrite($stream, 'foobar'));
+		$this->assertEquals(TRUE, rewind($stream));
+		$this->assertEquals('foobar', fread($stream, 100));
+		$this->assertEquals(TRUE, rewind($stream));
+		$this->assertEquals(3, fwrite($stream, 'bar'));
+		fclose($stream);
+
+		$stream = $this->getStream($fileName, 'r', 6);
+		$this->assertEquals('barbar', fread($stream, 100));
+		fclose($stream);
+	}
+
 	public function testSeek() {
 		$fileName = tempnam("/tmp", "FOO");
 		$stream = $this->getStream($fileName, 'w+', 0);