Skip to content

Commit 93cca86

Browse files
Carl SchwanCarlSchwan
Carl Schwan
authored and
CarlSchwan
committed
perf(s3): Expose pre-signed urls for S3
This is faster than going back to nextcloud to download the files. This is an opt-in setting that can be enabled by setting use_presigned_url in the object store config. Additionally add support for the proxy config which is needed in a docker setup. See juliusknorr/nextcloud-docker-dev#431 Signed-off-by: Carl Schwan <[email protected]>
1 parent df8d838 commit 93cca86

File tree

16 files changed

+151
-24
lines changed

16 files changed

+151
-24
lines changed

‎apps/dav/lib/Connector/Sabre/File.php‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException;
1919
use OCA\DAV\Connector\Sabre\Exception\UnsupportedMediaType;
2020
use OCP\App\IAppManager;
21+
use OCP\Constants;
2122
use OCP\Encryption\Exceptions\GenericEncryptionException;
2223
use OCP\Files;
2324
use OCP\Files\EntityTooLargeException;
@@ -539,18 +540,24 @@ public function getContentType() {
539540
}
540541

541542
/**
542-
* @return array|bool
543+
* @throws NotFoundException
544+
* @throws NotPermittedException
543545
*/
544-
public function getDirectDownload() {
546+
public function getDirectDownload(): array|false {
545547
if (Server::get(IAppManager::class)->isEnabledForUser('encryption')) {
546-
return [];
548+
return false;
547549
}
548-
[$storage, $internalPath] = $this->fileView->resolvePath($this->path);
549-
if (is_null($storage)) {
550-
return [];
550+
$node = $this->getNode();
551+
$storage = $node->getStorage();
552+
if (!$storage) {
553+
return false;
554+
}
555+
556+
if (!($node->getPermissions() & Constants::PERMISSION_READ)) {
557+
return false;
551558
}
552559

553-
return $storage->getDirectDownload($internalPath);
560+
return $storage->getDirectDownloadById((string)$node->getId());
554561
}
555562

556563
/**

‎apps/dav/lib/Connector/Sabre/FilesPlugin.php‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class FilesPlugin extends ServerPlugin {
5050
public const OCM_SHARE_PERMISSIONS_PROPERTYNAME = '{http://open-cloud-mesh.org/ns}share-permissions';
5151
public const SHARE_ATTRIBUTES_PROPERTYNAME = '{http://nextcloud.org/ns}share-attributes';
5252
public const DOWNLOADURL_PROPERTYNAME = '{http://owncloud.org/ns}downloadURL';
53+
public const DOWNLOADURL_EXPIRATION_PROPERTYNAME = '{http://nextcloud.org/ns}download-url-expiration';
5354
public const SIZE_PROPERTYNAME = '{http://owncloud.org/ns}size';
5455
public const GETETAG_PROPERTYNAME = '{DAV:}getetag';
5556
public const LASTMODIFIED_PROPERTYNAME = '{DAV:}lastmodified';
@@ -120,6 +121,7 @@ public function initialize(Server $server) {
120121
$server->protectedProperties[] = self::SHARE_ATTRIBUTES_PROPERTYNAME;
121122
$server->protectedProperties[] = self::SIZE_PROPERTYNAME;
122123
$server->protectedProperties[] = self::DOWNLOADURL_PROPERTYNAME;
124+
$server->protectedProperties[] = self::DOWNLOADURL_EXPIRATION_PROPERTYNAME;
123125
$server->protectedProperties[] = self::OWNER_ID_PROPERTYNAME;
124126
$server->protectedProperties[] = self::OWNER_DISPLAY_NAME_PROPERTYNAME;
125127
$server->protectedProperties[] = self::CHECKSUMS_PROPERTYNAME;
@@ -471,19 +473,30 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
471473
}
472474

473475
if ($node instanceof File) {
474-
$propFind->handle(self::DOWNLOADURL_PROPERTYNAME, function () use ($node) {
476+
$requestProperties = $propFind->getRequestedProperties();
477+
478+
if (in_array(self::DOWNLOADURL_PROPERTYNAME, $requestProperties, true)
479+
|| in_array(self::DOWNLOADURL_EXPIRATION_PROPERTYNAME, $requestProperties, true)) {
475480
try {
476481
$directDownloadUrl = $node->getDirectDownload();
477-
if (isset($directDownloadUrl['url'])) {
482+
} catch (StorageNotAvailableException|ForbiddenException) {
483+
$directDownloadUrl = null;
484+
}
485+
486+
$propFind->handle(self::DOWNLOADURL_PROPERTYNAME, function () use ($node, $directDownloadUrl) {
487+
if ($directDownloadUrl && isset($directDownloadUrl['url'])) {
478488
return $directDownloadUrl['url'];
479489
}
480-
} catch (StorageNotAvailableException $e) {
481490
return false;
482-
} catch (ForbiddenException $e) {
491+
});
492+
493+
$propFind->handle(self::DOWNLOADURL_EXPIRATION_PROPERTYNAME, function () use ($node, $directDownloadUrl) {
494+
if ($directDownloadUrl && isset($directDownloadUrl['expiration'])) {
495+
return $directDownloadUrl['expiration'];
496+
}
483497
return false;
484-
}
485-
return false;
486-
});
498+
});
499+
}
487500

488501
$propFind->handle(self::CHECKSUMS_PROPERTYNAME, function () use ($node) {
489502
$checksum = $node->getChecksum();

‎apps/files_sharing/lib/SharedStorage.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
use OCP\Server;
4141
use OCP\Share\IShare;
4242
use OCP\Util;
43+
use Override;
4344
use Psr\Log\LoggerInterface;
4445

4546
/**
@@ -558,8 +559,15 @@ public function getUnjailedPath(string $path): string {
558559
return parent::getUnjailedPath($path);
559560
}
560561

562+
#[Override]
561563
public function getDirectDownload(string $path): array|false {
562564
// disable direct download for shares
563-
return [];
565+
return false;
566+
}
567+
568+
#[Override]
569+
public function getDirectDownloadById(string $fileId): array|false {
570+
// disable direct download for shares
571+
return false;
564572
}
565573
}

‎lib/private/Files/ObjectStore/Azure.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,4 +117,8 @@ public function objectExists($urn) {
117117
public function copyObject($from, $to) {
118118
$this->getBlobClient()->copyBlob($this->containerName, $to, $this->containerName, $from);
119119
}
120+
121+
public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
122+
return null;
123+
}
120124
}

‎lib/private/Files/ObjectStore/ObjectStoreStorage.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use OCP\Files\Storage\IStorage;
3030
use OCP\IDBConnection;
3131
use OCP\Server;
32+
use Override;
3233
use Psr\Log\LoggerInterface;
3334

3435
class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFileWrite {
@@ -844,4 +845,22 @@ public function free_space(string $path): int|float|false {
844845

845846
return $available;
846847
}
848+
849+
#[Override]
850+
public function getDirectDownloadById(string $fileId): array|false {
851+
$expiration = new \DateTimeImmutable('+60 minutes');
852+
$url = $this->objectStore->preSignedUrl($this->getURN((int)$fileId), $expiration);
853+
return $url ? ['url' => $url, 'expiration' => $expiration->getTimestamp()] : false;
854+
}
855+
856+
#[Override]
857+
public function getDirectDownload(string $path): array|false {
858+
$path = $this->normalizePath($path);
859+
$cacheEntry = $this->getCache()->get($path);
860+
861+
if (!$cacheEntry || $cacheEntry->getMimeType() === FileInfo::MIMETYPE_FOLDER) {
862+
return false;
863+
}
864+
return $this->getDirectDownloadById((string)$cacheEntry->getId());
865+
}
847866
}

‎lib/private/Files/ObjectStore/S3ConnectionTrait.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ trait S3ConnectionTrait {
2828

2929
protected ?S3Client $connection = null;
3030

31+
private bool $usePresignedUrl = false;
32+
3133
protected function parseParams($params) {
3234
if (empty($params['bucket'])) {
3335
throw new \Exception('Bucket has to be configured.');
@@ -93,12 +95,15 @@ public function getConnection() {
9395
)
9496
);
9597

98+
$this->usePresignedUrl = $this->params['use_presigned_url'] ?? false;
99+
96100
$options = [
97101
'version' => $this->params['version'] ?? 'latest',
98102
'credentials' => $provider,
99103
'endpoint' => $base_url,
100104
'region' => $this->params['region'],
101105
'use_path_style_endpoint' => isset($this->params['use_path_style']) ? $this->params['use_path_style'] : false,
106+
'proxy' => isset($this->params['proxy']) ? $this->params['proxy'] : false,
102107
'signature_provider' => \Aws\or_chain([self::class, 'legacySignatureProvider'], ClientResolver::_default_signature_provider()),
103108
'csm' => false,
104109
'use_arn_region' => false,
@@ -256,4 +261,8 @@ protected function getSSECParameters(bool $copy = false): array {
256261
'SSECustomerKeyMD5' => md5($rawKey, true)
257262
];
258263
}
264+
265+
public function isUsePresignedUrl(): bool {
266+
return $this->usePresignedUrl;
267+
}
259268
}

‎lib/private/Files/ObjectStore/S3ObjectTrait.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
namespace OC\Files\ObjectStore;
88

99
use Aws\Command;
10+
use Aws\Exception\AwsException;
1011
use Aws\Exception\MultipartUploadException;
1112
use Aws\S3\Exception\S3MultipartUploadException;
1213
use Aws\S3\MultipartCopy;
@@ -291,4 +292,23 @@ public function copyObject($from, $to, array $options = []) {
291292
], $options));
292293
}
293294
}
295+
296+
public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
297+
$command = $this->getConnection()->getCommand('GetObject', [
298+
'Bucket' => $this->getBucket(),
299+
'Key' => $urn,
300+
]);
301+
302+
if (!$this->isUsePresignedUrl()) {
303+
return null;
304+
}
305+
306+
try {
307+
return (string)$this->getConnection()->createPresignedRequest($command, $expiration, [
308+
'signPayload' => true,
309+
])->getUri();
310+
} catch (AwsException) {
311+
return null;
312+
}
313+
}
294314
}

‎lib/private/Files/ObjectStore/StorageObjectStore.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,8 @@ public function objectExists($urn) {
7474
public function copyObject($from, $to) {
7575
$this->storage->copy($from, $to);
7676
}
77+
78+
public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
79+
return null;
80+
}
7781
}

‎lib/private/Files/ObjectStore/Swift.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,7 @@ public function copyObject($from, $to) {
134134
'destination' => $this->getContainer()->name . '/' . $to
135135
]);
136136
}
137+
public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
138+
return null;
139+
}
137140
}

‎lib/private/Files/Storage/Common.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
use OCP\Lock\ILockingProvider;
3939
use OCP\Lock\LockedException;
4040
use OCP\Server;
41+
use Override;
4142
use Psr\Log\LoggerInterface;
4243

4344
/**
@@ -445,13 +446,14 @@ public function instanceOfStorage(string $class): bool {
445446
return is_a($this, $class);
446447
}
447448

448-
/**
449-
* A custom storage implementation can return an url for direct download of a give file.
450-
*
451-
* For now the returned array can hold the parameter url - in future more attributes might follow.
452-
*/
449+
#[Override]
453450
public function getDirectDownload(string $path): array|false {
454-
return [];
451+
return false;
452+
}
453+
454+
#[Override]
455+
public function getDirectDownloadById(string $fileId): array|false {
456+
return false;
455457
}
456458

457459
public function verifyPath(string $path, string $fileName): void {

0 commit comments

Comments
 (0)