Update spx-upload-mimes.php#32
Conversation
Signed-off-by: Max Barrett <34328348+MaximillianGroup@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the SPX Upload MIME Types must-use plugin from a procedural implementation with anonymous closures into a namespaced, final class UploadMimeTypes. It encapsulates the MIME configurations as private class constants, uses static methods for hook callbacks, and introduces proper type-hinting (including a nullable $mimes parameter to prevent fatal errors during REST media uploads). Feedback suggests adding defensive checks using is_file() and is_readable() on the temporary file path before attempting to read it with finfo_file to prevent potential PHP warnings.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| // Fail secure: without fileinfo we cannot verify contents, so leave | ||
| // the default rejection in place rather than override blindly. | ||
| if ( ! \function_exists( 'finfo_open' ) ) { | ||
| return $data; | ||
| } | ||
|
|
There was a problem hiding this comment.
To prevent potential PHP warnings (e.g., finfo_file(): Failed to open stream: No such file or directory) when the temporary file path $file does not exist or is not readable, it is highly recommended to perform defensive checks using is_file() and is_readable() before attempting to open and read the file with finfo.
// Fail secure: without fileinfo we cannot verify contents, so leave
// the default rejection in place rather than override blindly.
if ( ! \function_exists( 'finfo_open' ) ) {
return $data;
}
if ( ! \is_file( $file ) || ! \is_readable( $file ) ) {
return $data;
}
$finfo = \finfo_open( \FILEINFO_MIME_TYPE );There was a problem hiding this comment.
@copilot - can you make this correction please - To prevent potential PHP warnings (e.g., finfo_file(): Failed to open stream: No such file or directory) when the temporary file path $file does not exist or is not readable, it is highly recommended to perform defensive checks using is_file() and is_readable() before attempting to open and read the file with finfo.
// Fail secure: without fileinfo we cannot verify contents, so leave
// the default rejection in place rather than override blindly.
if ( ! \function_exists( 'finfo_open' ) ) {
return $data;
}
if ( ! \is_file( $file ) || ! \is_readable( $file ) ) {
return $data;
}
$finfo = \finfo_open( \FILEINFO_MIME_TYPE );
There was a problem hiding this comment.
Pull request overview
This PR refactors the spx-upload-mimes.php MU-plugin to encapsulate its MIME allowlist + fileinfo-override logic into a namespaced class with static callbacks, while preserving the existing “fail-secure” content verification behavior for managed extensions.
Changes:
- Moved the MIME allowlist and fileinfo variant tables into
UploadMimeTypesasprivateclass constants and registered hooks via named static methods. - Prefixed WordPress/PHP global function and constant calls with
\for clarity and to avoid namespace resolution overhead/ambiguity. - Updated docblock/background explanation and adjusted the
wp_check_filetype_and_extcallback signature to accept nullable$mimes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Two things to flag:
First — the one thing I need you to verify: are SPX_EXTRA_MIMES or SPX_FINFO_VARIANTS referenced from any other file? The docblock calls them a "single source of truth," which hints they might be. I made them private const inside the class (the correct default for encapsulation). If something else reads them, tell me and I'll either make them public class constants (UploadMimeTypes::EXTRA_MIMES) or keep global back-compat aliases.
Second — a subtlety that bites people the moment they namespace WP code: once you're in a namespace, unqualified calls fall back to global for functions and constants but not for classes. Your file uses procedural finfo_open() etc. (functions, so they'd still resolve), but I prefixed every global call with \ anyway — \add_filter, \finfo_open, \FILEINFO_MIME_TYPE. That's the best-practice convention for namespaced WP code: it skips the namespace-fallback lookup and makes it unambiguous that you're calling core, not something local. It reads as noise at first; it's the correct habit.
Type of change
Affected components
nginx/— Nginx perimeter layervarnish/— Varnish cache layerapache/— Apache application layervar/www/html/wp-content/mu-plugins/— WordPress MU plugins.github/— Repository governance / CIValidation checklist
All items must be checked before requesting review.
nginx -tpasses (or Docker equivalent — see CONTRIBUTING.md)varnishd -C -f varnish/default.vclpasses (or Docker equivalent)apachectl -tpasses (or Docker equivalent)php -lpasses for any modified PHP filesspx-cloudflare-trust.confor the$from_cloudflaregeo blockTesting notes
Risk assessment
Reload or restart required:
nginx -s reloadsystemctl reload varnishsystemctl reload apache2Rollback notes
CHANGELOG entry