-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Trac 64439: Add theme download capability to Theme Editor #10731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from all commits
55f37f6
2029431
5571e67
1c6e301
8a7c128
e3e909d
b260e24
0b5b476
001683d
458f3a8
fa4a308
011801e
945ae0e
7c48a79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -77,6 +77,86 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| wp_die( __( 'The requested theme does not exist.' ) . ' ' . $theme->errors()->get_error_message() ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle theme download action: create a zip of the theme and send it to the browser. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( 'download_theme' === $action ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( ! current_user_can( 'edit_themes' ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| wp_die( '<p>' . __( 'Sorry, you are not allowed to download themes for this site.' ) . '</p>' ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Verify nonce. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( ! check_admin_referer( 'download-theme_' . $stylesheet ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| wp_die( '<p>' . __( 'Security check failed.' ) . '</p>' ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $theme_dir = $theme->get_stylesheet_directory(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( ! is_dir( $theme_dir ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| wp_die( '<p>' . __( 'Theme directory not found.' ) . '</p>' ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $zipname = $stylesheet; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $version = $theme->get( 'Version' ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( $version ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $zipname .= '.' . $version; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $tmpfile = get_temp_dir() . DIRECTORY_SEPARATOR . $zipname . '-' . time() . '.zip'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Attempt to extend execution time to allow large theme archives to be created. | |
| if ( function_exists( 'set_time_limit' ) ) { | |
| @set_time_limit( 300 ); | |
| } |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of getRealPath() without checking for failure could allow symlinks to be followed outside the theme directory, creating a security risk. If getRealPath() returns false (which can happen for broken symlinks), this will cause issues. Additionally, symlinks pointing outside the theme directory could be included in the archive. Consider checking the return value of getRealPath() and validating that the resolved path is still within the theme directory using a path containment check (e.g., checking if the resolved path starts with the theme directory path).
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ZipArchive::addFile() method can fail silently if a file is not readable or if there are permission issues, but there's no error checking after each file is added. This means some files could be skipped without any indication to the user. Consider checking the return value of addFile() and either logging errors or tracking the count of successfully added files to ensure the archive is complete.
| $zip->addFile( $file_path, $stylesheet . '/' . $relative_path ); | |
| if ( true !== $zip->addFile( $file_path, $stylesheet . '/' . $relative_path ) ) { | |
| $zip->close(); | |
| // Remove partially created archive. | |
| @unlink( $tmpfile ); | |
| wp_die( | |
| '<p>' . sprintf( | |
| /* translators: %s: File path. */ | |
| __( 'Could not add file to zip archive: %s' ), | |
| esc_html( $relative_path ) | |
| ) . '</p>' | |
| ); | |
| } |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the ZipArchive creation or file addition fails silently (e.g., due to permissions issues or disk space), the code continues to line 146 without checking if files were actually added to the archive. This could result in downloading an empty or incomplete zip file. Consider checking the number of files added to the archive or validating the zip file size before sending it to the user.
| $files = new RecursiveIteratorIterator( | |
| new RecursiveDirectoryIterator( $theme_dir, FilesystemIterator::SKIP_DOTS ), | |
| RecursiveIteratorIterator::LEAVES_ONLY | |
| ); | |
| foreach ( $files as $file ) { | |
| // With SKIP_DOTS and LEAVES_ONLY, we don't need to check isDir(). | |
| $file_path = $file->getRealPath(); | |
| $relative_path = substr( $file_path, strlen( $theme_dir ) + 1 ); | |
| $zip->addFile( $file_path, $stylesheet . '/' . $relative_path ); | |
| } | |
| $files = new RecursiveIteratorIterator( | |
| new RecursiveDirectoryIterator( $theme_dir, FilesystemIterator::SKIP_DOTS ), | |
| RecursiveIteratorIterator::LEAVES_ONLY | |
| ); | |
| $files_added = 0; | |
| foreach ( $files as $file ) { | |
| // With SKIP_DOTS and LEAVES_ONLY, we don't need to check isDir(). | |
| $file_path = $file->getRealPath(); | |
| $relative_path = substr( $file_path, strlen( $theme_dir ) + 1 ); | |
| if ( ! $zip->addFile( $file_path, $stylesheet . '/' . $relative_path ) ) { | |
| $zip->close(); | |
| wp_die( '<p>' . __( 'Could not create zip archive.' ) . '</p>' ); | |
| } | |
| $files_added++; | |
| } | |
| if ( 0 === $files_added ) { | |
| $zip->close(); | |
| wp_die( '<p>' . __( 'Could not create zip archive.' ) . '</p>' ); | |
| } |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of getRealPath() without checking for failure could cause issues if it returns false (which can happen for broken symlinks). Additionally, symlinks pointing outside the theme directory could be included in the archive, creating a security risk. Consider checking the return value of getRealPath() and validating that the resolved path is still within the theme directory using a path containment check.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an error occurs during zip creation (lines 108-143), the temporary file may be left on the server because there's no cleanup in the error paths. The wp_die() calls at lines 109 and 142 exit without deleting the partially created zip file. Consider adding cleanup code before wp_die() calls or using a try-finally pattern to ensure temporary files are always removed, even on error.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Content-Disposition header is missing proper quotes escaping. While sanitize_file_name() removes most problematic characters, the filename is already wrapped in double quotes in the header. If the sanitized filename contains a quote character or other special characters that survive sanitization, it could break the header format. Consider using RFC 6266 compliant filename encoding or additional escaping to ensure the header is properly formatted.
| header( 'Content-Disposition: attachment; filename="' . sanitize_file_name( $zipname ) . '.zip"' ); | |
| $download_filename = sanitize_file_name( $zipname ) . '.zip'; | |
| $header_filename = str_replace( array( '\\\\', '"' ), array( '\\\\\\\\', '\\"' ), $download_filename ); | |
| header( 'Content-Disposition: attachment; filename="' . $header_filename . '"' ); |
Uh oh!
There was an error while loading. Please reload this page.