Check cache file validation #6149
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds validation for downloaded files before they are cached to prevent corrupt or invalid files from being stored in the cache, addressing issue #6148. The validation includes basic file existence and size checks, as well as ZIP file integrity verification using the ZipArchive class.
Key Changes
- Added a new
validate_downloaded_file()method that performs file validation before caching - Integrated validation into the
filter_http_response()method to check files before they're imported into the cache - Added ZIP file-specific validation to ensure archive integrity
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( ( 'zip' === $ext || 'application/zip' === $mime ) && class_exists( '\ZipArchive' ) ) { | ||
| $zip = new \ZipArchive(); | ||
| $result = $zip->open( $file ); | ||
| if ( true !== $result ) { | ||
| return false; | ||
| } | ||
| // Optional deeper check: ensure we can read file list. | ||
| if ( 0 === $zip->numFiles ) { //phpcs:ignore | ||
| $zip->close(); | ||
| return false; | ||
| } | ||
| $zip->close(); | ||
| } |
There was a problem hiding this comment.
The resource leak can occur if $zip->open() succeeds but $zip->numFiles is 0. The $zip->close() is only called in the zero-files case, but not when the function returns true. This creates a resource leak where the ZipArchive remains open.
Consider closing the zip file in all code paths:
if ( ( 'zip' === $ext || 'application/zip' === $mime ) && class_exists( '\ZipArchive' ) ) {
$zip = new \ZipArchive();
$result = $zip->open( $file );
if ( true !== $result ) {
return false;
}
// Optional deeper check: ensure we can read file list.
$is_valid = $zip->numFiles > 0;
$zip->close();
if ( ! $is_valid ) {
return false;
}
}Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This reverts commit 65424a8.
Description :-