From 7e55e24e43109f4cf74e7d994878cbe147642f2e Mon Sep 17 00:00:00 2001 From: "Marcelo H. Bartsch" Date: Thu, 25 Jun 2026 08:23:22 +0000 Subject: [PATCH 1/2] Allow the use of 'prefix' in S3 buckets Signed-off-by: Marcelo H. Bartsch --- .../lib/Lib/Backend/AmazonS3.php | 2 + .../lib/Lib/Storage/AmazonS3.php | 59 ++++++++++++------- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/apps/files_external/lib/Lib/Backend/AmazonS3.php b/apps/files_external/lib/Lib/Backend/AmazonS3.php index 06196e8216cb7..d6eca29442fc9 100644 --- a/apps/files_external/lib/Lib/Backend/AmazonS3.php +++ b/apps/files_external/lib/Lib/Backend/AmazonS3.php @@ -27,6 +27,8 @@ public function __construct(IL10N $l, AccessKey $legacyAuth) { ->setText($l->t('S3-Compatible Object Storage')) ->addParameters([ new DefinitionParameter('bucket', $l->t('Bucket')), + (new DefinitionParameter('prefix', $l->t('Object key prefix'))) + ->setFlag(DefinitionParameter::FLAG_OPTIONAL), (new DefinitionParameter('hostname', $l->t('Hostname'))) ->setFlag(DefinitionParameter::FLAG_OPTIONAL), (new DefinitionParameter('port', $l->t('Port'))) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index 31c8f70f3fc54..7c01e98e16c6a 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -43,12 +43,15 @@ class AmazonS3 extends Common { private IMimeTypeDetector $mimeDetector; private ICache $memCache; private ?bool $versioningEnabled = null; + private string $prefix = ''; public function __construct(array $parameters) { parent::__construct($parameters); $this->parseParams($parameters); + $rawPrefix = ltrim(trim($parameters['prefix'] ?? ''), '/'); + $this->prefix = $rawPrefix !== '' ? rtrim($rawPrefix, '/') . '/' : ''; // @todo: using `key` here may be problematic with different authentication methods and/or key rotation... - $this->id = 'amazon::external::' . md5($this->params['hostname'] . ':' . $this->params['bucket'] . ':' . $this->params['key']); + $this->id = 'amazon::external::' . md5($this->params['hostname'] . ':' . $this->params['bucket'] . ':' . $this->params['key'] . ':' . $this->prefix); $this->initCaches(); $this->mimeDetector = Server::get(IMimeTypeDetector::class); /** @var ICacheFactory $cacheFactory */ @@ -78,6 +81,17 @@ private function cleanKey(string $path): string { return $path; } + private function addPrefix(string $path): string { + return $this->prefix . $path; + } + + private function stripPrefix(string $key): string { + if ($this->prefix === '' || !str_starts_with($key, $this->prefix)) { + return $key; + } + return substr($key, strlen($this->prefix)); + } + private function initCaches(): void { $this->objectCache = new CappedMemoryCache(2048); $this->directoryCache = new CappedMemoryCache(8192); @@ -115,7 +129,7 @@ private function headObject(string $key): array|false { try { $this->objectCache[$key] = $this->getConnection()->headObject([ 'Bucket' => $this->bucket, - 'Key' => $key + 'Key' => $this->addPrefix($key) ] + $this->getServerSideEncryptionParameters())->toArray(); } catch (S3Exception $e) { if ($e->getStatusCode() >= 500) { @@ -157,7 +171,7 @@ private function doesDirectoryExist(string $path): bool { // Do a prefix listing of objects to determine. $result = $this->getConnection()->listObjectsV2([ 'Bucket' => $this->bucket, - 'Prefix' => $path, + 'Prefix' => $this->addPrefix($path), 'MaxKeys' => 1, ]); @@ -208,7 +222,7 @@ public function mkdir(string $path): bool { try { $this->getConnection()->putObject([ 'Bucket' => $this->bucket, - 'Key' => $path . '/', + 'Key' => $this->addPrefix($path . '/'), 'Body' => '', 'ContentType' => FileInfo::MIMETYPE_FOLDER ] + $this->getServerSideEncryptionParameters()); @@ -258,7 +272,9 @@ private function batchDelete(?string $path = null): bool { 'Bucket' => $this->bucket ]; if ($path !== null) { - $params['Prefix'] = $path . '/'; + $params['Prefix'] = $this->addPrefix($path . '/'); + } elseif ($this->prefix !== '') { + $params['Prefix'] = $this->prefix; } try { $connection = $this->getConnection(); @@ -283,7 +299,7 @@ private function batchDelete(?string $path = null): bool { // we reached the end when the list is no longer truncated } while ($objects['IsTruncated']); if ($path !== '' && $path !== null) { - $this->deleteObject($path); + $this->deleteObject($this->addPrefix($path)); } } catch (S3Exception $e) { $this->logger->error($e->getMessage(), [ @@ -387,7 +403,7 @@ public function unlink(string $path): bool { } try { - $this->deleteObject($path); + $this->deleteObject($this->addPrefix($path)); $this->invalidateCache($path); } catch (S3Exception $e) { $this->logger->error($e->getMessage(), [ @@ -414,7 +430,7 @@ public function fopen(string $path, string $mode) { } try { - return $this->readObject($path); + return $this->readObject($this->addPrefix($path)); } catch (\Exception $e) { $this->logger->error($e->getMessage(), [ 'app' => 'files_external', @@ -447,7 +463,7 @@ public function fopen(string $path, string $mode) { } $tmpFile = Server::get(ITempManager::class)->getTemporaryFile($ext); if ($this->file_exists($path)) { - $source = $this->readObject($path); + $source = $this->readObject($this->addPrefix($path)); file_put_contents($tmpFile, $source); } @@ -476,7 +492,7 @@ public function touch(string $path, ?int $mtime = null): bool { $mimeType = $this->mimeDetector->detectPath($path); $this->getConnection()->putObject([ 'Bucket' => $this->bucket, - 'Key' => $this->cleanKey($path), + 'Key' => $this->addPrefix($this->cleanKey($path)), 'Metadata' => $metadata, 'Body' => '', 'ContentType' => $mimeType, @@ -502,7 +518,7 @@ public function copy(string $source, string $target, ?bool $isFile = null): bool if ($isFile === true || $this->is_file($source)) { try { - $this->copyObject($source, $target, [ + $this->copyObject($this->addPrefix($source), $this->addPrefix($target), [ 'StorageClass' => $this->storageClass, ]); $this->testTimeout(); @@ -583,7 +599,7 @@ public function getId(): string { public function writeBack(string $tmpFile, string $path): bool { try { $source = fopen($tmpFile, 'r'); - $this->writeObject($path, $source, $this->mimeDetector->detectPath($path)); + $this->writeObject($this->addPrefix($path), $source, $this->mimeDetector->detectPath($path)); $this->invalidateCache($path); unlink($tmpFile); @@ -617,20 +633,21 @@ public function getDirectoryContent(string $directory): \Traversable { $results = $this->getConnection()->getPaginator('ListObjectsV2', [ 'Bucket' => $this->bucket, 'Delimiter' => '/', - 'Prefix' => $path, + 'Prefix' => $this->addPrefix($path), ]); foreach ($results as $result) { // sub folders if (is_array($result['CommonPrefixes'])) { foreach ($result['CommonPrefixes'] as $prefix) { - if (preg_match('/\/{2,}$/', $prefix['Prefix'])) { - $this->logger->warning('Detected a repeating delimiter in prefix \'' . $prefix['Prefix'] + $strippedPrefix = $this->stripPrefix($prefix['Prefix']); + if (preg_match('/\/{2,}$/', $strippedPrefix)) { + $this->logger->warning('Detected a repeating delimiter in prefix \'' . $strippedPrefix . '\'. This is unsupported and its contents have been ignored.'); continue; } - $dir = $this->getDirectoryMetaData($prefix['Prefix']); + $dir = $this->getDirectoryMetaData($strippedPrefix); if ($dir) { yield $dir; } @@ -638,8 +655,10 @@ public function getDirectoryContent(string $directory): \Traversable { } if (is_array($result['Contents'])) { foreach ($result['Contents'] as $object) { - $this->objectCache[$object['Key']] = $object; - if ($object['Key'] !== $path) { + $unprefixedKey = $this->stripPrefix($object['Key']); + $object['Key'] = $unprefixedKey; + $this->objectCache[$unprefixedKey] = $object; + if ($unprefixedKey !== $path) { yield $this->objectToMetaData($object); } } @@ -743,7 +762,7 @@ public function writeStream(string $path, $stream, ?int $size = null): int { } $path = $this->normalizePath($path); - $this->writeObject($path, $stream, $this->mimeDetector->detectPath($path)); + $this->writeObject($this->addPrefix($path), $stream, $this->mimeDetector->detectPath($path)); $this->invalidateCache($path); return $size; @@ -757,7 +776,7 @@ public function getDirectDownload(string $path): array|false { $command = $this->getConnection()->getCommand('GetObject', [ 'Bucket' => $this->bucket, - 'Key' => $path, + 'Key' => $this->addPrefix($path), ]); $expiration = new \DateTimeImmutable('+60 minutes'); From 68f690ba2645f9ccd3255a13afb62196e2b9609b Mon Sep 17 00:00:00 2001 From: "Marcelo H. Bartsch" Date: Thu, 25 Jun 2026 08:51:50 +0000 Subject: [PATCH 2/2] add Tests Signed-off-by: Marcelo H. Bartsch --- .../tests/Storage/AmazonS3PrefixTest.php | 132 ++++++++++++++++++ .../tests/Storage/Amazons3PrefixTest.php | 96 +++++++++++++ 2 files changed, 228 insertions(+) create mode 100644 apps/files_external/tests/Storage/AmazonS3PrefixTest.php create mode 100644 apps/files_external/tests/Storage/Amazons3PrefixTest.php diff --git a/apps/files_external/tests/Storage/AmazonS3PrefixTest.php b/apps/files_external/tests/Storage/AmazonS3PrefixTest.php new file mode 100644 index 0000000000000..68bc69a9cc8d4 --- /dev/null +++ b/apps/files_external/tests/Storage/AmazonS3PrefixTest.php @@ -0,0 +1,132 @@ + 'test-bucket', + 'key' => 'test-key', + 'secret' => 'test-secret', + 'region' => 'us-east-1', + 'hostname' => 's3.us-east-1.amazonaws.com', + ], $params)); + } + + private function getPrefix(AmazonS3 $storage): string { + $ref = new \ReflectionProperty(AmazonS3::class, 'prefix'); + $ref->setAccessible(true); + return $ref->getValue($storage); + } + + private function callAddPrefix(AmazonS3 $storage, string $path): string { + $ref = new \ReflectionMethod(AmazonS3::class, 'addPrefix'); + $ref->setAccessible(true); + return $ref->invoke($storage, $path); + } + + private function callStripPrefix(AmazonS3 $storage, string $key): string { + $ref = new \ReflectionMethod(AmazonS3::class, 'stripPrefix'); + $ref->setAccessible(true); + return $ref->invoke($storage, $key); + } + + public static function prefixNormalizationProvider(): array { + return [ + 'empty string stays empty' => ['', ''], + 'bare name gets trailing slash' => ['nextcloud', 'nextcloud/'], + 'already has trailing slash' => ['nextcloud/', 'nextcloud/'], + 'leading slash is stripped' => ['/nextcloud/', 'nextcloud/'], + 'nested path' => ['a/b/c', 'a/b/c/'], + 'nested path with slashes' => ['/a/b/c/', 'a/b/c/'], + 'whitespace is trimmed' => [' nextcloud ', 'nextcloud/'], + ]; + } + + #[DataProvider('prefixNormalizationProvider')] + public function testPrefixNormalization(string $input, string $expected): void { + $storage = $this->makeStorage(['prefix' => $input]); + $this->assertSame($expected, $this->getPrefix($storage)); + } + + public function testNoPrefixParameterGivesEmptyPrefix(): void { + $storage = $this->makeStorage(); + $this->assertSame('', $this->getPrefix($storage)); + } + + public static function addPrefixProvider(): array { + return [ + ['nextcloud/', 'path/to/file', 'nextcloud/path/to/file'], + ['nextcloud/', '', 'nextcloud/'], + ['nextcloud/', 'dir/', 'nextcloud/dir/'], + ['a/b/', 'file.txt', 'a/b/file.txt'], + ['', 'path/to/file', 'path/to/file'], + ]; + } + + #[DataProvider('addPrefixProvider')] + public function testAddPrefix(string $prefix, string $path, string $expected): void { + $storage = $this->makeStorage(['prefix' => $prefix]); + $this->assertSame($expected, $this->callAddPrefix($storage, $path)); + } + + public static function stripPrefixProvider(): array { + return [ + 'strips matching prefix' => ['nextcloud/', 'nextcloud/path/to/file', 'path/to/file'], + 'strips to empty string' => ['nextcloud/', 'nextcloud/', ''], + 'strips with trailing slash on key' => ['nextcloud/', 'nextcloud/dir/', 'dir/'], + 'strips nested prefix' => ['a/b/', 'a/b/file.txt', 'file.txt'], + 'empty prefix is passthrough' => ['', 'path/to/file', 'path/to/file'], + 'non-matching key returned unchanged' => ['nextcloud/', 'other/path', 'other/path'], + ]; + } + + #[DataProvider('stripPrefixProvider')] + public function testStripPrefix(string $prefix, string $key, string $expected): void { + $storage = $this->makeStorage(['prefix' => $prefix]); + $this->assertSame($expected, $this->callStripPrefix($storage, $key)); + } + + public function testAddThenStripIsIdentity(): void { + $storage = $this->makeStorage(['prefix' => 'myns/']); + foreach (['file.txt', 'dir/file.txt', 'a/b/c/d.png', ''] as $path) { + $this->assertSame( + $path, + $this->callStripPrefix($storage, $this->callAddPrefix($storage, $path)), + "roundtrip failed for path: $path" + ); + } + } + + public function testStorageIdDiffersWithDifferentPrefix(): void { + $base = ['bucket' => 'b', 'key' => 'k', 'secret' => 's', 'region' => 'us-east-1', 'hostname' => 'h']; + $withoutPrefix = new AmazonS3($base); + $withPrefix = new AmazonS3($base + ['prefix' => 'myns/']); + $withOtherPrefix = new AmazonS3($base + ['prefix' => 'other/']); + + $this->assertNotSame($withoutPrefix->getId(), $withPrefix->getId()); + $this->assertNotSame($withPrefix->getId(), $withOtherPrefix->getId()); + $this->assertNotSame($withoutPrefix->getId(), $withOtherPrefix->getId()); + } + + public function testStorageIdSameForEquivalentPrefixForms(): void { + $base = ['bucket' => 'b', 'key' => 'k', 'secret' => 's', 'region' => 'us-east-1', 'hostname' => 'h']; + $bare = new AmazonS3($base + ['prefix' => 'myns']); + $trailing = new AmazonS3($base + ['prefix' => 'myns/']); + $leading = new AmazonS3($base + ['prefix' => '/myns/']); + + $this->assertSame($bare->getId(), $trailing->getId()); + $this->assertSame($trailing->getId(), $leading->getId()); + } +} diff --git a/apps/files_external/tests/Storage/Amazons3PrefixTest.php b/apps/files_external/tests/Storage/Amazons3PrefixTest.php new file mode 100644 index 0000000000000..f914c4420d4c4 --- /dev/null +++ b/apps/files_external/tests/Storage/Amazons3PrefixTest.php @@ -0,0 +1,96 @@ +loadConfig(__DIR__ . '/../config.amazons3.php'); + $this->instance = new AmazonS3($this->config + ['prefix' => 'test-prefix-a/']); + } + + protected function tearDown(): void { + if ($this->instance) { + $this->instance->rmdir(''); + } + + parent::tearDown(); + } + + public function testStat(): void { + $this->markTestSkipped('S3 doesn\'t update the parents folder mtime'); + } + + public function testPrefixIsolation(): void { + $storageA = new AmazonS3($this->config + ['prefix' => 'test-prefix-a/']); + $storageB = new AmazonS3($this->config + ['prefix' => 'test-prefix-b/']); + + try { + $storageA->file_put_contents('hello.txt', 'from-a'); + + // B must not see a file written by A + $this->assertFalse($storageB->file_exists('hello.txt'), 'Storage B must not see files written by Storage A'); + + $storageB->file_put_contents('hello.txt', 'from-b'); + + // Each storage reads its own copy + $this->assertSame('from-a', $storageA->file_get_contents('hello.txt')); + $this->assertSame('from-b', $storageB->file_get_contents('hello.txt')); + } finally { + $storageA->rmdir(''); + $storageB->rmdir(''); + } + } + + public function testPrefixIsolationDirectory(): void { + $storageA = new AmazonS3($this->config + ['prefix' => 'test-prefix-a/']); + $storageB = new AmazonS3($this->config + ['prefix' => 'test-prefix-b/']); + + try { + $storageA->mkdir('subdir'); + $storageA->file_put_contents('subdir/file.txt', 'data'); + + $this->assertFalse($storageB->is_dir('subdir'), 'Storage B must not see directories created by Storage A'); + $this->assertFalse($storageB->file_exists('subdir/file.txt'), 'Storage B must not see files in directories created by Storage A'); + } finally { + $storageA->rmdir(''); + $storageB->rmdir(''); + } + } + + public function testNoPrefixAndPrefixedMountDoNotOverlap(): void { + $withPrefix = new AmazonS3($this->config + ['prefix' => 'test-prefix-a/']); + $withoutPrefix = new AmazonS3($this->config); + + try { + $withPrefix->file_put_contents('scoped.txt', 'scoped'); + + // The un-prefixed mount must not see 'scoped.txt' at its root + $this->assertFalse($withoutPrefix->file_exists('scoped.txt'), 'Un-prefixed mount must not see files from prefixed mount at its root'); + } finally { + $withPrefix->rmdir(''); + } + } +}