From 15726db2051e7ab8d4ece7601a6e58e2139493d8 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 14 Mar 2025 09:12:24 +0100 Subject: [PATCH 1/2] refactor(dav): simplify length header handling Reduce nesting and drop duplicated sections. Signed-off-by: Ferdinand Thiessen --- apps/dav/lib/Connector/Sabre/File.php | 56 ++++++++++++--------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 67b562a6221cc..784b99c9028b3 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -230,42 +230,36 @@ public function put($data) { fclose($target); } - if ($result === false) { - $expected = -1; - $lengthHeader = $this->request->getHeader('content-length'); - if ($lengthHeader) { - $expected = (int)$lengthHeader; - } - if ($expected !== 0) { - throw new Exception( - $this->l10n->t( - 'Error while copying file to target location (copied: %1$s, expected filesize: %2$s)', - [ - $this->l10n->n('%n byte', '%n bytes', $count), - $this->l10n->n('%n byte', '%n bytes', $expected), - ], - ) - ); - } + $lengthHeader = $this->request->getHeader('content-length'); + $expected = $lengthHeader !== '' ? (int)$lengthHeader : -1; + if ($result === false && $expected >= 0) { + throw new Exception( + $this->l10n->t( + 'Error while copying file to target location (copied: %1$s, expected filesize: %2$s)', + [ + $this->l10n->n('%n byte', '%n bytes', $count), + $this->l10n->n('%n byte', '%n bytes', $expected), + ], + ) + ); } // if content length is sent by client: // double check if the file was fully received // compare expected and actual size - $lengthHeader = $this->request->getHeader('content-length'); - if ($lengthHeader && $this->request->getMethod() === 'PUT') { - $expected = (int)$lengthHeader; - if ($count !== $expected) { - throw new BadRequest( - $this->l10n->t( - 'Expected filesize of %1$s but read (from Nextcloud client) and wrote (to Nextcloud storage) %2$s. Could either be a network problem on the sending side or a problem writing to the storage on the server side.', - [ - $this->l10n->n('%n byte', '%n bytes', $expected), - $this->l10n->n('%n byte', '%n bytes', $count), - ], - ) - ); - } + if ($expected >= 0 + && $expected !== $count + && $this->request->getMethod() === 'PUT' + ) { + throw new BadRequest( + $this->l10n->t( + 'Expected filesize of %1$s but read (from Nextcloud client) and wrote (to Nextcloud storage) %2$s. Could either be a network problem on the sending side or a problem writing to the storage on the server side.', + [ + $this->l10n->n('%n byte', '%n bytes', $expected), + $this->l10n->n('%n byte', '%n bytes', $count), + ], + ) + ); } } catch (\Exception $e) { if ($e instanceof LockedException) { From d3fe0826b4b68b586ed1aec1a19d0625869f32f8 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 14 Mar 2025 17:59:47 +0100 Subject: [PATCH 2/2] fix(dav): allow uploading of files with long filenames A filename must be less or equal 255 characters, but when adding the `.part` and `.ocfiletransfer` extensions we might overflow this limit. So we should also use filename hashes for uploading when the file has a long filename, similar like when we are uploading to the user storage directly. Signed-off-by: Ferdinand Thiessen --- apps/dav/lib/Connector/Sabre/File.php | 12 ++++++-- build/integration/dav_features/dav-v2.feature | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 784b99c9028b3..3fcb4c3294b5d 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -127,8 +127,9 @@ public function put($data) { $view = Filesystem::getView(); if ($needsPartFile) { + $transferId = \rand(); // mark file as partial while uploading (ignored by the scanner) - $partFilePath = $this->getPartFileBasePath($this->path) . '.ocTransferId' . rand() . '.part'; + $partFilePath = $this->getPartFileBasePath($this->path) . '.ocTransferId' . $transferId . '.part'; if (!$view->isCreatable($partFilePath) && $view->isUpdatable($this->path)) { $needsPartFile = false; @@ -375,9 +376,14 @@ public function put($data) { private function getPartFileBasePath($path) { $partFileInStorage = \OC::$server->getConfig()->getSystemValue('part_file_in_storage', true); if ($partFileInStorage) { - return $path; + $filename = basename($path); + // hash does not need to be secure but fast and semi unique + $hashedFilename = hash('xxh128', $filename); + return substr($path, 0, strlen($path) - strlen($filename)) . $hashedFilename; } else { - return md5($path); // will place it in the root of the view with a unique name + // will place the .part file in the users root directory + // therefor we need to make the name (semi) unique - hash does not need to be secure but fast. + return hash('xxh128', $path); } } diff --git a/build/integration/dav_features/dav-v2.feature b/build/integration/dav_features/dav-v2.feature index 2c74030c46276..c575483d57ca7 100644 --- a/build/integration/dav_features/dav-v2.feature +++ b/build/integration/dav_features/dav-v2.feature @@ -108,6 +108,24 @@ Feature: dav-v2 When User "user0" uploads file "data/textfile.txt" to "/testquota/asdf.txt" Then the HTTP status code should be "201" + Scenario: Uploading a file with very long filename + Given using new dav path + And As an "admin" + And user "user0" exists + And user "user0" has a quota of "10 MB" + And As an "user0" + When User "user0" uploads file "data/textfile.txt" to "/long-filename-with-250-characters-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.txt" + Then the HTTP status code should be "201" + + Scenario: Uploading a file with a too long filename + Given using new dav path + And As an "admin" + And user "user0" exists + And user "user0" has a quota of "10 MB" + And As an "user0" + When User "user0" uploads file "data/textfile.txt" to "/long-filename-with-251-characters-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.txt" + Then the HTTP status code should be "400" + Scenario: Create a search query on image Given using new dav path And As an "admin" @@ -132,3 +150,14 @@ Feature: dav-v2 Then Favorite search should work And the single response should contain a property "{http://owncloud.org/ns}favorite" with value "1" + Scenario: Create a search query on favorite + Given using new dav path + And As an "admin" + And user "user0" exists + And As an "user0" + When User "user0" uploads file "data/green-square-256.png" to "/fav_image.png" + Then Favorite search should work + And the response should be empty + When user "user0" favorites element "/fav_image.png" + Then Favorite search should work + And the single response should contain a property "{http://owncloud.org/ns}favorite" with value "1"