Merge pull request #29391 from nextcloud/backport/29220/stable22

[stable22] s3 external storage fixes

7 files changed, 371 insertions, 146 deletions

diff --git a/.github/workflows/s3-external.yml b/.github/workflows/s3-external.yml
new file mode 100644
index 00000000000..497de81ae2f
--- /dev/null
+++ b/.github/workflows/s3-external.yml
@@ -0,0 +1,124 @@
+name: S3 External storage
+on:
+  push:
+    branches:
+      - master
+      - stable*
+    paths:
+      - 'apps/files_external/**'
+  pull_request:
+    paths:
+      - 'apps/files_external/**'
+
+env:
+  APP_NAME: files_external
+
+jobs:
+  s3-external-tests-minio:
+    runs-on: ubuntu-latest
+
+    strategy:
+      # do not stop on another job's failure
+      fail-fast: false
+      matrix:
+        php-versions: ['7.4', '8.0']
+
+    name: php${{ matrix.php-versions }}-minio
+
+    services:
+      minio:
+        env:
+          MINIO_ACCESS_KEY: minio
+          MINIO_SECRET_KEY: minio123
+        image: bitnami/minio:2021.10.6
+        ports:
+          - "9000:9000"
+
+    steps:
+      - name: Checkout server
+        uses: actions/checkout@v2
+        with:
+          submodules: true
+
+      - name: Set up php ${{ matrix.php-versions }}
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php-versions }}
+          tools: phpunit
+          extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, zip, gd
+
+      - name: Set up Nextcloud
+        run: |
+          mkdir data
+          ./occ maintenance:install --verbose --database=sqlite --database-name=nextcloud --database-host=127.0.0.1 --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password
+          ./occ app:enable --force ${{ env.APP_NAME }}
+          php -S localhost:8080 &
+      - name: PHPUnit
+        run: |
+          echo "<?php return ['run' => true,'hostname' => 'localhost','key' => 'minio','secret' => 'minio123', 'bucket' => 'bucket', 'port' => 9000, 'use_ssl' => false, 'autocreate' => true, 'use_path_style' => true];" > apps/${{ env.APP_NAME }}/tests/config.amazons3.php
+          phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/Amazons3Test.php
+          phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/VersionedAmazonS3Test.php
+      - name: S3 logs
+        if: always()
+        run: |
+          docker ps -a
+          docker logs $(docker ps -aq)
+  s3-external-tests-localstack:
+    runs-on: ubuntu-latest
+
+    strategy:
+      # do not stop on another job's failure
+      fail-fast: false
+      matrix:
+        php-versions: ['7.4', '8.0']
+
+    name: php${{ matrix.php-versions }}-localstack
+
+    services:
+      minio:
+        env:
+          SERVICES: s3
+          DEBUG: 1
+        image: localstack/localstack:0.12.7
+        ports:
+          - "4566:4566"
+
+    steps:
+      - name: Checkout server
+        uses: actions/checkout@v2
+        with:
+          submodules: true
+
+      - name: Set up php ${{ matrix.php-versions }}
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php-versions }}
+          tools: phpunit
+          extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, zip, gd
+
+      - name: Set up Nextcloud
+        run: |
+          mkdir data
+          ./occ maintenance:install --verbose --database=sqlite --database-name=nextcloud --database-host=127.0.0.1 --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password
+          ./occ app:enable --force ${{ env.APP_NAME }}
+          php -S localhost:8080 &
+      - name: PHPUnit
+        run: |
+          echo "<?php return ['run' => true,'hostname' => 'localhost','key' => 'ignored','secret' => 'ignored', 'bucket' => 'bucket', 'port' => 4566, 'use_ssl' => false, 'autocreate' => true, 'use_path_style' => true];" > apps/${{ env.APP_NAME }}/tests/config.amazons3.php
+          phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/Amazons3Test.php
+          phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/VersionedAmazonS3Test.php
+      - name: S3 logs
+        if: always()
+        run: |
+          docker ps -a
+          docker logs $(docker ps -aq)
+
+  s3-external-summary:
+    runs-on: ubuntu-latest
+    needs: [s3-external-tests-minio, s3-external-tests-localstack]
+
+    if: always()
+
+    steps:
+      - name: Summary status
+        run: if ${{ needs.s3-external-tests-minio.result != 'success' }} || ${{ needs.s3-external-tests-localstack.result != 'success' }}; then exit 1; fi
diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php
index 1bdd11e39bd..827fd63d1d6 100644
--- a/apps/files_external/lib/Lib/Storage/AmazonS3.php
+++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php
@@ -49,7 +49,10 @@ use OC\Files\Cache\CacheEntry;
 use OC\Files\ObjectStore\S3ConnectionTrait;
 use OC\Files\ObjectStore\S3ObjectTrait;
 use OCP\Constants;
