From c3fc2d81f45fd51498dc63ef674cc505022cbe92 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 18 Nov 2024 17:35:09 +0100 Subject: [PATCH 1/2] fix: use php://temp instead of php://memory for multi-part upload buffer this should reduce potential memory issues if the limit is set very high Signed-off-by: Robin Appelman --- lib/private/Files/ObjectStore/S3ObjectTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 2e625033751c5..dfe96294c6903 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -144,7 +144,7 @@ public function writeObject($urn, $stream, ?string $mimetype = null) { // ($psrStream->isSeekable() && $psrStream->getSize() !== null) evaluates to true for a On-Seekable stream // so the optimisation does not apply - $buffer = new Psr7\Stream(fopen('php://memory', 'rwb+')); + $buffer = new Psr7\Stream(fopen('php://temp', 'rwb+')); Utils::copyToStream($psrStream, $buffer, $this->putSizeLimit); $buffer->seek(0); if ($buffer->getSize() < $this->putSizeLimit) { From ef0ce7c886277b98bfe7753471a0573d00cb5cc5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 18 Nov 2024 17:53:50 +0100 Subject: [PATCH 2/2] fix: don't perform the extra buffering in s3 stream write when the stream size is known Signed-off-by: Robin Appelman --- .../Files/ObjectStore/S3ObjectTrait.php | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index dfe96294c6903..9d7cfa644e66d 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -140,20 +140,33 @@ protected function writeMultiPart(string $urn, StreamInterface $stream, ?string * @since 7.0.0 */ public function writeObject($urn, $stream, ?string $mimetype = null) { + $canSeek = fseek($stream, 0, SEEK_CUR) === 0; $psrStream = Utils::streamFor($stream); - // ($psrStream->isSeekable() && $psrStream->getSize() !== null) evaluates to true for a On-Seekable stream - // so the optimisation does not apply - $buffer = new Psr7\Stream(fopen('php://temp', 'rwb+')); - Utils::copyToStream($psrStream, $buffer, $this->putSizeLimit); - $buffer->seek(0); - if ($buffer->getSize() < $this->putSizeLimit) { - // buffer is fully seekable, so use it directly for the small upload - $this->writeSingle($urn, $buffer, $mimetype); + + $size = $psrStream->getSize(); + if ($size === null || !$canSeek) { + // The s3 single-part upload requires the size to be known for the stream. + // So for input streams that don't have a known size, we need to copy (part of) + // the input into a temporary stream so the size can be determined + $buffer = new Psr7\Stream(fopen('php://temp', 'rw+')); + Utils::copyToStream($psrStream, $buffer, $this->putSizeLimit); + $buffer->seek(0); + if ($buffer->getSize() < $this->putSizeLimit) { + // buffer is fully seekable, so use it directly for the small upload + $this->writeSingle($urn, $buffer, $mimetype); + } else { + $loadStream = new Psr7\AppendStream([$buffer, $psrStream]); + $this->writeMultiPart($urn, $loadStream, $mimetype); + } } else { - $loadStream = new Psr7\AppendStream([$buffer, $psrStream]); - $this->writeMultiPart($urn, $loadStream, $mimetype); + if ($size < $this->putSizeLimit) { + $this->writeSingle($urn, $psrStream, $mimetype); + } else { + $this->writeMultiPart($urn, $psrStream, $mimetype); + } } + $psrStream->close(); } /**