From 33c0091358bb6e927d0339bef6f5f91ff24d8aff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <323649642+oc-tmueller@users.noreply.github.com> Date: Wed, 16 Sep 2026 12:33:25 +0200 Subject: [PATCH 1/2] fix: release the stream when a bitmap preview cannot be decoded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bitmap::getThumbnail() opened the file and closed it only on the success path. The catch around getResizedPreview() returned without closing, so every failed decode leaked one file descriptor for the lifetime of the process. A preview pre-generation run or a cron preview job over a directory of files ImageMagick has no coder for exhausts the descriptors one file at a time. The open was also unchecked. A storage that cannot open the file returns false rather than throwing, and stream_get_contents(false) raises a TypeError - an \Error, so it escapes the \Exception handler directly underneath and surfaces as a 500 instead of the missing preview every other failure here degrades to. Closing moves into a finally block, and a false return from fopen() is handled explicitly. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --- lib/private/Preview/Bitmap.php | 12 +++- tests/lib/Preview/BitmapStreamTest.php | 86 ++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 tests/lib/Preview/BitmapStreamTest.php diff --git a/lib/private/Preview/Bitmap.php b/lib/private/Preview/Bitmap.php index b7f783c5cb38..ac7b3ea08f11 100644 --- a/lib/private/Preview/Bitmap.php +++ b/lib/private/Preview/Bitmap.php @@ -45,6 +45,12 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) { return false; } $stream = $file->fopen('r'); + if ($stream === false) { + // stream_get_contents() below would raise a TypeError, which is an \Error and + // so would escape the handler underneath rather than degrade to no preview + Util::writeLog('core', 'Could not open ' . $file->getPath() . ' for a preview', Util::ERROR); + return false; + } // Creates \Imagick object from bitmap or vector file try { @@ -52,10 +58,12 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) { } catch (\Exception $e) { Util::writeLog('core', 'ImageMagick says: ' . $e->getmessage(), Util::ERROR); return false; + } finally { + // also on the failure path: any content ImageMagick has no coder for lands + // here, so leaking the handle would be routine rather than exceptional + \fclose($stream); } - \fclose($stream); - //new bitmap image object $image = new \OC_Image(); $image->loadFromData((string)$bp); diff --git a/tests/lib/Preview/BitmapStreamTest.php b/tests/lib/Preview/BitmapStreamTest.php new file mode 100644 index 000000000000..1ed03f9b176c --- /dev/null +++ b/tests/lib/Preview/BitmapStreamTest.php @@ -0,0 +1,86 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @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 Test\Preview; + +use OC\Preview\Photoshop; +use OCP\Files\File; +use Test\TestCase; + +/** + * @requires extension imagick + */ +class BitmapStreamTest extends TestCase { + /** + * @return array{0: File, 1: resource} + */ + private function makeFile(string $content): array { + $stream = \fopen('php://memory', 'rb+'); + \fwrite($stream, $content); + \rewind($stream); + $file = $this->createMock(File::class); + $file->method('fopen')->willReturn($stream); + $file->method('getSize')->willReturn(\strlen($content)); + $file->method('getPath')->willReturn('/test/bitmap-stream'); + return [$file, $stream]; + } + + /** + * getResizedPreview() throwing is a routine outcome rather than an exceptional one - + * any content ImageMagick has no coder for reaches it - so the handle has to be + * released on that path too. Otherwise a preview pre-generation run, or a cron + * preview job, over a directory of undecodable files exhausts the process's + * descriptors one file at a time. + */ + public function testClosesTheStreamWhenDecodingThrows(): void { + list($file, $stream) = $this->makeFile('x'); + + $result = (new Photoshop())->getThumbnail($file, 32, 32, false); + + $this->assertFalse($result, 'undecodable content must not produce a preview'); + $this->assertFalse(\is_resource($stream), 'the stream must be closed on the failure path'); + } + + public function testClosesTheStreamOnSuccess(): void { + $png = \file_get_contents(\OC::$SERVERROOT . '/tests/data/testimage.png'); + list($file, $stream) = $this->makeFile($png); + + $result = (new Photoshop())->getThumbnail($file, 32, 32, false); + + $this->assertNotFalse($result, 'a PNG should still decode'); + $this->assertFalse(\is_resource($stream), 'the stream must be closed on the success path'); + } + + /** + * A storage that cannot open the file returns false rather than throwing, and + * stream_get_contents(false) raises a TypeError - an \Error, so it would escape the + * \Exception handler in getThumbnail() and surface as a 500 instead of a missing + * preview. + */ + public function testReturnsFalseWhenTheFileCannotBeOpened(): void { + $file = $this->createMock(File::class); + $file->method('fopen')->willReturn(false); + $file->method('getSize')->willReturn(1024); + $file->method('getPath')->willReturn('/test/unopenable'); + + $this->assertFalse((new Photoshop())->getThumbnail($file, 32, 32, false)); + } +} From 7bcca418c5604f5296e17dfe4d6767331ce36ca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <323649642+oc-tmueller@users.noreply.github.com> Date: Wed, 16 Sep 2026 12:34:04 +0200 Subject: [PATCH 2/2] docs: add changelog entry for the bitmap preview stream leak (#41835) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --- changelog/unreleased/41835 | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changelog/unreleased/41835 diff --git a/changelog/unreleased/41835 b/changelog/unreleased/41835 new file mode 100644 index 000000000000..d60f03a150fb --- /dev/null +++ b/changelog/unreleased/41835 @@ -0,0 +1,11 @@ +Bugfix: Release the file handle when a bitmap preview cannot be decoded + +Bitmap previews closed the file they had opened only when decoding succeeded, so +every file that could not be decoded leaked a file handle for the lifetime of +the process. Generating previews for a directory of files that ImageMagick has +no decoder for could therefore exhaust the available file handles. + +Bitmap previews for a file that cannot be opened at all now report no preview +instead of failing the whole request. + +https://github.com/owncloud/core/pull/41835