fix: return no preview when a preview file cannot be opened or read - #41855
Merged
Merged
Conversation
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.
An unopenable file 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. #41835 fixed this shape in Bitmap; SVG never got
the same treatment.
The guard tests is_resource() rather than comparing against false, because
View::fopen() returns null - not false - for a path isForbiddenFileOrDir()
rejects and for one Filesystem::resolvePath() finds no storage for.
Bitmap::getThumbnail() had that same narrow check, so it is widened here too:
there the null slips past the guard into stream_get_contents() and then into
fclose() in the finally, two uncatchable TypeErrors instead of one.
The read itself can also 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.
SVG's tests 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 wrapper that throws from stream_read(), since a plain fopen()
of an unreadable path fails at open time instead, and asserts release through
the caller's own handle rather than by counting /proc/self/fd, which is
Linux-only. Every case confirmed failing first, including the new null row
against Bitmap's old guard.
A 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.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
|
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. |
Written for admins rather than reviewers: no PHP type names, no mention of which handler swallowed what. Three things the first draft got wrong about its own PR. The title said "cannot be opened or read", promising a behaviour change on the read path that was never broken - a failing read already produced the icon, it just held the handle. The leak paragraph followed a sentence about the bitmap providers without naming a provider, so it read as covering them too, when #41835 had already put their fclose() in a finally and the leak fixed here is SVG-only. And the entry described only the old behaviour, where changelog/TEMPLATE and the sibling entries pair that with a statement of what now happens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
phil-davis
approved these changes
Sep 24, 2026
phil-davis
left a comment
Contributor
There was a problem hiding this comment.
Looks OK. Some comments are a bit tortuous to read.
4 tasks
oc-tmueller
added a commit
that referenced
this pull request
Sep 24, 2026
…iews [10.16] Backport of #41827. That PR carries the per-provider Imagick coder pin from #41834 as well, because #41834 was merged into its branch, so the squash commit on master contains both changes and so does this backport. Bitmap::getResizedPreview() sanitized SVG content before handing it to Imagick::readImageBlob(), but fell back to the ORIGINAL, unsanitized bytes whenever the sanitizer returned an empty string - which it does for any content libxml cannot parse, not only for genuinely malformed SVG. A malformed SVG, or any non-XML payload such as a raw MVG script, therefore reached ImageMagick unsanitized, where an <image xlink:href="MSL:..."> or an MVG "fill 'url(...)'" primitive can execute an MSL script that reads and writes arbitrary files as the web user. getResizedPreview() now rejects content whose libmagic-detected media type is text/*, image/svg*, application/xml or image/x-mvg before calling into Imagick at all, and each provider pins the exact coder it serves instead of letting ImageMagick re-derive the format from the content. Adapted for PHP 7.4, the only version 10.16 supports. Master justifies several of these guards by PHP 8 raising an \Error that escapes catch (\Exception); on 7.4 the same calls only warn, so every such claim was re-derived on the target runtime rather than carried over: - finfo_buffer(false, ...) warns and returns false on 7.4, and detectString() returned that false to the new deny-list, where it collapses to '' and matches no entry. Here the missing guard admitted the content the list exists to reject; it is on PHP 8 that it turns a missing preview into a 500. - the same holds for popen()/fgets()/pclose() in detect() and for fopen(false, ...) in the branch taken without ext-fileinfo. - the (string) cast on $file->getMimeType() is required on 7.4 too: passing null to a userland string-typed parameter is a TypeError on 7.4 as well. Measured, because the surrounding guards are not. 10.16 keeps its own "$stream === false" check and $image->loadFromData($bp); the is_resource() form and the (string) cast on that call are master-only, from #41855 and #41449, and the three-way merge preserved both correctly. The measurements the coder-pin comments rest on were re-taken on owncloudci/php:7.4, this branch's own CI image. It ships the same ImageMagick 6.9.11-60 and Ghostscript 9.55.0 as the 8.3 image and every figure reproduced: plain PostScript and EPSF-branded content both render 612x792 unpinned, pinned EPS and pinned PS alike; a %!PS-Adobe payload read unpinned reaches the PS coder at 612x792 while the TTF pin gives 800x480. That image also registers no SVG coder and no HEIF coder distinct from HEIC - which is precisely what PDFTest's old SVG-based guard got wrong and what Heic's single HEIC pin is there for. Verified in owncloudci/php:7.4: tests/lib/Preview/ plus tests/lib/Files/Type/DetectionTest.php at 68 tests / 207 assertions, against 41 tests / 123 assertions before, with 12 environment skips (Movie, Office, SVG). The PDF cases run here for the first time. php -l clean under 7.4 on all 21 changed files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
oc-tmueller
added a commit
that referenced
this pull request
Sep 24, 2026
…e [10.16] Backport of #41855, folded into this backport at the maintainer's request rather than opened as its own 10.16 PR. SVG::getThumbnail() read the handle from $file->fopen('r') without checking it, and released it only on the success path. Bitmap::getThumbnail() checked for false but not for null. Both now use !is_resource(), and the SVG provider closes the handle in a finally so a read that throws from the wrapper stack - the encryption module does, on a missing or damaged key - cannot hold the descriptor and the view's shared lock for the rest of the request. Reworded and re-tested for PHP 7.4, where the consequence differs from master's: - master's changelog and comments say an unopened file made the request fail with a server error. On 7.4 it does not. stream_get_contents() warns and hands on false, the prefix check turns that into a bare XML declaration, and Imagick rejects it - so the preview already degraded to a media type icon, after two misleading log lines. It is on PHP 8 that those calls raise a TypeError, an \Error that escapes the catch (\Exception) as a 500. The changelog now describes the logging noise, which is what this fix removes here. - SVGStreamTest's unopenable case therefore asserts that no warning is emitted, not that the return value is false: master's assertFalse() passes with the guard reverted on this runtime, so it could never go red. Renamed accordingly, and the handler honours error_reporting() so that diagnostics the code under test silences with @ cannot fail it, the same line OC\Log\ErrorHandler::onError() draws. Verified red before green - see the PR description for the counts. - BitmapStreamTest's equivalent case keeps its existing 7.4 assertion and gains #41855's data provider, so the null handle is covered here too. Verified in owncloudci/php:7.4: tests/lib/Preview/ plus tests/lib/Files/Type/DetectionTest.php at 72 tests / 219 assertions / 0 failures, 12 environment skips (Movie, Office, SVG - this image registers no SVG coder). php -l clean under 7.4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
oc-tmueller
added a commit
that referenced
this pull request
Sep 25, 2026
…iews [10.16] (OC10-164) (#41863) * fix: prevent arbitrary file write via unsanitized SVG/MVG bitmap previews [10.16] Backport of #41827. That PR carries the per-provider Imagick coder pin from #41834 as well, because #41834 was merged into its branch, so the squash commit on master contains both changes and so does this backport. Bitmap::getResizedPreview() sanitized SVG content before handing it to Imagick::readImageBlob(), but fell back to the ORIGINAL, unsanitized bytes whenever the sanitizer returned an empty string - which it does for any content libxml cannot parse, not only for genuinely malformed SVG. A malformed SVG, or any non-XML payload such as a raw MVG script, therefore reached ImageMagick unsanitized, where an <image xlink:href="MSL:..."> or an MVG "fill 'url(...)'" primitive can execute an MSL script that reads and writes arbitrary files as the web user. getResizedPreview() now rejects content whose libmagic-detected media type is text/*, image/svg*, application/xml or image/x-mvg before calling into Imagick at all, and each provider pins the exact coder it serves instead of letting ImageMagick re-derive the format from the content. Adapted for PHP 7.4, the only version 10.16 supports. Master justifies several of these guards by PHP 8 raising an \Error that escapes catch (\Exception); on 7.4 the same calls only warn, so every such claim was re-derived on the target runtime rather than carried over: - finfo_buffer(false, ...) warns and returns false on 7.4, and detectString() returned that false to the new deny-list, where it collapses to '' and matches no entry. Here the missing guard admitted the content the list exists to reject; it is on PHP 8 that it turns a missing preview into a 500. - the same holds for popen()/fgets()/pclose() in detect() and for fopen(false, ...) in the branch taken without ext-fileinfo. - the (string) cast on $file->getMimeType() is required on 7.4 too: passing null to a userland string-typed parameter is a TypeError on 7.4 as well. Measured, because the surrounding guards are not. 10.16 keeps its own "$stream === false" check and $image->loadFromData($bp); the is_resource() form and the (string) cast on that call are master-only, from #41855 and #41449, and the three-way merge preserved both correctly. The measurements the coder-pin comments rest on were re-taken on owncloudci/php:7.4, this branch's own CI image. It ships the same ImageMagick 6.9.11-60 and Ghostscript 9.55.0 as the 8.3 image and every figure reproduced: plain PostScript and EPSF-branded content both render 612x792 unpinned, pinned EPS and pinned PS alike; a %!PS-Adobe payload read unpinned reaches the PS coder at 612x792 while the TTF pin gives 800x480. That image also registers no SVG coder and no HEIF coder distinct from HEIC - which is precisely what PDFTest's old SVG-based guard got wrong and what Heic's single HEIC pin is there for. Verified in owncloudci/php:7.4: tests/lib/Preview/ plus tests/lib/Files/Type/DetectionTest.php at 68 tests / 207 assertions, against 41 tests / 123 assertions before, with 12 environment skips (Movie, Office, SVG). The PDF cases run here for the first time. php -l clean under 7.4 on all 21 changed files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * test: make BitmapStreamTest survive the OC10-164 coder pin [10.16] Backport of #41838. Without it the previous commit leaves this file red: testClosesTheStreamOnSuccess fed a PNG through the Photoshop provider, and the PSD pin refuses exactly that mismatch. The fixture is replaced by a PSD Imagick writes itself, so the case needs no fixture and cannot skip - it must not, since the success-path fclose() assertion is the whole subject of the file. The undecodable payload becomes bytes no coder claims, with a control assertion mirroring isDangerousToDecode(): both existing assertions are satisfied by any early return, so a build whose libmagic read the old payload as text/* would have had it refused at the new mime gate, stayed green, and silently stopped covering the decode. Measured on owncloudci/php:7.4 as application/octet-stream, which does reach the decode. Diverges from master in three places, all because this branch is PHP 7.4: - the unopenable-file case keeps its 10.16 shape, asserting that no warning is emitted. Master asserts the return value, which cannot fail here: stream_get_contents(false) only warns on 7.4 and the result is false either way, so the returned value cannot tell an unopenable file from an undecodable one. Only the warning can. - master's note that an unstubbed getMimeType() mock yields a TypeError does not hold on this branch: getThumbnail() casts the value, so an unstubbed mock gives ''. That is worse rather than better for a test - seven of the eight providers answer '' with the same constant they answer anything with, so the case would pass while proving nothing about which coder ran. Font is the one provider that branches on the mime type. The comment says that instead. - owncloudci/php:7.4 rather than :8.3 named as the build with no SVG renderer, confirmed on it. Verified in owncloudci/php:7.4: 3 tests / 7 assertions / 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * fix: report a preview file that cannot be opened without logging noise [10.16] Backport of #41855, folded into this backport at the maintainer's request rather than opened as its own 10.16 PR. SVG::getThumbnail() read the handle from $file->fopen('r') without checking it, and released it only on the success path. Bitmap::getThumbnail() checked for false but not for null. Both now use !is_resource(), and the SVG provider closes the handle in a finally so a read that throws from the wrapper stack - the encryption module does, on a missing or damaged key - cannot hold the descriptor and the view's shared lock for the rest of the request. Reworded and re-tested for PHP 7.4, where the consequence differs from master's: - master's changelog and comments say an unopened file made the request fail with a server error. On 7.4 it does not. stream_get_contents() warns and hands on false, the prefix check turns that into a bare XML declaration, and Imagick rejects it - so the preview already degraded to a media type icon, after two misleading log lines. It is on PHP 8 that those calls raise a TypeError, an \Error that escapes the catch (\Exception) as a 500. The changelog now describes the logging noise, which is what this fix removes here. - SVGStreamTest's unopenable case therefore asserts that no warning is emitted, not that the return value is false: master's assertFalse() passes with the guard reverted on this runtime, so it could never go red. Renamed accordingly, and the handler honours error_reporting() so that diagnostics the code under test silences with @ cannot fail it, the same line OC\Log\ErrorHandler::onError() draws. Verified red before green - see the PR description for the counts. - BitmapStreamTest's equivalent case keeps its existing 7.4 assertion and gains #41855's data provider, so the null handle is covered here too. Verified in owncloudci/php:7.4: tests/lib/Preview/ plus tests/lib/Files/Type/DetectionTest.php at 72 tests / 219 assertions / 0 failures, 12 environment skips (Movie, Office, SVG - this image registers no SVG coder). php -l clean under 7.4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * docs: point the changelog entries at the reopened backport PR The backport PR was reopened as #41863, because #41828 had been opened under the wrong GitHub account and a PR's author cannot be changed. The three changelog entries linked #41828, which is now closed, so they move to #41863. The master PR links are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --------- Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
oc-tmueller
added a commit
that referenced
this pull request
Sep 25, 2026
Bump version.php to 11.0.1 and materialize the changelog fragments for the first patch release of the 11.0 line. $OC_Version becomes [11, 0, 1, 0]: the 4th digit is the internal DB-upgrade patch level, not the public patch number, and nothing in this release needs it moved. The 15 fragments in changelog/unreleased/ move into changelog/11.0.1_2026-09-25/ (git mv, so the renames stay tracked) and CHANGELOG.md is regenerated with calens. unreleased/ keeps its .gitkeep and is now empty, ready for the next cycle. Security: - #41827 reject SVG/script content before it reaches ImageMagick bitmap previews - #41834 pin the Imagick coder for each preview provider - #41856 prevent path traversal via appconfig public_/remote_ keys Bugfixes: - #41676 reduce priority of checkPropFind event - #41779 do not echo secrets when setting config values via occ - #41782 restore index usage for filecache writes on Oracle - #41807 show federated users in the share dialog when local users also match - #41808 avoid a deprecation notice when hashing the file cache path on Oracle - #41824 ship only the app payload in the release tarballs - #41835 release the file handle when a bitmap preview cannot be decoded - #41855 show a media type icon when a preview file cannot be opened - #41869 restrict federated address book sync to the trusted server Changes: - #41775 update PHP dependencies - #41785 require rhukster/dom-sanitizer as a tagged release - #41808 restore Oracle database support in the command line installer The #41824 fragment is carried over from #41825, which prepared this release on a release/v11.0.1 branch that the org maintenance ruleset freezes after its first push; that branch cannot take the changelog commit, so this supersedes it. Once merged, v11.0.1 gets tagged on the merged commit and the release bundles are built and published from owncloud/server-release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
oc-tmueller
added a commit
that referenced
this pull request
Sep 25, 2026
Bump version.php to 10.16.5 and materialize the changelog fragments for this release. $OC_Version becomes [10, 16, 5, 0]: the 4th digit is the internal DB-upgrade patch level, not the public patch number, and nothing in this release needs it moved. The 11 fragments in changelog/unreleased/ move into changelog/10.16.5_2026-09-25/ (git mv, so the renames stay tracked) and CHANGELOG.md is regenerated with calens. unreleased/ keeps its .gitkeep and is now empty, ready for the next cycle. Security: - #41784 update PHP dependencies to close published advisories - #41803 prevent path traversal via appconfig public_/remote_ keys - #41827 reject SVG/script content before it reaches ImageMagick bitmap previews - #41834 pin the Imagick coder for each preview provider Bugfixes: - #41782 restore index usage for filecache writes on Oracle - #41808 avoid a deprecation notice when hashing the file cache path on Oracle - #41814 show federated users in the share dialog when local users also match - #41819 speed up Oracle schema introspection - #41835 release the file handle when a bitmap preview cannot be decoded - #41855 report a preview file that cannot be opened without logging noise Changes: - #41788 update PHP dependencies Same shape as 852062d ("feat: release 10.16.4"), which did the changelog and the version bump in one commit. Once merged, v10.16.5 gets tagged on the merged commit and the release bundles are built and published from owncloud/server-release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
oc-tmueller
added a commit
that referenced
this pull request
Sep 25, 2026
* feat: release 10.16.5 Bump version.php to 10.16.5 and materialize the changelog fragments for this release. $OC_Version becomes [10, 16, 5, 0]: the 4th digit is the internal DB-upgrade patch level, not the public patch number, and nothing in this release needs it moved. The 11 fragments in changelog/unreleased/ move into changelog/10.16.5_2026-09-25/ (git mv, so the renames stay tracked) and CHANGELOG.md is regenerated with calens. unreleased/ keeps its .gitkeep and is now empty, ready for the next cycle. Security: - #41784 update PHP dependencies to close published advisories - #41803 prevent path traversal via appconfig public_/remote_ keys - #41827 reject SVG/script content before it reaches ImageMagick bitmap previews - #41834 pin the Imagick coder for each preview provider Bugfixes: - #41782 restore index usage for filecache writes on Oracle - #41808 avoid a deprecation notice when hashing the file cache path on Oracle - #41814 show federated users in the share dialog when local users also match - #41819 speed up Oracle schema introspection - #41835 release the file handle when a bitmap preview cannot be decoded - #41855 report a preview file that cannot be opened without logging noise Changes: - #41788 update PHP dependencies Same shape as 852062d ("feat: release 10.16.4"), which did the changelog and the version bump in one commit. Once merged, v10.16.5 gets tagged on the merged commit and the release bundles are built and published from owncloud/server-release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * fix: correct three defects in the 10.16.5 release notes Found reviewing the release commit, all three in text that ships inside the tarball, so this is the last point at which they are free to fix. **Six updated dependencies were unnamed.** Diffing composer.lock at v10.16.4 against this branch gives 17 changed production packages; the entry listed 11. Missing: sabre/dav (4.7.0 to 4.7.1), sabre/event (5.1.7 to 5.1.9), sabre/vobject (4.5.8 to 4.6.1), pimple/pimple (v3.6.1 to v3.6.2), nikic/php-parser (v5.7.0 to v5.9.0) and dg/composer-cleaner (v2.2.1 to v2.2.2). All but sabre/event are direct entries in composer.json's require, and sabre/vobject is a minor bump of the vCard/iCalendar parser behind CalDAV and CardDAV — an administrator auditing what moved in that stack for 10.16.5 would have seen nothing. Five of the six came from #41787, whose URL was missing from the entry as well; nikic/php-parser then went on to v5.9.0 in #41830, which was already listed. Earlier releases on this branch do list sabre bumps here (10.10.0, 10.11.0, 10.12.0), so the omission also broke the house convention. **Two entries linked a pull request that never reached this branch.** #41819 was merged into ci/oracle-db-in-github-actions-10.16, an intermediate branch that no longer exists on the remote; the change reached 10.16 through #41815 (9408736). Likewise the 41835 entry cited only master's #41835, while the 10.16 delivery was #41837 (6443822). Both now cite the 10.16 pull request first, matching what 41827, 41834 and 41855 already do with #41863 — so calens also makes the branch's own pull request the primary link. CHANGELOG.md is regenerated with calens and, as on the rest of this branch, written without a trailing newline: `ocrelease changelog` captures calens' stdout through execa, which strips it, and every released section on 10.16 was produced that way. Adding one here would only create churn at the next release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * chore: fold the federated address book sync fix into the 10.16.5 release notes Its fix merged into 10.16 after the release notes were first prepared, so its changelog fragment was still sitting in changelog/unreleased and would have been deferred to the next release while the fix itself shipped in this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --------- Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
oc-tmueller
added a commit
that referenced
this pull request
Sep 25, 2026
* feat: release 11.0.1 Bump version.php to 11.0.1 and materialize the changelog fragments for the first patch release of the 11.0 line. $OC_Version becomes [11, 0, 1, 0]: the 4th digit is the internal DB-upgrade patch level, not the public patch number, and nothing in this release needs it moved. The 15 fragments in changelog/unreleased/ move into changelog/11.0.1_2026-09-25/ (git mv, so the renames stay tracked) and CHANGELOG.md is regenerated with calens. unreleased/ keeps its .gitkeep and is now empty, ready for the next cycle. Security: - #41827 reject SVG/script content before it reaches ImageMagick bitmap previews - #41834 pin the Imagick coder for each preview provider - #41856 prevent path traversal via appconfig public_/remote_ keys Bugfixes: - #41676 reduce priority of checkPropFind event - #41779 do not echo secrets when setting config values via occ - #41782 restore index usage for filecache writes on Oracle - #41807 show federated users in the share dialog when local users also match - #41808 avoid a deprecation notice when hashing the file cache path on Oracle - #41824 ship only the app payload in the release tarballs - #41835 release the file handle when a bitmap preview cannot be decoded - #41855 show a media type icon when a preview file cannot be opened - #41869 restrict federated address book sync to the trusted server Changes: - #41775 update PHP dependencies - #41785 require rhukster/dom-sanitizer as a tagged release - #41808 restore Oracle database support in the command line installer The #41824 fragment is carried over from #41825, which prepared this release on a release/v11.0.1 branch that the org maintenance ruleset freezes after its first push; that branch cannot take the changelog commit, so this supersedes it. Once merged, v11.0.1 gets tagged on the merged commit and the release bundles are built and published from owncloud/server-release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * fix: name every updated PHP dependency in the 11.0.1 changelog composer/semver (3.4.4 to 3.5.0) and nikic/php-parser (v5.8.0 to v5.9.0) both moved since v11.0.0, both are direct entries in composer.json's require, and neither was named in the fragment. Diffing composer.lock at v11.0.0 against this branch gives 26 changed production packages; the fragment listed 24. Each was missed by the pass that bumped it: #41864 edited this fragment in the same commit that raised composer/semver, and #41829 did the same for nikic/php-parser. The list is not direct-only either — it already names transitive dependencies such as guzzlehttp/psr7 and symfony/mime — so both are omissions, not a scoping decision. An administrator reconciling the 11.0.1 notes against advisory ranges would read both as unchanged. The four remaining differences are require-dev only (myclabs/deep-copy, phpunit/phpunit, sebastian/exporter, sebastian/recursion-context) and stay out, matching the fragment's existing production-only scope; the tarball installs --no-dev. Also restores CHANGELOG.md's trailing newline. `ocrelease changelog` writes calens' stdout as captured by execa, which strips it; the last six "chore: update changelog" revisions of the file all end in one, and so does calens' own output, so the next regeneration would have put it back as a one-line no-op diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --------- Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SVG::getThumbnail()passed$file->fopen('r')straight intostream_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.
fopenyieldsfalse, andstream_get_contents(false)raises aTypeError. ATypeErroris an\Error, not an\Exception, so it went straight past the handler a few lines below.The guard tests
is_resource()rather than comparing againstfalse, becauseView::basicOperation()returnsnull— notfalse— for a pathisForbiddenFileOrDir()rejects and for oneFilesystem::resolvePath()finds no storage for.Bitmap::getThumbnail()had the same narrow=== falsecheck, so it is widened here too: there anullslips past the guard intostream_get_contents()and then intofclose()in thefinally, two uncatchableTypeErrors 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, butfclose()sat after the read and never ran, holding the descriptor and the shared lock theViewwrapper releases only on close. The read now has its owntry/finally, matchingBitmap.Tests
SVG's cases live in their own file rather than in
SVGTest, which extendsProvider, needs the database, and skips wherever ImageMagick registers no SVG coder —owncloudci/php:8.3included. Everything asserted happens before any SVG is decoded, so the cases run anywhereext-imagickis present.The leak case drives a userland stream wrapper that throws from
stream_read(), since a plainfopen()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.phpgives 2TypeErrors plus theis_resourcefailure, and the newnullrow fails againstBitmap's old guard.Verification
tests/lib/Preview/inowncloudci/php:8.3: 59 tests, 160 assertions, 0 failures, 18 skips (all pre-existing, for the missing SVG coder).php-cs-fixerclean over 2436 files.php -lclean under 7.4.Note for the 10.16 backport
On PHP 7.4
stream_get_contents(false)only warns and returnsfalse, and PHPUnit converts that warning intoPHPUnit\Framework\Error\Warning, which extends\Exceptionand 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,MP3andOfficehave nofopenguard either, andImage/TXThave notry/catchat all, so the same file still 500s forimage/*andtext/plain— the two most common preview types. Left out to keep this diff reviewable; happy to follow up.🤖 Generated with Claude Code