From 7c29de3faa2b06ed8f23bd799bb5c97bf84eb7f8 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 15 Jul 2026 00:13:30 +0200 Subject: [PATCH 1/3] fix(preview): properly handle encoded content Signed-off-by: Ferdinand Thiessen --- lib/private/Preview/SVG.php | 40 ++++++++++++++++++++++++++---- tests/lib/Preview/SVGTest.php | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/lib/private/Preview/SVG.php b/lib/private/Preview/SVG.php index 0400038d9802c..624cac2065ac6 100644 --- a/lib/private/Preview/SVG.php +++ b/lib/private/Preview/SVG.php @@ -45,15 +45,19 @@ public function getMimeType(): string { public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { try { $content = stream_get_contents($file->fopen('r')); - if (substr($content, 0, 5) !== '' . $content; + if ($content === false) { + return null; } - - // Do not parse SVG files with references - if (preg_match('/["\s](xlink:)?href\s*=/i', $content)) { + // check if the file can be processed by this provider + if (!$this->canBeProcessed($content)) { return null; } + $content = ltrim($content); + if (substr($content, 0, 5) !== '' . $content; + } + $svg = new \Imagick(); $svg->pingImageBlob($content); @@ -87,4 +91,30 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { } return null; } + + /** + * Check if the file can be processed by this provider, + * meaning the SVG is safe to be processed and does not contain any external references. + */ + protected function canBeProcessed(string $content): bool { + // check for allowed encodings and convert if necessary + $encoding = mb_detect_encoding($content, ['UTF-8', 'ISO-2022-JP', 'ISO-8859-1'], true); + if ($encoding === false) { + return false; + } elseif ($encoding !== 'UTF-8') { + $content = mb_convert_encoding($content, 'UTF-8', $encoding); + } + + // Strip all non-printable/control characters except newlines/tabs + $content = preg_replace('/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/', '', $content); + if ($content === null) { + return false; + } + + // check for any potential external reference (include custom namespace prefix) + if (preg_match('/["\s\']([a-z_][a-z0-9_.-]*:)?href\s*=/i', $content)) { + return false; + } + return true; + } } diff --git a/tests/lib/Preview/SVGTest.php b/tests/lib/Preview/SVGTest.php index 07e96eec9ab65..b41486f5b73f6 100644 --- a/tests/lib/Preview/SVGTest.php +++ b/tests/lib/Preview/SVGTest.php @@ -72,4 +72,50 @@ public function testGetThumbnailSVGHref(string $content): void { self::assertNull($this->provider->getThumbnail($file, 512, 512)); } + + #[\PHPUnit\Framework\Attributes\DataProvider('dataGetThumbnailSVGHrefNamespace')] + #[\PHPUnit\Framework\Attributes\RequiresPhpExtension('imagick')] + public function testGetThumbnailSvgHrefNamespace(string $namespace): void { + $handle = fopen('php://temp', 'w+'); + fwrite($handle, ' + +'); + rewind($handle); + + $file = $this->createMock(File::class); + $file->method('fopen') + ->willReturn($handle); + + self::assertNull($this->provider->getThumbnail($file, 512, 512)); + } + + public static function dataGetThumbnailSVGHrefNamespace(): array { + return [ + ['xlink'], + ['foo'], + ['_foo'], + ['fo_12'], + ['foo-bar'], + ['Fo_B1-ar'], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('dataGetThumbnailSvgEncoded')] + #[\PHPUnit\Framework\Attributes\RequiresPhpExtension('imagick')] + public function testGetThumbnailSvgEncoded(string $content): void { + $handle = fopen('php://temp', 'w+'); + fwrite($handle, $content); + rewind($handle); + + $file = $this->createMock(File::class); + $file->method('fopen') + ->willReturn($handle); + self::assertNull($this->provider->getThumbnail($file, 512, 512)); + } + + public static function dataGetThumbnailSvgEncoded(): array { + return [ + 'iso-2022-jp' => ["\n\n\n"], + ]; + } } From 9da2963b4cde025623e0ed1da8def2e162f01135 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sat, 18 Jul 2026 17:13:40 +0200 Subject: [PATCH 2/3] chore: adjust for old PHP Signed-off-by: Ferdinand Thiessen --- tests/lib/Preview/Provider.php | 11 ++++++++--- tests/lib/Preview/SVGTest.php | 16 +++++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/tests/lib/Preview/Provider.php b/tests/lib/Preview/Provider.php index a2e5170c82cd0..409ace14c4760 100644 --- a/tests/lib/Preview/Provider.php +++ b/tests/lib/Preview/Provider.php @@ -22,6 +22,9 @@ namespace Test\Preview; use OC\Files\Node\File; +use OC\Preview\Provider as PreviewProvider; +use OCP\Files\IRootFolder; +use OCP\Preview\IProviderV2; abstract class Provider extends \Test\TestCase { /** @var string */ @@ -30,7 +33,7 @@ abstract class Provider extends \Test\TestCase { protected $width; /** @var int */ protected $height; - /** @var \OC\Preview\Provider */ + /** @var IProviderV2|PreviewProvider */ protected $provider; /** @var int */ protected $maxWidth = 1024; @@ -137,8 +140,10 @@ protected function prepareTestFile($fileName, $fileContent) { * @return bool|\OCP\IImage */ private function getPreview($provider) { - $file = new File(\OC::$server->getRootFolder(), $this->rootView, $this->imgPath); - $preview = $provider->getThumbnail($file, $this->maxWidth, $this->maxHeight, $this->scalingUp); + $file = new File(\OC::$server->get(IRootFolder::class), $this->rootView, $this->imgPath); + $preview = ($provider instanceof PreviewProvider) + ? $provider->getThumbnail($file, $this->maxWidth, $this->maxHeight, $this->scalingUp, $this->rootView) + : $provider->getThumbnail($file, $this->maxWidth, $this->maxHeight, $this->scalingUp); if (get_class($this) === BitmapTest::class && $preview === null) { $this->markTestSkipped('An error occured while operating with Imagick.'); diff --git a/tests/lib/Preview/SVGTest.php b/tests/lib/Preview/SVGTest.php index b41486f5b73f6..3cf6aa0463b51 100644 --- a/tests/lib/Preview/SVGTest.php +++ b/tests/lib/Preview/SVGTest.php @@ -21,6 +21,8 @@ namespace Test\Preview; +use OCP\Files\File; + /** * Class SVGTest * @@ -66,15 +68,17 @@ public function testGetThumbnailSVGHref(string $content): void { '); rewind($handle); - $file = $this->createMock(\OCP\Files\File::class); + $file = $this->createMock(File::class); $file->method('fopen') ->willReturn($handle); self::assertNull($this->provider->getThumbnail($file, 512, 512)); } - #[\PHPUnit\Framework\Attributes\DataProvider('dataGetThumbnailSVGHrefNamespace')] - #[\PHPUnit\Framework\Attributes\RequiresPhpExtension('imagick')] + /** + * @dataProvider dataGetThumbnailSVGHrefNamespace + * @requires extension imagick + */ public function testGetThumbnailSvgHrefNamespace(string $namespace): void { $handle = fopen('php://temp', 'w+'); fwrite($handle, ' @@ -100,8 +104,10 @@ public static function dataGetThumbnailSVGHrefNamespace(): array { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('dataGetThumbnailSvgEncoded')] - #[\PHPUnit\Framework\Attributes\RequiresPhpExtension('imagick')] + /** + * @dataProvider dataGetThumbnailSvgEncoded + * @requires extension imagick + */ public function testGetThumbnailSvgEncoded(string $content): void { $handle = fopen('php://temp', 'w+'); fwrite($handle, $content); From 2927e383ceda9f7ab21773f24ebdbbf24e482074 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 21 Jul 2026 13:51:25 +0200 Subject: [PATCH 3/3] fix(Preview): reorder encodings for detection Signed-off-by: Ferdinand Thiessen --- lib/private/Preview/SVG.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Preview/SVG.php b/lib/private/Preview/SVG.php index 624cac2065ac6..3ebe0be163b03 100644 --- a/lib/private/Preview/SVG.php +++ b/lib/private/Preview/SVG.php @@ -98,7 +98,7 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { */ protected function canBeProcessed(string $content): bool { // check for allowed encodings and convert if necessary - $encoding = mb_detect_encoding($content, ['UTF-8', 'ISO-2022-JP', 'ISO-8859-1'], true); + $encoding = mb_detect_encoding($content, ['ISO-2022-JP', 'UTF-8', 'ISO-8859-1'], true); if ($encoding === false) { return false; } elseif ($encoding !== 'UTF-8') {