+use OCP\Files\FileInfo;
 use OCP\Files\IMimeTypeDetector;
+use OCP\ICacheFactory;
+use OCP\IMemcache;
 
 class AmazonS3 extends \OC\Files\Storage\Common {
 	use S3ConnectionTrait;
@@ -71,6 +74,12 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 	/** @var IMimeTypeDetector */
 	private $mimeDetector;
 
+	/** @var bool|null */
+	private $versioningEnabled = null;
+
+	/** @var IMemcache */
+	private $memCache;
+
 	public function __construct($parameters) {
 		parent::__construct($parameters);
 		$this->parseParams($parameters);
@@ -78,6 +87,9 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 		$this->directoryCache = new CappedMemoryCache();
 		$this->filesCache = new CappedMemoryCache();
 		$this->mimeDetector = \OC::$server->get(IMimeTypeDetector::class);
+		/** @var ICacheFactory $cacheFactory */
+		$cacheFactory = \OC::$server->get(ICacheFactory::class);
+		$this->memCache = $cacheFactory->createLocal('s3-external');
 	}
 
 	/**
@@ -120,12 +132,20 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 				unset($this->objectCache[$existingKey]);
 			}
 		}
-		unset($this->directoryCache[$key], $this->filesCache[$key]);
+		unset($this->filesCache[$key]);
+		$keys = array_keys($this->directoryCache->getData());
+		$keyLength = strlen($key);
+		foreach ($keys as $existingKey) {
+			if (substr($existingKey, 0, $keyLength) === $key) {
+				unset($this->directoryCache[$existingKey]);
+			}
+		}
+		unset($this->directoryCache[$key]);
 	}
 
 	/**
 	 * @param $key
-	 * @return Result|boolean
+	 * @return array|false
 	 */
 	private function headObject($key) {
 		if (!isset($this->objectCache[$key])) {
@@ -133,7 +153,7 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 				$this->objectCache[$key] = $this->getConnection()->headObject([
 					'Bucket' => $this->bucket,
 					'Key' => $key
-				]);
+				])->toArray();
 			} catch (S3Exception $e) {
 				if ($e->getStatusCode() >= 500) {
 					throw $e;
@@ -142,6 +162,9 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 			}
 		}
 
+		if (is_array($this->objectCache[$key]) && !isset($this->objectCache[$key]["Key"])) {
+			$this->objectCache[$key]["Key"] = $key;
+		}
 		return $this->objectCache[$key];
 	}
 
