[wp-trac] [WordPress Trac] #42562: media: wp_get_attachment_metadata() can return WP_Error object which leads to Fatal error

[WordPress Trac]

#42562: media: wp_get_attachment_metadata() can return WP_Error object which leads
to Fatal error
--------------------------+-----------------------------
 Reporter:  indigo74      |      Owner:
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  Media         |    Version:  4.8.3
 Severity:  normal        |   Keywords:
  Focuses:                |
--------------------------+-----------------------------
 Dear all,

 I searched for an existing ticket, but I didn't found one. Sorry if I
 missed something.

 Lately, when opening the Media library, I was just seeing a spinning
 circle and nothing shown. It was a call to admin-ajax.php which returns
 Error 500.

 After activating WP_DEBUG, I saw this error:
 {{{
 PHP Fatal error:  Uncaught Error: Cannot use object of type WP_Error as
 array in /home/clients/ef4f22c1fd864ad322af8c3169dede6f/web/next/wp-
 includes/media.php:3145
 Stack trace:
 #0 [internal function]: wp_prepare_attachment_for_js(Object(WP_Post))
 #1 /home/clients/ef4f22c1fd864ad322af8c3169dede6f/web/next/wp-
 admin/includes/ajax-actions.php(2479): array_map('wp_prepare_atta...',
 Array)
 #2 /home/clients/ef4f22c1fd864ad322af8c3169dede6f/web/next/wp-includes
 /class-wp-hook.php(298): wp_ajax_query_attachments('')
 #3 /home/clients/ef4f22c1fd864ad322af8c3169dede6f/web/next/wp-includes
 /class-wp-hook.php(323): WP_Hook->apply_filters('', Array)
 #4 /home/clients/ef4f22c1fd864ad322af8c3169dede6f/web/next/wp-
 includes/plugin.php(453): WP_Hook->do_action(Array)
 #5 /home/clients/ef4f22c1fd864ad322af8c3169dede6f/web/next/wp-admin/admin-
 ajax.php(91): do_action('wp_ajax_query-a...')
 #6 {main}
   thrown in /home/clients/ef4f22c1fd864ad322af8c3169dede6f/web/next/wp-
 includes/media.php on line 3145
 }}}


 Looking at media.php, function wp_prepare_attachment_for_js(), line 3145:
 (https://core.trac.wordpress.org/browser/tags/4.8/src/wp-
 includes/media.php#L3141)
 {{{#!php
 <?php
 function wp_prepare_attachment_for_js( $attachment ) {
         if ( ! $attachment = get_post( $attachment ) )
                 return;

         if ( 'attachment' != $attachment->post_type )
                 return;

         $meta = wp_get_attachment_metadata( $attachment->ID ); /* <--
 $meta is retrieved here */

         /* ... */

         if ( isset( $meta['filesize'] ) ) { /* <-- ERROR IS HERE, LINE
 3145 */
                 $bytes = $meta['filesize'];
         } elseif ( file_exists( $attached_file ) ) {
                 $bytes = filesize( $attached_file );
         } else {
                 $bytes = '';
         }

         /* ... */
 }
 }}}

 In this case, $meta is set to WP_Error object.

 If I print_r($meta) before the issue, I get:

 {{{
 WP_Error Object
 (
     [errors] => Array
         (
             [skip_svg_generate] => Array
                 (
                     [0] => Skipping SVG file.
                 )

         )

     [error_data] => Array
         (
         )

 )
 }}}


 The reason is because I have an old SVG file. It was uploaded using the
 SAFE SVG plugin. This plugin was removed, but the SVG file stayed.

 Anyway, I believe a check is missing on the $meta variable to gracefully
 handle this kind of case.

 This is the first time I open a ticket in WordPress, I hope I explained it
 well.

 Regards
 Indigo.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/42562>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform