Merge pull request #49352 from nextcloud/s3-disable-multipart

improve handling of large single-part s3 uploads

2 files changed, 33 insertions, 11 deletions

diff --git a/apps/dav/lib/Upload/AssemblyStream.php b/apps/dav/lib/Upload/AssemblyStream.php
index 736905d01c2..642a8604b17 100644
--- a/apps/dav/lib/Upload/AssemblyStream.php
+++ b/apps/dav/lib/Upload/AssemblyStream.php
@@ -75,6 +75,10 @@ class AssemblyStream implements \Icewind\Streams\File {
 			$offset = $this->size + $offset;
 		}
 
+		if ($offset === $this->pos) {
+			return true;
+		}
+
 		if ($offset > $this->size) {
 			return false;
 		}
@@ -95,7 +99,7 @@ class AssemblyStream implements \Icewind\Streams\File {
 
 		$stream = $this->getStream($this->nodes[$nodeIndex]);
 		$nodeOffset = $offset - $nodeStart;
-		if (fseek($stream, $nodeOffset) === -1) {
+		if ($nodeOffset > 0 && fseek($stream, $nodeOffset) === -1) {
 			return false;
 		}
 		$this->currentNode = $nodeIndex;
@@ -126,9 +130,14 @@ class AssemblyStream implements \Icewind\Streams\File {
 			}
 		}
 
-		do {
+		$collectedData = '';
+		// read data until we either got all the data requested or there is no more stream left
+		while ($count > 0 && !is_null($this->currentStream)) {
 			$data = fread($this->currentStream, $count);
 			$read = strlen($data);
+
+			$count -= $read;
+			$collectedData .= $data;
 			$this->currentNodeRead += $read;
 
 			if (feof($this->currentStream)) {
@@ -145,14 +154,11 @@ class AssemblyStream implements \Icewind\Streams\File {
 					$this->currentStream = null;
 				}
 			}
-			// if no data read, try again with the next node because
-			// returning empty data can make the caller think there is no more
-			// data left to read
-		} while ($read === 0 && !is_null($this->currentStream));
+		}
 
 		// update position
-		$this->pos += $read;
-		return $data;
+		$this->pos += strlen($collectedData);
+		return $collectedData;
 	}
 
 	/**
diff --git a/apps/dav/tests/unit/Upload/AssemblyStreamTest.php b/apps/dav/tests/unit/Upload/AssemblyStreamTest.php
index 51c53fb262f..854d02bfc10 100644
--- a/apps/dav/tests/unit/Upload/AssemblyStreamTest.php
+++ b/apps/dav/tests/unit/Upload/AssemblyStreamTest.php
@@ -25,12 +25,16 @@ class AssemblyStreamTest extends \Test\TestCase {
 	/**
 	 * @dataProvider providesNodes()
 	 */
-	public function testGetContentsFread($expected, $nodes): void {
+	public function testGetContentsFread($expected, $nodes, $chunkLength = 3): void {
 		$stream = AssemblyStream::wrap($nodes);
 
 		$content = '';
 		while (!feof($stream)) {
-			$content .= fread($stream, 3);
+			$chunk = fread($stream, $chunkLength);
+			$content .= $chunk;
+			if ($chunkLength !== 3) {
+				$this->assertEquals($chunkLength, strlen($chunk));
+			}
 		}
 
 		$this->assertEquals($expected, $content);
@@ -103,7 +107,19 @@ class AssemblyStreamTest extends \Test\TestCase {
 				]],
 			'a ton of nodes' => [
 				$tonofdata, $tonofnodes
-			]
+			],
+			'one read over multiple nodes' => [
+				'1234567890', [
+					$this->buildNode('0', '1234'),
+					$this->buildNode('1', '5678'),
+					$this->buildNode('2', '90'),
+				], 10],
+			'two reads over multiple nodes' => [
+				'1234567890', [
+					$this->buildNode('0', '1234'),
+					$this->buildNode('1', '5678'),
+					$this->buildNode('2', '90'),
+				], 5],
 		];
 	}