diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 97a588ca2e..549120bc2c 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -97,29 +97,40 @@ trait S3ObjectTrait { $count += $read; }); - $s3params = [ - 'bucket' => $this->bucket, - 'key' => $urn, - 'part_size' => $this->uploadPartSize, - 'params' => [ - 'ContentType' => $mimetype - ] + $this->getSseKmsPutParameters(), - ]; - $uploader = new MultipartUploader($this->getConnection(), $countStream, $s3params); + + if ($count === 0 && feof($countStream)) { + // This is an empty file so just touch it then + $s3params = [ + 'params' => $this->getSseKmsPutParameters(), + ]; + $uploader = new ObjectUploader($this->getConnection(), $this->bucket, $urn, '', 'private', $s3params); + } else { + $s3params = [ + 'bucket' => $this->bucket, + 'key' => $urn, + 'part_size' => $this->uploadPartSize, + 'params' => [ + 'ContentType' => $mimetype + ] + $this->getSseKmsPutParameters(), + ]; + + // maybe, we should also use ObjectUploader here in the future + // it does direct uploads for small files < 5MB and multipart otherwise + // $uploader = new ObjectUploader($this->getConnection(), $this->bucket, $urn, $countStream, 'private', $s3params); + $uploader = new MultipartUploader($this->getConnection(), $countStream, $s3params); + } try { $uploader->upload(); } catch (S3MultipartUploadException $e) { - // This is an empty file so just touch it then - if ($count === 0 && feof($countStream)) { - $s3params = [ - 'params' => $this->getSseKmsPutParameters(), - ]; - $uploader = new ObjectUploader($this->getConnection(), $this->bucket, $urn, '', 'private', $s3params); - $uploader->upload(); - } else { - throw $e; - } + // if anything goes wrong with multipart, make sure that you donĀ“t poison s3 bucket with fragments + $this->getConnection()->abortMultipartUpload([ + 'Bucket' => $this->bucket, + 'Key' => $urn, + 'UploadId' => $uploader->getState()->getId() + ]); + + throw $e; } finally { // this handles [S3] fclose(): supplied resource is not a valid stream resource #23373 // see https://stackoverflow.com/questions/11247507/fclose-18-is-not-a-valid-stream-resource/11247555 diff --git a/tests/lib/Files/ObjectStore/S3Test.php b/tests/lib/Files/ObjectStore/S3Test.php index ebffdcdc74..1a0349ca65 100644 --- a/tests/lib/Files/ObjectStore/S3Test.php +++ b/tests/lib/Files/ObjectStore/S3Test.php @@ -22,8 +22,10 @@ namespace Test\Files\ObjectStore; use Icewind\Streams\Wrapper; +use Icewind\Streams\CallbackWrapper; use OC\Files\ObjectStore\S3; + class MultiPartUploadS3 extends S3 { public function writeObject($urn, $stream, string $mimetype = null) { $this->getConnection()->upload($this->bucket, $urn, $stream, 'private', [ @@ -94,4 +96,22 @@ class S3Test extends ObjectStoreTest { fseek($read, 100, SEEK_CUR); $this->assertEquals(substr($data, 210, 100), fread($read, 100)); } + + public function testEmptyUpload() { + $s3 = $this->getInstance(); + + $emptyStream = fopen('php://memory', 'w+'); + $count = 0; + $countStream = CallbackWrapper::wrap($emptyStream, function ($read) use (&$count) { + $count += $read; + }); + $this->assertEquals($count, 0); + $this->assertTrue(feof($countStream)); + + $s3->writeObject('emptyuploadtest', $emptyString); + + $result = $s3->readObject('emptyuploadtest'); + $this->assertEquals('', stream_get_contents($result)); + } + }