WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 13 months ago

#35593 reviewing defect (bug)

Metadata from wrong file in media_handle_upload

Reported by: normanrz Owned by: joemcgill
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch needs-testing reporter-feedback close
Focuses: administration Cc:

Description (last modified by swissspidy)

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.

Attachments (1)

media.diff (779 bytes) - added by normanrz 22 months ago.

Download all attachments as: .zip

Change History (6)

@normanrz
22 months ago

#1 @swissspidy
22 months ago

  • Description modified (diff)
  • Keywords has-patch needs-testing added
  • Summary changed from Metadata from wronge file in media_handle_upload to Metadata from wrong file in media_handle_upload

#2 @obenland
20 months ago

  • Version trunk deleted

#3 @joemcgill
18 months ago

  • Keywords reporter-feedback added
  • Owner set to joemcgill
  • Status changed from new to reviewing

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?

#4 @joemcgill
15 months ago

  • Keywords close added

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

#5 @normanrz
13 months ago

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

Note: See TracTickets for help on using tickets.