From cb2a6d688fd48e9f4f903df0d45bfaf184b3e08f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 4 Sep 2023 14:56:06 +0200 Subject: [PATCH 1/3] Detect aborted connection in OC\Files\View and stop writing data to the output buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Files/View.php | 12 ++++++++++-- lib/private/legacy/OC_Files.php | 3 --- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 28b2cf6764071..a6b802ffe8542 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -428,6 +428,7 @@ public function readfile($path) { $chunkSize = 524288; // 512 kB chunks while (!feof($handle)) { echo fread($handle, $chunkSize); + $this->checkConnectionStatus(); flush(); } fclose($handle); @@ -480,6 +481,7 @@ public function readfilePart($path, $from, $to) { $len = $chunkSize; } echo fread($handle, $len); + $this->checkConnectionStatus(); flush(); } return ftell($handle) - $from; @@ -490,6 +492,14 @@ public function readfilePart($path, $from, $to) { return false; } + + private function checkConnectionStatus(): void { + $connectionStatus = \connection_status(); + if ($connectionStatus !== 0) { + throw new \RuntimeException("Connection lost. Status: $connectionStatus"); + } + } + /** * @param string $path * @return mixed @@ -1053,7 +1063,6 @@ public function toTmpFile($path) { public function fromTmpFile($tmpFile, $path) { $this->assertPathLength($path); if (Filesystem::isValidPath($path)) { - // Get directory that the file is going into $filePath = dirname($path); @@ -1809,7 +1818,6 @@ private function assertPathLength($path) { * @return boolean */ private function targetIsNotShared(IStorage $targetStorage, string $targetInternalPath) { - // note: cannot use the view because the target is already locked $fileId = (int)$targetStorage->getCache()->getId($targetInternalPath); if ($fileId === -1) { diff --git a/lib/private/legacy/OC_Files.php b/lib/private/legacy/OC_Files.php index 6a3a44d6cc0e5..48e36dac2cee5 100644 --- a/lib/private/legacy/OC_Files.php +++ b/lib/private/legacy/OC_Files.php @@ -238,9 +238,6 @@ public static function get($dir, $files, $params = null) { OC::$server->getLogger()->logException($ex); $l = \OC::$server->getL10N('lib'); $hint = method_exists($ex, 'getHint') ? $ex->getHint() : ''; - if ($event && $event->getErrorMessage() !== null) { - $hint .= ' ' . $event->getErrorMessage(); - } \OC_Template::printErrorPage($l->t('Cannot download file'), $hint, 200); } } From 168b07d038ddac0e3f5468e1c180d2ce39217316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 4 Sep 2023 16:39:41 +0200 Subject: [PATCH 2/3] Lower log to debug for connections aborted by the client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/View.php | 3 +- lib/private/legacy/OC_Files.php | 4 +++ lib/public/Files/ConnectionLostException.php | 30 ++++++++++++++++++++ 5 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 lib/public/Files/ConnectionLostException.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index e9c50b812e053..0f871f4dda841 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -274,6 +274,7 @@ 'OCP\\Files\\Config\\IMountProviderCollection' => $baseDir . '/lib/public/Files/Config/IMountProviderCollection.php', 'OCP\\Files\\Config\\IRootMountProvider' => $baseDir . '/lib/public/Files/Config/IRootMountProvider.php', 'OCP\\Files\\Config\\IUserMountCache' => $baseDir . '/lib/public/Files/Config/IUserMountCache.php', + 'OCP\\Files\\ConnectionLostException' => $baseDir . '/lib/public/Files/ConnectionLostException.php', 'OCP\\Files\\DavUtil' => $baseDir . '/lib/public/Files/DavUtil.php', 'OCP\\Files\\EmptyFileNameException' => $baseDir . '/lib/public/Files/EmptyFileNameException.php', 'OCP\\Files\\EntityTooLargeException' => $baseDir . '/lib/public/Files/EntityTooLargeException.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 5a85e16d1b42c..91b2cfb2c2026 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -307,6 +307,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Files\\Config\\IMountProviderCollection' => __DIR__ . '/../../..' . '/lib/public/Files/Config/IMountProviderCollection.php', 'OCP\\Files\\Config\\IRootMountProvider' => __DIR__ . '/../../..' . '/lib/public/Files/Config/IRootMountProvider.php', 'OCP\\Files\\Config\\IUserMountCache' => __DIR__ . '/../../..' . '/lib/public/Files/Config/IUserMountCache.php', + 'OCP\\Files\\ConnectionLostException' => __DIR__ . '/../../..' . '/lib/public/Files/ConnectionLostException.php', 'OCP\\Files\\DavUtil' => __DIR__ . '/../../..' . '/lib/public/Files/DavUtil.php', 'OCP\\Files\\EmptyFileNameException' => __DIR__ . '/../../..' . '/lib/public/Files/EmptyFileNameException.php', 'OCP\\Files\\EntityTooLargeException' => __DIR__ . '/../../..' . '/lib/public/Files/EntityTooLargeException.php', diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index a6b802ffe8542..908302426fa73 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -53,6 +53,7 @@ use OCA\Files_Sharing\SharedMount; use OCP\Constants; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\ConnectionLostException; use OCP\Files\EmptyFileNameException; use OCP\Files\FileNameTooLongException; use OCP\Files\InvalidCharacterInPathException; @@ -496,7 +497,7 @@ public function readfilePart($path, $from, $to) { private function checkConnectionStatus(): void { $connectionStatus = \connection_status(); if ($connectionStatus !== 0) { - throw new \RuntimeException("Connection lost. Status: $connectionStatus"); + throw new ConnectionLostException("Connection lost. Status: $connectionStatus"); } } diff --git a/lib/private/legacy/OC_Files.php b/lib/private/legacy/OC_Files.php index 48e36dac2cee5..abba0d9368f4c 100644 --- a/lib/private/legacy/OC_Files.php +++ b/lib/private/legacy/OC_Files.php @@ -233,6 +233,10 @@ public static function get($dir, $files, $params = null) { OC::$server->getLogger()->logException($ex); $l = \OC::$server->getL10N('lib'); \OC_Template::printErrorPage($l->t('Cannot download file'), $ex->getMessage(), 200); + } catch (\OCP\Files\ConnectionLostException $ex) { + self::unlockAllTheFiles($dir, $files, $getType, $view, $filename); + OC::$server->getLogger()->logException($ex, ['level' => \OCP\ILogger::DEBUG]); + \OC_Template::printErrorPage('Connection lost', $ex->getMessage(), 200); } catch (\Exception $ex) { self::unlockAllTheFiles($dir, $files, $getType, $view, $filename); OC::$server->getLogger()->logException($ex); diff --git a/lib/public/Files/ConnectionLostException.php b/lib/public/Files/ConnectionLostException.php new file mode 100644 index 0000000000000..bad00a1f1d202 --- /dev/null +++ b/lib/public/Files/ConnectionLostException.php @@ -0,0 +1,30 @@ + + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCP\Files; + +/** + * Exception for lost connection with the + * @since 25.0.11 + */ +class ConnectionLostException extends \RuntimeException { +} From 6b2580d49ac19592c6857f328396e23551943dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <91878298+come-nc@users.noreply.github.com> Date: Tue, 5 Sep 2023 11:31:00 +0200 Subject: [PATCH 3/3] Apply suggestions from code review in View.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Benjamin Gaussorgues Signed-off-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com> Signed-off-by: Côme Chilliet --- lib/private/Files/View.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 908302426fa73..2f4a4f26c15b6 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -426,11 +426,11 @@ public function readfile($path) { } $handle = $this->fopen($path, 'rb'); if ($handle) { - $chunkSize = 524288; // 512 kB chunks + $chunkSize = 524288; // 512 kiB chunks while (!feof($handle)) { echo fread($handle, $chunkSize); - $this->checkConnectionStatus(); flush(); + $this->checkConnectionStatus(); } fclose($handle); return $this->filesize($path); @@ -482,8 +482,8 @@ public function readfilePart($path, $from, $to) { $len = $chunkSize; } echo fread($handle, $len); - $this->checkConnectionStatus(); flush(); + $this->checkConnectionStatus(); } return ftell($handle) - $from; } @@ -496,7 +496,7 @@ public function readfilePart($path, $from, $to) { private function checkConnectionStatus(): void { $connectionStatus = \connection_status(); - if ($connectionStatus !== 0) { + if ($connectionStatus !== CONNECTION_NORMAL) { throw new ConnectionLostException("Connection lost. Status: $connectionStatus"); } }