Metadata from wrong file in media_handle_upload

[normanrz]

In media_handle_upload and media_handle_sideload the attachment metadata is generated after inserting the attachment. For that the same file name is used. However, if a plugin changes the filename in an add_attachment hook the wrong file name will be used. Thus the wrong metadata is written or it is cleared.

See wp-admin/includes/media.php:

368	        // Save the data
369	        $id = wp_insert_attachment($attachment, $file, $post_id);
370	        if ( !is_wp_error($id) ) {
371	                wp_update_attachment_metadata( $id, wp_generate_attachment_metadata( $id, $file ) );
372	        }

In my patch I query the filename again before updating the metadata.

[joemcgill]

Hi @normanrz,

Thanks for the submission. I can see how this would indeed cause unexpected behavior.

However, it seems like a plugin that is firing on an action hook to change the filename should also take responsibility to update the metadata using the wp_update_attachment_metadata hook rather than adding an extra DB hit for everyone. Any reason why that wouldn't be a workable solution?

[joemcgill]

Hi @normanrz,

Curious if you were able to look into my previous suggestion about updating the metadata using the wp_update_attachment_metadata hook?

Thanks,
Joe

[normanrz]

Hi @joemcgill,
thanks for looking into this and sorry for the long wait.
I think the problem is that when I use the drag&drop feature to upload files the following is happening:

• media_handle_upload is invoked and creates the $file variable
• media_handle_upload calls wp_insert_attachment which then calls the add_attachment hook
• now my plugin changes filename (and in fact calls wp_update_attachment_metadata)
• the plugin returns to wp_insert_attachment and that returns to media_handle_upload
• finally media_handle_upload invokes wp_update_attachment_metadata again, but with the stale $file object (because my plugin actually changed some of the metadata) which it created before

It seems to me that I can't handle this in the plugin because any metadata will be later overriden by the stale information from media_handle_upload.

Thanks
Norman

[hellofromTonya]

@joemcgill What do you think of @normanrz last comment? Is this ticket still a close candidate?

[joemcgill]

@hellofromTonya sounds like this could use further investigation to reproduce and fix this issue. I don't want to be a bottle neck further on this if someone else wants to investigate.

[hellofromTonya]

Thanks @joemcgill. Assigning this ticket to me as owner. Moving to Awaiting Review for further investigation.

[hellofromTonya]

Removing myself as owner to avoid bottleneck.