Repository navigation
Conversation
libmagic detects TrueType fonts as `font/ttf` since file 5.47, while `WP_Font_Utils::get_allowed_font_mime_types()` expects `font/sfnt`. PHP 8.6 bundles libmagic 5.48, and builds that link the system libmagic can use a newer one on older PHP versions, so TTF uploads to the Font Library were rejected. During font uploads, a `.ttf` file detected as either `font/sfnt` or `font/ttf` is now accepted. Props dhrupo. Fixes #66419.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
🟡 Changes recommended
The font MIME fallback can restore a policy-rejected TTF upload’s extension and type, bypassing MIME allowlist restrictions.
1 open issue
What changed in this PR
This PR updates Font Library uploads to accept TrueType fonts detected as font/ttf by newer libmagic versions, while retaining support for font/sfnt.
Changes:
- Adds a MIME compatibility callback for
.ttffiles. - Registers the callback only during Font Library uploads.
- Adds tests for both MIME types, rejected inputs, and already accepted files.
| File | Description |
|---|---|
| tests/phpunit/tests/fonts/font-library/wpFontUtils/allowTtfMimeTypes.php | Tests TTF MIME compatibility and rejection cases. |
| src/wp-includes/rest-api/endpoints/class-wp-rest-font-faces-controller.php | Scopes the compatibility filter to font uploads. |
| src/wp-includes/fonts/class-wp-font-utils.php | Adds the TTF MIME compatibility callback. |
Review effort: Balanced
desrosj
left a comment
desrosj
left a comment
There was a problem hiding this comment.
Thanks for this @dhrupo!
I've requested a change, and this will required some reworking of the unit tests.
It's important to note that this will only work if php/php-src#25051 is fixed in time for 8.6.0 general release. Otherwise ttf files will still fail.
It was noted that the issue should be fixed prior to 8.6 being branched, but that appears to have happened in the last 24 hours even though this was not fixed. I'm not clear on where this upstream fix stands in relation to 8.6.0 vs. 8.6.1.
There was a problem hiding this comment.
| 'ttf' => PHP_VERSION_ID >= 80600 ? 'font/ttf' : 'font/sfnt', |
I'd like to suggest using this approach to match the prior art instead of adding a new function that hooks into a filter.
While there is the possibility that a PHP < 8.6 build could use the system-installed version of libmagic instead (which could possibly be 5.47+), the same is true for the woff and woff2 file types below. A better solution can be explored separately and applied to all 3 types.
This will require the unit tests to be reworked. It seems there are currently no tests for WP_Font_Utils::get_allowed_mime_types(), though.
There was a problem hiding this comment.
Thanks @desrosj, applied in c9bc8ec. The wp_check_filetype_and_ext callback is gone, so the policy concern Copilot raised no longer applies, and the ttf entry now uses your suggested PHP_VERSION_ID check.
For the tests, I replaced the callback tests with Tests_Fonts_WpFontUtils_GetAllowedFontMimeTypes. It runs wp_check_filetype_and_ext() on the real OpenSans-Regular.* test fonts with the font MIME types and checks that each file is accepted with its allowed type, so a future libmagic change for any of the four types should show up there.
Results for the fonts and font-library groups (237 tests):
- PHP 7.4.33 and 8.5.12: pass.
- PHP 8.6.0-dev as bundled now (libmagic 5.48 without the fix): the
ttfcase andtest_create_item_with_multiple_font_filesfail, because finfo returnsapplication/octet-streamfor TTF. This is finfo detects TrueType fonts as application/octet-stream since libmagic 5.48 php/php-src#25051. - PHP-8.6 built with Fix GH-25051: Restore TrueType font MIME detection php/php-src#25063 applied: finfo returns
font/ttf, and all tests pass. With the trunk version of the map on the same build, thettfcase fails.
On the upstream status: php/php-src#25063 is still open against PHP-8.6, and the PHP-8.6.0 branch was created on November 3, so as far as I can tell it will be in 8.6.1 unless it is also merged into PHP-8.6.0.
Replace the `wp_check_filetype_and_ext` callback with a `PHP_VERSION_ID` check in `WP_Font_Utils::get_allowed_font_mime_types()`, matching the existing WOFF and WOFF2 entries. PHP 8.6 bundles a libmagic that reports TrueType fonts as `font/ttf` instead of `font/sfnt`. Add a test that checks the detected MIME type of each test font file against the allowed font MIME types. See #66419.
This comment has been minimized.
This comment has been minimized.
libmagic 5.47 changed the MIME type reported for TrueType fonts from `font/sfnt` to the canonical `font/ttf`. The libmagic bundled with PHP 8.6 includes this change, but PHP builds that use the system libmagic can report either type regardless of the PHP version. `wp_check_filetype_and_ext()` now accepts either type when the file's contents are detected as a TrueType font, and `WP_Font_Utils::get_allowed_font_mime_types()` returns `font/ttf` for TTF files on all PHP versions instead of switching on `PHP_VERSION_ID`. See #66419. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…letype_and_ext()`. Tests that a TrueType font is accepted whether `font/ttf` or `font/sfnt` is expected. Whichever type the local libmagic reports, one of the two cases expects the other, so the handling for both types is exercised on every PHP version. The test is skipped when libmagic reports something else for TrueType fonts, as it does on PHP 8.6.0 builds affected by php/php-src#25051. Also tests that a font of another format named with a `.ttf` extension is still rejected. See #66419. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
desrosj
left a comment
desrosj
left a comment
There was a problem hiding this comment.
A few small changes to the new test class.
As I was reviewing this, I decided that I'm not happy with the tradeoff of using a PHP version comparison here. The mime type returned is not necessarily tied to a specific PHP version because PHP can be compiled using any version of libmagic. This creates an edge case where a site using PHP <8.6 running libmagic >= 5.47 would not be able to upload TTF files at all.
I'm not sure how common this is in practice, but there is already an established pattern for handling scenarios like this found in wp_check_filetype_and_ext() where a handful of known alternate reported mime types for an extension are explicitly marked as acceptable.
In the interest in moving this forward so it can land before the final beta release this upcoming Tuesday, I've gone and made the following changes:
- Removed the version check in
WP_Font_Utilsand setttfto only return the correctedfont/ttfvalue. - Inserted a new conditional block that allows a
.ttffile to report as witherfont/ttforfont/sfntwithinwp_check_filetype_and_ext(). - A new test method that validates the added conditions for
.ttffiles.
Temporarily skip the new test for `ttf` files when the upstream bug is still present.
|
Thanks for reworking this, the |
… be detected. Some libmagic versions, such as the one bundled with PHP 8.6, cannot detect TrueType fonts and report them as `application/octet-stream`. This causes the TTF file to be rejected and the whole request in `test_create_item_with_multiple_font_files()` to fail with a 400. The TTF file is now left out of the upload when it cannot be detected, so the remaining formats are still tested. See php/php-src#25051. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
libmagic detects TrueType fonts as
font/ttfsince file 5.47, whileWP_Font_Utils::get_allowed_font_mime_types()expectsfont/sfnt, so TTF uploads to the Font Library are rejected withrest_font_upload_invalid_file_type. PHP 8.6 bundles libmagic 5.48, and builds that link the system libmagic can use a newer one on older PHP versions.This adds
WP_Font_Utils::allow_ttf_mime_types()onwp_check_filetype_and_extduring font uploads only, next to the existingupload_mimesfilter. It accepts a.ttffile detected as eitherfont/sfntorfont/ttf.application/octet-streamand other extensions stay rejected.Testing
Tests_Fonts_WpFontUtils_AllowTtfMimeTypes(--group 66419): fail on trunk (method missing),OK (7 tests, 9 assertions)on PHP 7.4.33, 8.5.12 and 8.6.0-dev.--group font-library:OK (204 tests, 919 assertions)on PHP 7.4.33 and 8.5.12.test_create_item_with_multiple_font_filesstill fails: that build reports TTF asapplication/octet-streambecause of an upstream libmagic regression (fixed in file/file@ba72caa54b, reported in finfo detects TrueType fonts as application/octet-stream since libmagic 5.48 php/php-src#25051). With the upstream fix the result isfont/ttf, which this change accepts.Trac ticket: https://core.trac.wordpress.org/ticket/66419
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Finding the issue by running the test suite on PHP 8.6, writing the change and tests, and running the checks above. I reviewed every line and checked the results myself.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.