From ad5be4f04bc2730fd4bd6b989137cbaeb2f8338e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 29 Nov 2023 17:31:34 +0100 Subject: [PATCH 1/2] only do a multipart s3 copy when above the regular copy limit Signed-off-by: Robin Appelman --- .../tests/Storage/Amazons3MultiPartTest.php | 68 +++++++++++++++++++ .../Files/ObjectStore/S3ConnectionTrait.php | 4 ++ .../Files/ObjectStore/S3ObjectTrait.php | 30 +++++--- 3 files changed, 91 insertions(+), 11 deletions(-) create mode 100644 apps/files_external/tests/Storage/Amazons3MultiPartTest.php diff --git a/apps/files_external/tests/Storage/Amazons3MultiPartTest.php b/apps/files_external/tests/Storage/Amazons3MultiPartTest.php new file mode 100644 index 0000000000000..d102a2a064efd --- /dev/null +++ b/apps/files_external/tests/Storage/Amazons3MultiPartTest.php @@ -0,0 +1,68 @@ + + * + * @author Maxence Lange + * @author Robin Appelman + * + * @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 . + * + */ +namespace OCA\files_external\tests\Storage; + +use OCA\Files_External\Lib\Storage\AmazonS3; + +/** + * Class Amazons3Test + * + * @group DB + * + * @package OCA\Files_External\Tests\Storage + */ +class Amazons3MultiPartTest extends \Test\Files\Storage\Storage { + private $config; + /** @var AmazonS3 */ + protected $instance; + + protected function setUp(): void { + parent::setUp(); + + $this->config = include('files_external/tests/config.amazons3.php'); + if (! is_array($this->config) or ! $this->config['run']) { + $this->markTestSkipped('AmazonS3 backend not configured'); + } + $this->instance = new AmazonS3($this->config + [ + 'putSizeLimit' => 1, + 'copySizeLimit' => 1, + ]); + } + + protected function tearDown(): void { + if ($this->instance) { + $this->instance->rmdir(''); + } + + parent::tearDown(); + } + + 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/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php index 49942b385bcda..51d9f687a13b8 100644 --- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php +++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php @@ -71,6 +71,9 @@ trait S3ConnectionTrait { /** @var int */ private $putSizeLimit; + /** @var int */ + private $copySizeLimit; + protected $test; protected function parseParams($params) { @@ -87,6 +90,7 @@ protected function parseParams($params) { $this->storageClass = !empty($params['storageClass']) ? $params['storageClass'] : 'STANDARD'; $this->uploadPartSize = $params['uploadPartSize'] ?? 524288000; $this->putSizeLimit = $params['putSizeLimit'] ?? 104857600; + $this->copySizeLimit = $params['copySizeLimit'] ?? 5242880000; $params['region'] = empty($params['region']) ? 'eu-west-1' : $params['region']; $params['hostname'] = empty($params['hostname']) ? 's3.' . $params['region'] . '.amazonaws.com' : $params['hostname']; if (!isset($params['port']) || $params['port'] === '') { diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 217e1a1a2ff7e..65952a76a6132 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -196,16 +196,24 @@ public function copyObject($from, $to, array $options = []) { 'Key' => $from, ] + $this->getSSECParameters()); - $copy = new MultipartCopy($this->getConnection(), [ - "source_bucket" => $this->getBucket(), - "source_key" => $from - ], array_merge([ - "bucket" => $this->getBucket(), - "key" => $to, - "acl" => "private", - "params" => $this->getSSECParameters() + $this->getSSECParameters(true), - "source_metadata" => $sourceMetadata - ], $options)); - $copy->copy(); + $size = (int)($sourceMetadata->get('Size') ?? $sourceMetadata->get('ContentLength')); + + if ($size > $this->copySizeLimit) { + $copy = new MultipartCopy($this->getConnection(), [ + "source_bucket" => $this->getBucket(), + "source_key" => $from + ], array_merge([ + "bucket" => $this->getBucket(), + "key" => $to, + "acl" => "private", + "params" => $this->getSSECParameters() + $this->getSSECParameters(true), + "source_metadata" => $sourceMetadata + ], $options)); + $copy->copy(); + } else { + $this->getConnection()->copy($this->getBucket(), $from, $this->getBucket(), $to, 'private', array_merge([ + 'params' => $this->getSSECParameters() + $this->getSSECParameters(true) + ], $options)); + } } } From f5fe5316657ee32caca74072b3b140577ef7c55b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 28 Dec 2023 15:31:08 +0100 Subject: [PATCH 2/2] fix(s3): Add config option to disable multipart copy for certain s3 providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/Files/ObjectStore/S3ConnectionTrait.php | 3 +++ lib/private/Files/ObjectStore/S3ObjectTrait.php | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php index 51d9f687a13b8..e6ed9c2ef5b8c 100644 --- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php +++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php @@ -74,6 +74,8 @@ trait S3ConnectionTrait { /** @var int */ private $copySizeLimit; + private bool $useMultipartCopy = true; + protected $test; protected function parseParams($params) { @@ -91,6 +93,7 @@ protected function parseParams($params) { $this->uploadPartSize = $params['uploadPartSize'] ?? 524288000; $this->putSizeLimit = $params['putSizeLimit'] ?? 104857600; $this->copySizeLimit = $params['copySizeLimit'] ?? 5242880000; + $this->useMultipartCopy = (bool)($params['useMultipartCopy'] ?? true); $params['region'] = empty($params['region']) ? 'eu-west-1' : $params['region']; $params['hostname'] = empty($params['hostname']) ? 's3.' . $params['region'] . '.amazonaws.com' : $params['hostname']; if (!isset($params['port']) || $params['port'] === '') { diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 65952a76a6132..2ef9614ac85c3 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -198,7 +198,7 @@ public function copyObject($from, $to, array $options = []) { $size = (int)($sourceMetadata->get('Size') ?? $sourceMetadata->get('ContentLength')); - if ($size > $this->copySizeLimit) { + if ($this->useMultipartCopy && $size > $this->copySizeLimit) { $copy = new MultipartCopy($this->getConnection(), [ "source_bucket" => $this->getBucket(), "source_key" => $from