feat: add file upload validation rules with WordPress integration#15
feat: add file upload validation rules with WordPress integration#15csemazharul wants to merge 6 commits intomainfrom
Conversation
- Add FileRule, ImageRule, MaxFileRule, MimesRule with $_FILES array and attachment ID support - Add EncodingRule (mb_detect_encoding), ExtensionsRule (wp_check_filetype), MimetypesRule (content-based via wp_check_filetype_and_ext) - Fix ImageRule: replace non-existent wp_get_image_mime_types() with wp_get_ext_types() + wp_get_mime_types(); use wp_check_filetype_and_ext for content-based verification instead of trusting user-controlled $_FILES['type'] - Fix MaxFileRule: use cached metadata['filesize'] instead of redundant filesize() calls - Extend BetweenRule and SizeRule with file size (KB) awareness; guard WP functions with function_exists() for non-WP environments - Fix Rule::setParameterValues to support multi-value single-key params (e.g. mimes:jpg,png,pdf) through the Validator pipeline - Extract getAttachmentSizeBytes() to Helpers trait, eliminating duplication across MaxFileRule, BetweenRule, SizeRule Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of file validation rules for WordPress, including checks for file type, size, image dimensions, and encoding. It also updates existing rules like BetweenRule and SizeRule to handle file inputs. Feedback focuses on improving performance by caching static MIME type lookups and sampling file contents for encoding detection, enhancing security by verifying MIME types from file content rather than client-provided headers, and ensuring consistent file size calculation logic across different rules.
| return false; | ||
| } | ||
|
|
||
| $contents = file_get_contents($filePath); |
There was a problem hiding this comment.
Loading the entire file content into memory using file_get_contents() can lead to memory exhaustion (OOM) errors, especially with large file uploads. Since mb_detect_encoding only needs a representative sample of the text to detect the encoding, consider reading only the first few kilobytes of the file to improve performance and stability.
$contents = file_get_contents($filePath, false, null, 0, 32768);| if (is_array($value) && isset($value['name']) && isset($value['type'])) { | ||
| return [ | ||
| 'name' => $value['name'], | ||
| 'type' => $value['type'], | ||
| ]; | ||
| } |
There was a problem hiding this comment.
The MimesRule currently trusts the type key from the $_FILES array, which is provided by the client and can be easily spoofed. To improve security and maintain consistency with ImageRule and MimetypesRule, you should use wp_check_filetype_and_ext() on the temporary file path to determine the actual MIME type based on the file's content.
if (is_array($value) && isset($value['name'], $value['tmp_name'])) {
$checked = wp_check_filetype_and_ext($value['tmp_name'], $value['name']);
return [
'name' => $value['name'],
'type' => $checked['type'] ?? '',
];
}| private function getImageMimeTypes(): array | ||
| { | ||
| $imageExtensions = wp_get_ext_types()['image'] ?? []; | ||
| $imageMimes = []; | ||
| foreach (wp_get_mime_types() as $exts => $mime) { | ||
| foreach (explode('|', $exts) as $ext) { | ||
| if (in_array($ext, $imageExtensions, true)) { | ||
| $imageMimes[] = $mime; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return array_values(array_unique($imageMimes)); | ||
| } |
There was a problem hiding this comment.
The getImageMimeTypes method performs multiple loops and calls several WordPress functions (wp_get_ext_types, wp_get_mime_types) every time a field is validated. Since this list of allowed image MIME types is static for the duration of the request, consider caching the result in a static variable to improve performance.
private function getImageMimeTypes(): array
{
static $imageMimes = null;
if ($imageMimes !== null) {
return $imageMimes;
}
$imageExtensions = wp_get_ext_types()['image'] ?? [];
$imageMimes = [];
foreach (wp_get_mime_types() as $exts => $mime) {
foreach (explode('|', $exts) as $ext) {
if (in_array($ext, $imageExtensions, true)) {
$imageMimes[] = $mime;
break;
}
}
}
$imageMimes = array_values(array_unique($imageMimes));
return $imageMimes;
}| $size = (int) $this->getParameter('size'); | ||
|
|
||
| if (is_array($value) && isset($value['size'])) { | ||
| return (int) round($value['size'] / 1024) === $size; |
There was a problem hiding this comment.
There is an inconsistency in how file sizes are calculated between SizeRule (which uses round()) and BetweenRule (which uses float comparison). This can lead to confusing results where a file might pass a between range but fail a size check that falls within that range. Consider using a consistent conversion logic across all file-related rules.
|
|
||
| $length = $this->getValueLength($value); | ||
| if (is_array($value) && isset($value['size'])) { | ||
| $sizeKB = (int) $value['size'] / 1024; |
There was a problem hiding this comment.
The (int) cast here is applied only to $value['size'] before the division, which is redundant as the size is already an integer. If the intention was to truncate the result of the division to an integer (KB), parentheses are missing. However, using floats for range comparison is generally more accurate. Also, note the inconsistency with SizeRule which uses round().
$sizeKB = $value['size'] / 1024;- EncodingRule: read only first 32KB via WP_Filesystem (falls back to file_get_contents) to prevent OOM on large files; rename helper to getFileContentForEncodingDetection for clarity - MimesRule: replace trust of user-controlled $_FILES['type'] with content-based wp_check_filetype_and_ext(), consistent with ImageRule - BetweenRule: align KB conversion to use (int) round() matching SizeRule, eliminating inconsistency flagged in review - README: document new file rules (extensions, mimetypes, encoding) and add file size notes to between/size rules Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…yle constraints - Rename ImageDimensionsRule -> DimensionRule (file and class) - Rename getImageDimensions() -> getDimensions() - Replace positional width,height params with key=value constraint syntax: min_width, max_width, min_height, max_height, width, height, ratio - ratio supports fraction (3/2) and float (1.5) with 0.01 tolerance - Fix EncodingRule: check WP_Filesystem() return before using $wp_filesystem to prevent fatal error when filesystem init fails - Update README: document new dimension rule syntax with examples Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
file,image,max_file,mimes,encoding,extensions,mimetypesbetweenandsizerules with file size (KB) awareness alongside their original string/int/array behaviorwp_check_filetype_and_ext,wp_get_ext_types,wp_get_mime_types,wp_get_attachment_metadata) instead of raw PHP where equivalents existKey fixes over the initial draft
ImageRule: replaced non-existentwp_get_image_mime_types()withwp_get_ext_types()['image']+wp_get_mime_types(); switched from trusting user-controlled$_FILES['type']to content-basedwp_check_filetype_and_ext()MaxFileRule: fixed dead-code branch (metadata['file']check) to usemetadata['filesize']via sharedgetAttachmentSizeBytes()helperRule::setParameterValues: fixed multi-value single-key rules (mimes:jpg,png,pdf) being silently dropped by the Validator pipeline due to array count mismatchBetweenRule/SizeRule: WP attachment path guarded withfunction_exists('get_attached_file')for non-WP environmentsTest plan
imagerule: acceptsimage/jpeg,image/png,image/gif; rejectsapplication/pdf,text/plainmax_filerule: accepts files within KB limit, rejects oversized filesmimesrule: matches by MIME type and extension fallbackencodingrule: detects encoding from file contentsextensionsrule: checks extension viawp_check_filetype()mimetypesrule: content-based MIME check viawp_check_filetype_and_ext()between/sizerules: existing string/int/array behavior unchanged; file size path exercised with$_FILESarrayfilerule: rejects missing keys, non-OK error codes, emptytmp_name🤖 Generated with Claude Code