@@ -159,63 +182,45 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 	 * @throws \Exception
 	 */
 	private function doesDirectoryExist($path) {
-		if (!isset($this->directoryCache[$path])) {
+		if ($path === '.' || $path === '') {
+			return true;
+		}
+		$path = rtrim($path, '/') . '/';
+
+		if (isset($this->directoryCache[$path])) {
+			return $this->directoryCache[$path];
+		}
+		try {
 			// Maybe this isn't an actual key, but a prefix.
 			// Do a prefix listing of objects to determine.
-			try {
-				$result = $this->getConnection()->listObjects([
-					'Bucket' => $this->bucket,
-					'Prefix' => rtrim($path, '/'),
-					'MaxKeys' => 1,
-					'Delimiter' => '/',
-				]);
+			$result = $this->getConnection()->listObjectsV2([
+				'Bucket' => $this->bucket,
+				'Prefix' => $path,
+				'MaxKeys' => 1,
+			]);
 
-				if ((isset($result['Contents'][0]['Key']) && $result['Contents'][0]['Key'] === rtrim($path, '/') . '/')
-					 || isset($result['CommonPrefixes'])) {
-					$this->directoryCache[$path] = true;
-				} else {
-					$this->directoryCache[$path] = false;
-				}
-			} catch (S3Exception $e) {
-				if ($e->getStatusCode() === 403) {
-					$this->directoryCache[$path] = false;
-				}
-				throw $e;
+			if (isset($result['Contents'])) {
+				$this->directoryCache[$path] = true;
+				return true;
 			}
-		}
 
-		return $this->directoryCache[$path];
-	}
+			// empty directories have their own object
+			$object = $this->headObject($path);
 
-	/**
-	 * Updates old storage ids (v0.2.1 and older) that are based on key and secret to new ones based on the bucket name.
-	 * TODO Do this in a repair step. requires iterating over all users and loading the mount.json from their home
-	 *
-	 * @param array $params
-	 */
-	public function updateLegacyId(array $params) {
-		$oldId = 'amazon::' . $params['key'] . md5($params['secret']);
-
-		// find by old id or bucket
-		$stmt = \OC::$server->getDatabaseConnection()->prepare(
-			'SELECT `numeric_id`, `id` FROM `*PREFIX*storages` WHERE `id` IN (?, ?)'
-		);
-		$stmt->execute([$oldId, $this->id]);
-		while ($row = $stmt->fetch()) {
-			$storages[$row['id']] = $row['numeric_id'];
+			if ($object) {
+				$this->directoryCache[$path] = true;
+				return true;
+			}
+		} catch (S3Exception $e) {
+			if ($e->getStatusCode() >= 400 && $e->getStatusCode() < 500) {
+				$this->directoryCache[$path] = false;
+			}
+			throw $e;
 		}
 
-		if (isset($storages[$this->id]) && isset($storages[$oldId])) {
-			// if both ids exist, delete the old storage and corresponding filecache entries
-			\OC\Files\Cache\Storage::remove($oldId);
-		} elseif (isset($storages[$oldId])) {
-			// if only the old id exists do an update
-			$stmt = \OC::$server->getDatabaseConnection()->prepare(
-				'UPDATE `*PREFIX*storages` SET `id` = ? WHERE `id` = ?'
-			);
-			$stmt->execute([$this->id, $oldId]);
-		}
-		// only the bucket based id may exist, do nothing
+
+		$this->directoryCache[$path] = false;
+		return false;
 	}
 
 	/**
@@ -248,7 +253,7 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 				'Bucket' => $this->bucket,
 				'Key' => $path . '/',
 				'Body' => '',
-				'ContentType' => 'httpd/unix-directory'
+				'ContentType' => FileInfo::MIMETYPE_FOLDER
 			]);
 			$this->testTimeout();
 		} catch (S3Exception $e) {
@@ -284,7 +289,9 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 	protected function clearBucket() {
 		$this->clearCache();
 		try {
-			$this->getConnection()->clearBucket($this->bucket);
+			$this->getConnection()->clearBucket([
+				"Bucket" => $this->bucket
+			]);
 			return true;
 			// clearBucket() is not working with Ceph, so if it fails we try the slower approach
 		} catch (\Exception $e) {
@@ -318,7 +325,9 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 				}
 				// we reached the end when the list is no longer truncated
 			} while ($objects['IsTruncated']);
-			$this->deleteObject($path);
+			if ($path !== '' && $path !== null) {
+				$this->deleteObject($path);
+			}
 		} catch (S3Exception $e) {
 			\OC::$server->getLogger()->logException($e, ['app' => 'files_external']);
 			return false;
@@ -327,54 +336,12 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 	}
 
 	public function opendir($path) {
-		$path = $this->normalizePath($path);
-
-		if ($this->isRoot($path)) {
-			$path = '';
-		} else {
-			$path .= '/';
-		}
-
 		try {
-			$files = [];
-			$results = $this->getConnection()->getPaginator('ListObjects', [
-				'Bucket' => $this->bucket,
-				'Delimiter' => '/',
-				'Prefix' => $path,
-			]);
-
-			foreach ($results as $result) {
-				// sub folders
-				if (is_array($result['CommonPrefixes'])) {
-					foreach ($result['CommonPrefixes'] as $prefix) {
-						$directoryName = trim($prefix['Prefix'], '/');
-						$files[] = substr($directoryName, strlen($path));
-						$this->directoryCache[$directoryName] = true;
-					}
-				}
-				if (is_array($result['Contents'])) {
-					foreach ($result['Contents'] as $object) {
-						if (isset($object['Key']) && $object['Key'] === $path) {
-							// it's the directory itself, skip
-							continue;
-						}
-						$file = basename(
-							isset($object['Key']) ? $object['Key'] : $object['Prefix']
-						);
-						$files[] = $file;
-
-						// store this information for later usage
-						$this->filesCache[$path . $file] = [
-							'ContentLength' => $object['Size'],
-							'LastModified' => (string)$object['LastModified'],
-						];
-					}
-				}
-			}
-
-			return IteratorDirectory::wrap($files);
+			$content = iterator_to_array($this->getDirectoryContent($path));
+			return IteratorDirectory::wrap(array_map(function (array $item) {
+				return $item['name'];
+			}, $content));
 		} catch (S3Exception $e) {
-			\OC::$server->getLogger()->logException($e, ['app' => 'files_external']);
 			return false;
 		}
 	}
@@ -382,33 +349,18 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 	public function stat($path) {
 		$path = $this->normalizePath($path);
 
-		try {
-			$stat = [];
-			if ($this->is_dir($path)) {
-				$cacheEntry = $this->getCache()->get($path);
-				if ($cacheEntry instanceof CacheEntry) {
-					$stat['size'] = $cacheEntry->getSize();
-					$stat['mtime'] = $cacheEntry->getMTime();
-				} else {
-					// Use dummy values
-					$stat['size'] = -1; // Pending
-					$stat['mtime'] = time();
-				}
-			} else {
-				$stat['size'] = $this->getContentLength($path);
-				$stat['mtime'] = strtotime($this->getLastModified($path));
+		if ($this->is_dir($path)) {
+			$stat = $this->getDirectoryMetaData($path);
+		} else {
+			$object = $this->headObject($path);
+			if ($object === false) {
+				return false;
 			}
-			$stat['atime'] = time();
-
-			return $stat;
-		} catch (S3Exception $e) {
-			\OC::$server->getLogger()->logException($e, ['app' => 'files_external']);
-			return false;
+			$stat = $this->objectToMetaData($object);
 		}
-	}
+		$stat['atime'] = time();
 
-	public function hasUpdated($path, $time) {
-		return $this->getMountOption('filesystem_check_changes', 1) === 1 || parent::hasUpdated($path, $time);
+		return $stat;
 	}
 
 	/**
@@ -463,7 +415,7 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 		}
 
 		try {
-			return $this->isRoot($path) || $this->doesDirectoryExist($path);
+			return $this->doesDirectoryExist($path);
 		} catch (S3Exception $e) {
 			\OC::$server->getLogger()->logException($e, ['app' => 'files_external']);
 			return false;
@@ -478,6 +430,9 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 		}
 
 		try {
+			if (isset($this->directoryCache[$path]) && $this->directoryCache[$path]) {
+				return 'dir';
+			}
 			if (isset($this->filesCache[$path]) || $this->headObject($path)) {
 				return 'file';
 			}
@@ -603,11 +558,11 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 		return true;
 	}
 
-	public function copy($path1, $path2) {
+	public function copy($path1, $path2, $isFile = null) {
 		$path1 = $this->normalizePath($path1);
 		$path2 = $this->normalizePath($path2);
 
-		if ($this->is_file($path1)) {
+		if ($isFile === true || $this->is_file($path1)) {
 			try {
 				$this->getConnection()->copyObject([
 					'Bucket' => $this->bucket,
@@ -623,28 +578,17 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 			$this->remove($path2);
 
 			try {
-				$this->getConnection()->copyObject([
-					'Bucket' => $this->bucket,
-					'Key' => $path2 . '/',
-					'CopySource' => S3Client::encodeKey($this->bucket . '/' . $path1 . '/')
-				]);
+				$this->mkdir($path2);
 				$this->testTimeout();
 			} catch (S3Exception $e) {
 				\OC::$server->getLogger()->logException($e, ['app' => 'files_external']);
 				return false;
 			}
 
-			$dh = $this->opendir($path1);
-			if (is_resource($dh)) {
-				while (($file = readdir($dh)) !== false) {
-					if (\OC\Files\Filesystem::isIgnoredDir($file)) {
-						continue;
-					}
-
-					$source = $path1 . '/' . $file;
-					$target = $path2 . '/' . $file;
-					$this->copy($source, $target);
-				}
+			foreach ($this->getDirectoryContent($path1) as $item) {
+				$source = $path1 . '/' . $item['name'];
+				$target = $path2 . '/' . $item['name'];
+				$this->copy($source, $target, $item['mimetype'] !== FileInfo::MIMETYPE_FOLDER);
 			}
 		}
 
@@ -711,4 +655,102 @@ class AmazonS3 extends \OC\Files\Storage\Common {
 	public static function checkDependencies() {
 		return true;
 	}
+
+	public function getDirectoryContent($directory): \Traversable {
+		$path = $this->normalizePath($directory);
+
+		if ($this->isRoot($path)) {
+			$path = '';
+		} else {
+			$path .= '/';
+		}
+
+		$results = $this->getConnection()->getPaginator('ListObjectsV2', [
+			'Bucket' => $this->bucket,
+			'Delimiter' => '/',
+			'Prefix' => $path,
+		]);
+
+		foreach ($results as $result) {
+			// sub folders
+			if (is_array($result['CommonPrefixes'])) {
+				foreach ($result['CommonPrefixes'] as $prefix) {
+					$dir = $this->getDirectoryMetaData($prefix['Prefix']);
+					if ($dir) {
+						yield $dir;
+					}
+				}
+			}
+			if (is_array($result['Contents'])) {
+				foreach ($result['Contents'] as $object) {
+					$this->objectCache[$object['Key']] = $object;
+					if ($object['Key'] !== $path) {
+						yield $this->objectToMetaData($object);
+					}
+				}
+			}
+		}
+	}
+
+	private function objectToMetaData(array $object): array {
+		return [
+			'name' => basename($object['Key']),
+			'mimetype' => $this->mimeDetector->detectPath($object['Key']),
+			'mtime' => strtotime($object['LastModified']),
+			'storage_mtime' => strtotime($object['LastModified']),
+			'etag' => $object['ETag'],
+			'permissions' => Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE,
+			'size' => (int)($object['Size'] ?? $object['ContentLength']),
+		];
+	}
+
+	private function getDirectoryMetaData(string $path): ?array {
+		$path = trim($path, '/');
+		// when versioning is enabled, delete markers are returned as part of CommonPrefixes
+		// resulting in "ghost" folders, verify that each folder actually exists
+		if ($this->versioningEnabled() && !$this->doesDirectoryExist($path)) {
+			return null;
+		}
+		$cacheEntry = $this->getCache()->get($path);
+		if ($cacheEntry instanceof CacheEntry) {
+			return $cacheEntry->getData();
+		} else {
+			return [
+				'name' => basename($path),
+				'mimetype' => FileInfo::MIMETYPE_FOLDER,
+				'mtime' => time(),
+				'storage_mtime' => time(),
+				'etag' => uniqid(),
+				'permissions' => Constants::PERMISSION_ALL,
+				'size' => -1,
+			];
+		}
+	}
+
+	public function versioningEnabled(): bool {
+		if ($this->versioningEnabled === null) {
+			$cached = $this->memCache->get('versioning-enabled::' . $this->getBucket());
+			if ($cached === null) {
+				$result = $this->getConnection()->getBucketVersioning(['Bucket' => $this->getBucket()]);
+				$this->versioningEnabled = $result->get('Status') === 'Enabled';
+				$this->memCache->set('versioning-enabled::' . $this->getBucket(), $this->versioningEnabled, 60);
+			} else {
+				$this->versioningEnabled = $cached;
+			}
+		}
+		return $this->versioningEnabled;
+	}
+
+	public function hasUpdated($path, $time) {
+		// for files we can get the proper mtime
+		if ($path !== '' && $object = $this->headObject($path)) {
+			$stat = $this->objectToMetaData($object);
+			return $stat['mtime'] > $time;
+		} else {
+			// for directories, the only real option we have is to do a prefix listing and iterate over all objects
+			// however, since this is just as expensive as just re-scanning the directory, we can simply return true
+			// and have the scanner figure out if anything has actually changed
+			return true;
+		}
+	}
 }
diff --git a/apps/files_external/tests/Storage/Amazons3Test.php b/apps/files_external/tests/Storage/Amazons3Test.php
index c013d304cce..d231539fb54 100644
--- a/apps/files_external/tests/Storage/Amazons3Test.php
+++ b/apps/files_external/tests/Storage/Amazons3Test.php
@@ -38,6 +38,8 @@ use OCA\Files_External\Lib\Storage\AmazonS3;
  */
 class Amazons3Test extends \Test\Files\Storage\Storage {
 	private $config;
+	/** @var AmazonS3 */
+	protected $instance;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -60,4 +62,8 @@ class Amazons3Test extends \Test\Files\Storage\Storage {
 	public function testStat() {
 		$this->markTestSkipped('S3 doesn\'t update the parents folder mtime');
 	}
+
+	public function testHashInFileName() {
+		$this->markTestSkipped('Localstack has a bug with hashes in filename');
+	}
 }
diff --git a/apps/files_external/tests/Storage/VersionedAmazonS3Test.php b/apps/files_external/tests/Storage/VersionedAmazonS3Test.php
new file mode 100644
index 00000000000..a16a9944d57
--- /dev/null
+++ b/apps/files_external/tests/Storage/VersionedAmazonS3Test.php
@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2021 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Files_External\Tests\Storage;
+
+/**
+ * @group DB
+ */
+class VersionedAmazonS3Test extends Amazons3Test {
+	protected function setUp(): void {
+		parent::setUp();
+		try {
+			$this->instance->getConnection()->putBucketVersioning([
+				'Bucket' => $this->instance->getBucket(),
+				'VersioningConfiguration' => [
+					'Status' => 'Enabled',
+				],
+			]);
+		} catch (\Exception $e) {
+			$this->markTestSkipped("s3 backend doesn't seem to support versioning");
+		}
+	}
+}
diff --git a/lib/private/Files/Cache/Watcher.php b/lib/private/Files/Cache/Watcher.php
index 1c1ce6d777b..acc76f263dc 100644
--- a/lib/private/Files/Cache/Watcher.php
+++ b/lib/private/Files/Cache/Watcher.php
@@ -88,7 +88,14 @@ class Watcher implements IWatcher {
 		}
 		if ($cachedEntry === false || $this->needsUpdate($path, $cachedEntry)) {
 			$this->update($path, $cachedEntry);
-			return true;
+
+			if ($cachedEntry === false) {
+				return true;
+			} else {
+				// storage backends can sometimes return false positives, only return true if the scanner actually found a change
+				$newEntry = $this->cache->get($path);
+				return $newEntry->getStorageMTime() > $cachedEntry->getStorageMTime();
+			}
 		} else {
 			return false;
 		}
diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php
index bb71306c17d..5fd3b7727ae 100644
--- a/lib/private/Files/ObjectStore/S3ObjectTrait.php
+++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php
@@ -64,7 +64,7 @@ trait S3ObjectTrait {
 			}
 			$opts = [
 				'http' => [
-					'protocol_version' => 1.1,
+					'protocol_version' => $request->getProtocolVersion(),
 					'header' => $headers,
 				],
 			];
diff --git a/tests/lib/Files/Storage/Storage.php b/tests/lib/Files/Storage/Storage.php
index 9fae1a8484a..c4248b7e0da 100644
--- a/tests/lib/Files/Storage/Storage.php
+++ b/tests/lib/Files/Storage/Storage.php
@@ -498,6 +498,9 @@ abstract class Storage extends \Test\TestCase {
 		$this->assertTrue($this->instance->file_exists('target/subfolder'));
 		$this->assertTrue($this->instance->file_exists('target/subfolder/test.txt'));
 
+		$contents = iterator_to_array($this->instance->getDirectoryContent(''));
+		$this->assertCount(1, $contents);
+
 		$this->assertEquals('foo', $this->instance->file_get_contents('target/test1.txt'));
 		$this->assertEquals('qwerty', $this->instance->file_get_contents('target/test2.txt'));
 		$this->assertEquals('bar', $this->instance->file_get_contents('target/subfolder/test.txt'));