Skip to content

Commit 2687b73

Browse files
committed
fix: validate written size for s3 multipart uploads
Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent 4eda352 commit 2687b73

3 files changed

Lines changed: 28 additions & 7 deletions

File tree

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@ public function put($data) {
204204
}
205205
}
206206

207+
$lengthHeader = $this->request->getHeader('content-length');
208+
$expected = $lengthHeader !== '' ? (int)$lengthHeader : null;
209+
207210
if ($partStorage->instanceOfStorage(IWriteStreamStorage::class)) {
208211
$isEOF = false;
209212
$wrappedData = CallbackWrapper::wrap($data, null, null, null, null, function ($stream) use (&$isEOF): void {
@@ -215,7 +218,7 @@ public function put($data) {
215218
$count = -1;
216219
try {
217220
/** @var IWriteStreamStorage $partStorage */
218-
$count = $partStorage->writeStream($internalPartPath, $wrappedData);
221+
$count = $partStorage->writeStream($internalPartPath, $wrappedData, $expected);
219222
} catch (GenericFileException $e) {
220223
$logger = Server::get(LoggerInterface::class);
221224
$logger->error('Error while writing stream to storage: ' . $e->getMessage(), ['exception' => $e, 'app' => 'webdav']);
@@ -235,10 +238,7 @@ public function put($data) {
235238
[$count, $result] = Files::streamCopy($data, $target, true);
236239
fclose($target);
237240
}
238-
239-
$lengthHeader = $this->request->getHeader('content-length');
240-
$expected = $lengthHeader !== '' ? (int)$lengthHeader : -1;
241-
if ($result === false && $expected >= 0) {
241+
if ($result === false && $expected != null) {
242242
throw new Exception(
243243
$this->l10n->t(
244244
'Error while copying file to target location (copied: %1$s, expected filesize: %2$s)',
@@ -253,7 +253,7 @@ public function put($data) {
253253
// if content length is sent by client:
254254
// double check if the file was fully received
255255
// compare expected and actual size
256-
if ($expected >= 0
256+
if ($expected != null
257257
&& $expected !== $count
258258
&& $this->request->getMethod() === 'PUT'
259259
) {

lib/private/Files/ObjectStore/ObjectStoreStorage.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,9 @@ public function writeStream(string $path, $stream, ?int $size = null): int {
475475
'original-storage' => $this->getId(),
476476
'original-path' => $path,
477477
];
478+
if ($size) {
479+
$metadata['size'] = $size;
480+
}
478481

479482
$stat['mimetype'] = $mimetype;
480483
$stat['etag'] = $this->getETag($path);

lib/private/Files/ObjectStore/S3ObjectTrait.php

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
*/
77
namespace OC\Files\ObjectStore;
88

9+
use Aws\Command;
10+
use Aws\Exception\MultipartUploadException;
911
use Aws\S3\Exception\S3MultipartUploadException;
1012
use Aws\S3\MultipartCopy;
1113
use Aws\S3\MultipartUploader;
@@ -125,6 +127,8 @@ protected function writeMultiPart(string $urn, StreamInterface $stream, array $m
125127
$concurrency = $this->concurrency;
126128
$exception = null;
127129
$state = null;
130+
$size = $stream->getSize();
131+
$totalWritten = 0;
128132

129133
// retry multipart upload once with concurrency at half on failure
130134
while (!$uploaded && $attempts <= 1) {
@@ -139,6 +143,15 @@ protected function writeMultiPart(string $urn, StreamInterface $stream, array $m
139143
'Metadata' => $this->buildS3Metadata($metaData),
140144
'StorageClass' => $this->storageClass,
141145
] + $this->getSSECParameters(),
146+
'before_upload' => function (Command $command) use (&$totalWritten) {
147+
$totalWritten += $command['ContentLength'];
148+
},
149+
'before_complete' => function ($_command) use (&$totalWritten, $size, &$uploader, &$attempts) {
150+
if ($size !== null && $totalWritten != $size) {
151+
$e = new \Exception('Incomplete multi part upload, expected ' . $size . ' bytes, wrote ' . $totalWritten);
152+
throw new MultipartUploadException($uploader->getState(), $e);
153+
}
154+
},
142155
]);
143156

144157
try {
@@ -155,6 +168,9 @@ protected function writeMultiPart(string $urn, StreamInterface $stream, array $m
155168
if ($stream->isSeekable()) {
156169
$stream->rewind();
157170
}
171+
} catch (MultipartUploadException $e) {
172+
$exception = $e;
173+
break;
158174
}
159175
}
160176

@@ -180,7 +196,9 @@ public function writeObject($urn, $stream, ?string $mimetype = null) {
180196

181197
public function writeObjectWithMetaData(string $urn, $stream, array $metaData): void {
182198
$canSeek = fseek($stream, 0, SEEK_CUR) === 0;
183-
$psrStream = Utils::streamFor($stream);
199+
$psrStream = Utils::streamFor($stream, [
200+
'size' => $metaData['size'] ?? null,
201+
]);
184202

185203

186204
$size = $psrStream->getSize();

0 commit comments

Comments
 (0)