fix: return no preview when a preview file cannot be opened or read

[oc-tmueller]

SVG::getThumbnail() passed $file->fopen('r') straight into stream_get_contents() without checking it, and closed the handle on the success path only. Both produced a 500 where the user should have seen a media-type icon.

This is the same defect #41835 fixed in Bitmap; SVG never got the same treatment. Split out of #41827 deliberately, so that security fix stays minimal and cherry-pickable.

The two failures

Unopenable file. fopen yields false, and stream_get_contents(false) raises a TypeError. A TypeError is an \Error, not an \Exception, so it went straight past the handler a few lines below.

The guard tests is_resource() rather than comparing against false, because View::basicOperation() returns null — not false — for a path isForbiddenFileOrDir() rejects and for one Filesystem::resolvePath() finds no storage for. Bitmap::getThumbnail() had the same narrow === false check, so it is widened here too: there a null slips past the guard into stream_get_contents() and then into fclose() in the finally, two uncatchable TypeErrors instead of one.

Unreadable file. The read itself can throw from the wrapper stack — the encryption module does exactly that on a missing or corrupt key. That is an \Exception, so it was caught and turned into "no preview" correctly, but fclose() sat after the read and never ran, holding the descriptor and the shared lock the View wrapper releases only on close. The read now has its own try/finally, matching Bitmap.

Tests

SVG's cases live in their own file rather than in SVGTest, which extends Provider, needs the database, and skips wherever ImageMagick registers no SVG coder — owncloudci/php:8.3 included. Everything asserted happens before any SVG is decoded, so the cases run anywhere ext-imagick is present.

The leak case drives a userland stream wrapper that throws from stream_read(), since a plain fopen() of an unreadable path fails at open time instead. It asserts release through the caller's own handle rather than by counting /proc/self/fd, which is Linux-only, and asserts the wrapper registered and yielded an open handle first so it cannot pass on a false premise.

Every case was confirmed failing first: reverting only SVG.php gives 2 TypeErrors plus the is_resource failure, and the new null row fails against Bitmap's old guard.

Verification

tests/lib/Preview/ in owncloudci/php:8.3: 59 tests, 160 assertions, 0 failures, 18 skips (all pre-existing, for the missing SVG coder). php-cs-fixer clean over 2436 files. php -l clean under 7.4.

Note for the 10.16 backport

On PHP 7.4 stream_get_contents(false) only warns and returns false, and PHPUnit converts that warning into PHPUnit\Framework\Error\Warning, which extends \Exception and is therefore swallowed by the handler in these methods — so the unopenable cases would pass there without the fix. The same trap applied to #41835's backport.

Known gap, not addressed here

Image, TXT, Movie, MP3 and Office have no fopen guard either, and Image/TXT have no try/catch at all, so the same file still 500s for image/* and text/plain — the two most common preview types. Left out to keep this diff reviewable; happy to follow up.

🤖 Generated with Claude Code

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[phil-davis]

Looks OK. Some comments are a bit tortuous to read.