From 6ffd7173f909788dd1808e493b94ec390a785604 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 31 Aug 2020 12:25:20 +0200 Subject: [PATCH 1/2] Don't lose filecache entry on s3 overwrite error If the object store errors we should not always delete the filecache entry. As this might lead to people losing access to their files. Signed-off-by: Roeland Jago Douma --- .../Files/ObjectStore/ObjectStoreStorage.php | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index a5112bcbba686..b2ec094a71e14 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -465,11 +465,22 @@ public function writeStream(string $path, $stream, int $size = null): int { $this->objectStore->writeObject($urn, $stream); } } catch (\Exception $ex) { - $this->getCache()->remove($uploadPath); - $this->logger->logException($ex, [ - 'app' => 'objectstore', - 'message' => 'Could not create object ' . $urn . ' for ' . $path, - ]); + if (!$exists) { + /* + * Only remove the entry if we are dealing with a new file. + * Else people lose access to existing files + */ + $this->getCache()->remove($uploadPath); + $this->logger->logException($ex, [ + 'app' => 'objectstore', + 'message' => 'Could not create object ' . $urn . ' for ' . $path, + ]); + } else { + $this->logger->logException($ex, [ + 'app' => 'objectstore', + 'message' => 'Could not update object ' . $urn . ' for ' . $path, + ]); + } throw $ex; // make this bubble up } From 789b33aba498dddb7c73a9966da75c82008d42de Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 31 Aug 2020 12:28:04 +0200 Subject: [PATCH 2/2] Only update the filecache entry once the file has been written to S3 If we already update before we have no way to revert if the upload fails. Signed-off-by: Roeland Jago Douma --- .../Files/ObjectStore/ObjectStoreStorage.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index b2ec094a71e14..faa0342935e9a 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -446,7 +446,13 @@ public function writeStream(string $path, $stream, int $size = null): int { $exists = $this->getCache()->inCache($path); $uploadPath = $exists ? $path : $path . '.part'; - $fileId = $this->getCache()->put($uploadPath, $stat); + + if ($exists) { + $fileId = $stat['fileid']; + } else { + $fileId = $this->getCache()->put($uploadPath, $stat); + } + $urn = $this->getURN($fileId); try { //upload to object storage @@ -461,6 +467,7 @@ public function writeStream(string $path, $stream, int $size = null): int { if (is_resource($countStream)) { fclose($countStream); } + $stat['size'] = $size; } else { $this->objectStore->writeObject($urn, $stream); } @@ -484,7 +491,9 @@ public function writeStream(string $path, $stream, int $size = null): int { throw $ex; // make this bubble up } - if (!$exists) { + if ($exists) { + $this->getCache()->update($fileId, $stat); + } else { if ($this->objectStore->objectExists($urn)) { $this->getCache()->move($uploadPath, $path); } else {