Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/files_external/lib/Lib/Backend/AmazonS3.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')))
Expand Down
59 changes: 39 additions & 20 deletions apps/files_external/lib/Lib/Storage/AmazonS3.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
]);

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
Expand All @@ -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(), [
Expand Down Expand Up @@ -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(), [
Expand All @@ -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',
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -617,29 +633,32 @@ 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;
}
}
}
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);
}
}
Expand Down Expand Up @@ -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;
Expand All @@ -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');

Expand Down
96 changes: 96 additions & 0 deletions apps/files_external/tests/Storage/AmazonS3PrefixTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Files_External\Tests\Storage;

use OCA\Files_External\Lib\Storage\AmazonS3;

/**
* Runs the full storage test suite against an S3 mount that has a prefix set,
* and adds isolation tests to verify that two mounts with different prefixes
* in the same bucket cannot see each other's files.
*/
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
#[\PHPUnit\Framework\Attributes\Group('S3')]
class Amazons3PrefixTest extends \Test\Files\Storage\Storage {
use ConfigurableStorageTrait;

/** @var AmazonS3 */
protected $instance;

protected function setUp(): void {
parent::setUp();

$this->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('');
}
}
}
Loading