add basic tests for s3 seeking and add some error handling if reopen return the wrong range

Signed-off-by: Robin Appelman <robin@icewind.nl>

3 files changed, 38 insertions, 12 deletions

diff --git a/lib/private/Files/Stream/SeekableHttpStream.php b/lib/private/Files/Stream/SeekableHttpStream.php
index 80f05f7ea09..8fe54839e25 100644
--- a/lib/private/Files/Stream/SeekableHttpStream.php
+++ b/lib/private/Files/Stream/SeekableHttpStream.php
@@ -76,7 +76,8 @@ class SeekableHttpStream implements File {
 	/** @var int */
 	private $offset = 0;
 
-	private function reconnect($range) {
+	private function reconnect(int $start) {
+		$range = $start . '-';
 		if ($this->current != null) {
 			fclose($this->current);
 		}
@@ -88,14 +89,23 @@ class SeekableHttpStream implements File {
 		}
 
 		$responseHead = stream_get_meta_data($this->current)['wrapper_data'];
-		$contentRange = array_values(array_filter($responseHead, function ($v) {
+		$rangeHeaders = array_values(array_filter($responseHead, function ($v) {
 			return preg_match('#^content-range:#i', $v) === 1;
-		}))[0];
+		}));
+		if (!$rangeHeaders) {
+			return false;
+		}
+		$contentRange = $rangeHeaders[0];
 
 		$content = trim(explode(':', $contentRange)[1]);
 		$range = trim(explode(' ', $content)[1]);
-		$begin = explode('-', $range)[0];
-		$this->offset = intval($begin);
+		$begin = intval(explode('-', $range)[0]);
+
+		if ($begin !== $start) {
+			return false;
+		}
+
+		$this->offset = $begin;
 
 		return true;
 	}
@@ -104,7 +114,7 @@ class SeekableHttpStream implements File {
 		$options = stream_context_get_options($this->context)[self::PROTOCOL];
 		$this->openCallback = $options['callback'];
 
-		return $this->reconnect('0-');
+		return $this->reconnect(0);
 	}
 
 	function stream_read($count) {
@@ -122,12 +132,12 @@ class SeekableHttpStream implements File {
 				if ($offset === $this->offset) {
 					return true;
 				}
-				return $this->reconnect($offset . '-');
+				return $this->reconnect($offset);
 			case SEEK_CUR:
 				if ($offset === 0) {
 					return true;
 				}
-				return $this->reconnect(($this->offset + $offset) . '-');
+				return $this->reconnect($this->offset + $offset);
 			case SEEK_END:
 				return false;
 		}
diff --git a/tests/lib/Files/ObjectStore/ObjectStoreTest.php b/tests/lib/Files/ObjectStore/ObjectStoreTest.php
index 1383c0149a2..67c41eb7ccc 100644
--- a/tests/lib/Files/ObjectStore/ObjectStoreTest.php
+++ b/tests/lib/Files/ObjectStore/ObjectStoreTest.php
@@ -31,7 +31,7 @@ abstract class ObjectStoreTest extends TestCase {
 	 */
 	abstract protected function getInstance();
 
-	private function stringToStream($data) {
+	protected function stringToStream($data) {
 		$stream = fopen('php://temp', 'w+');
 		fwrite($stream, $data);
 		rewind($stream);
diff --git a/tests/lib/Files/ObjectStore/S3Test.php b/tests/lib/Files/ObjectStore/S3Test.php
index b56978f4fee..525e020445a 100644
--- a/tests/lib/Files/ObjectStore/S3Test.php
+++ b/tests/lib/Files/ObjectStore/S3Test.php
@@ -27,7 +27,7 @@ use OC\Files\ObjectStore\S3;
 class MultiPartUploadS3 extends S3 {
 	function writeObject($urn, $stream) {
 		$this->getConnection()->upload($this->bucket, $urn, $stream, 'private', [
-			'mup_threshold' => 1
+			'mup_threshold' => 1,
 		]);
 	}
 }
@@ -36,8 +36,8 @@ class NonSeekableStream extends Wrapper {
 	public static function wrap($source) {
 		$context = stream_context_create([
 			'nonseek' => [
-				'source' => $source
-			]
+				'source' => $source,
+			],
 		]);
 		return Wrapper::wrapSource($source, $context, 'nonseek', self::class);
 	}
@@ -83,4 +83,20 @@ class S3Test extends ObjectStoreTest {
 
 		$this->assertEquals(file_get_contents(__FILE__), stream_get_contents($result));
 	}
+
+	public function testSeek() {
+		$data = file_get_contents(__FILE__);
+
+		$instance = $this->getInstance();
+		$instance->writeObject('seek', $this->stringToStream($data));
+
+		$read = $instance->readObject('seek');
+		$this->assertEquals(substr($data, 0, 100), fread($read, 100));
+
+		fseek($read, 10);
+		$this->assertEquals(substr($data, 10, 100), fread($read, 100));
+
+		fseek($read, 100, SEEK_CUR);
+		$this->assertEquals(substr($data, 210, 100), fread($read, 100));
+	}
 }