Media metaboxes return fatal error if no author metadata present

[carloscastilloadhoc]

Images uploaded programatically to the media library via the wp_insert_attachment function won't have author metadata. This causes a fatal error in the metaboxes when you try to use the "edit media" option in the media library.

Specifically: Uncaught Error: Call to a member function exists() on bool in wp-admin/includes/media.php line 3295

This can be fixed for example by expanding the condition "if($author->exists())" with "if($author && $author->exists())".

While I haven't personally encountered them, this may cause additional errors in other places where the same check is performed (for example the wp_prepare_attachment_for_js function in wp-includes/media.php).

Thank you and kind regards.

[hellofromTonya]

Hello @carloscastilloadhoc,

Welcome to WordPress Trac! Thank you for this ticket.

Introduced in #46866 and changeset [49207], a check is missing for get_userdata() as it returns false when no user is found. At a minimum, this code should be guarded. Moving this ticket into 5.6.1 to add this guard.

For the case where an image is programmatically created without an assigned user, more discussions is needed. Pinging @antpb @garrett-eclipse for their thoughts.

[SergeyBiryukov]

As noted above, the code in attachment_submitbox_metadata() uses get_userdata():

$author = get_userdata( $post->post_author );

which indeed returns false on failure.

However, the similar code in wp_prepare_attachment_for_js() creates a new instance of WP_User instead:

$author = new WP_User( $attachment->post_author );

Would be great to have some consistency here.

[antpb]

initial patch for attachment_submitbox_metadata

[antpb]

Thanks @SergeyBiryukov , good call on consistency. I've uploaded an initial patch for us to work off of. @garrett-eclipse would really appreciate a review. Is there any other level of guard this needs? I assume since new WP_User() it can't result in fatal.

[garrett-eclipse]

Thanks for the catch @carloscastilloadhoc, the triage @hellofromTonya, the idea @SergeyBiryukov and the patch @antpb. It's looking and working good for me so am marking for commit. Carlos if you could confirm the change fixes your install that would also be appreciated.

[carloscastilloadhoc]

Replying to garrett-eclipse:

Thanks for the catch @carloscastilloadhoc, the triage @hellofromTonya, the idea @SergeyBiryukov and the patch @antpb. It's looking and working good for me so am marking for commit. Carlos if you could confirm the change fixes your install that would also be appreciated.

I do confirm the patch fixes the issue on my end, thanks.

[SergeyBiryukov]

#52145 was marked as a duplicate.

[SergeyBiryukov]

In 49979:

Media: Use consistent method for instantiating an attachment author object in Media Library.

Previously, attachments without an author could cause a PHP fatal error due to calling the ::exists() method on a false value.

Props antpb, carloscastilloadhoc, hellofromTonya, garrett-eclipse.
Fixes #52030.

[SergeyBiryukov]

Reopening for backporting to the 5.6 branch.

[SergeyBiryukov]

In 49995:

Media: Use consistent method for instantiating an attachment author object in Media Library.

Previously, attachments without an author could cause a PHP fatal error due to calling the ::exists() method on a false value.

Follow-up to [49207].

Props antpb, carloscastilloadhoc, hellofromTonya, garrett-eclipse.
Merges [49979] to the 5.6 branch.
Fixes #52030.