Make WordPress Core

Opened 12 years ago

Last modified 6 years ago

#21679 new enhancement

media_handle_upload does not provide a way to change the file's name

Reported by: willshouse's profile Willshouse Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.5
Component: Media Keywords: has-patch
Focuses: Cc:

Description (last modified by johnbillion)

wp-admin/includes/media.php has two operations that I believe should be reversed. Basically, $name is set based on the name of the raw uploaded file ( $_FILES[$file_id]['name']), however in the wp_handle_upload function you are able to use wp_handle_upload_prefilter to adjust the file's name - but when after the wp_handle_upload returns the changes will not show up in the title of the media dialog field even though the file has been renamed properly:

	$name = $_FILES[$file_id]['name'];
	$file = wp_handle_upload($_FILES[$file_id], $overrides, $time);

	if ( isset($file['error']) )
		return new WP_Error( 'upload_error', $file['error'] );

	$name_parts = pathinfo($name);
	$name = trim( substr( $name, 0, -(1 + strlen($name_parts['extension'])) ) );

In short, uploading a file named Picture.png and changing the name to test3.png using the wp_handle_upload_prefilter filter does in fact allow the file's name to be changed before it is saved, but the "title" is then incorrectly displayed in the media uploader dialog box.

http://img24.imageshack.us/img24/3679/pictureyp.png

This would be relatively easy to fix using this patch:

--- media.php	2012-08-23 23:57:02.000000000 -0400
+++ media-patch.php	2012-06-06 12:00:08.000000000 -0400
@@ -209,8 +209,8 @@
 			$time = $post->post_date;
 	}
 
+	$name = $_FILES[$file_id]['name'];
 	$file = wp_handle_upload($_FILES[$file_id], $overrides, $time);
-	$name = $file['name'];
 
 	if ( isset($file['error']) )
 		return new WP_Error( 'upload_error', $file['error'] );

Attachments (3)

media.patch (381 bytes) - added by Willshouse 12 years ago.
patch
Picture.png (28.8 KB) - added by Willshouse 12 years ago.
media upload screnshot
21679.patch (564 bytes) - added by SergeyBiryukov 12 years ago.

Download all attachments as: .zip

Change History (18)

@Willshouse
12 years ago

patch

@Willshouse
12 years ago

media upload screnshot

#1 @c3mdigital
12 years ago

  • Keywords dev-feedback added
  • Version changed from 3.4.1 to 2.5

@Wilishouse could you take a look at the patch. The diff seems off . Make sure you are at latest version of trunk. Make your changes then run the diff command from the root directory of the install.

Possibly related #18730, #16330

#2 @johnbillion
12 years ago

  • Description modified (diff)

#3 follow-up: @SergeyBiryukov
12 years ago

media.patch looks reversed. Even with that corrected, it would cause notices, since wp_handle_upload() result doesn't contain the name key:

Notice: Undefined index: name in wp-admin/includes/media.php on line 215
Notice: Undefined index: extension in wp-admin/includes/media.php on line 221

21679.patch fixes the notices, but has one side effect.

Currently, if you upload "Picture.jpg" several times, the attachments will have "Picture" title.

With the patch, the titles would be "Picture", "Picture1", "Picture2", etc.

A workaround for the inability to change the title with wp_handle_upload_prefilter would probably be to hook into add_attachment and alter the title after the attachment has been uploaded.

#4 @jsdavis02
11 years ago

I just fixed this on an install of mine. Wilishouse did the same thing I initially did assuming we got back the actual file object instead of a modified associative array from file.php...

return apply_filters( 'wp_handle_upload', array( 'file' => $new_file, 'url' => $url, 'type' => $type ), 'upload' );

So to still avoid changing two core files I just did a quick change to media.php to strip it from the full upload path, so it has a filename that ran through all sanitizing and filter calls.

 //$name = $_FILES[$file_id]['name'];  --wrong time to pull and wrong place to pull from, 
 //allow handle upload and prefilter to run first
	$file = wp_handle_upload($_FILES[$file_id], $overrides, $time);
	$name = substr($file['file'],strrpos($file['file'],'/')+1); 

It really should not be difficult to add this in and it should be pretty low impact

#5 @jsdavis02
11 years ago

  • Cc jsdavis02 added
  • Keywords needs-patch added; has-patch removed

#6 @danielbachhuber
10 years ago

Similarly, there's no way to filter the target destination for the file, which is set by $time.

#16849 is related in the sense that all of these are requests to change the behavior of wp_handle_upload()

#7 in reply to: ↑ 3 @ericlewis
10 years ago

Replying to SergeyBiryukov:

Currently, if you upload "Picture.jpg" several times, the attachments will have "Picture" title.

With the patch, the titles would be "Picture", "Picture1", "Picture2", etc.

I don't think that's a bad side effect, maybe better than naming them all the same.

21679.patch looks good to me.

#8 @ericlewis
10 years ago

  • Keywords has-patch added; needs-testing dev-feedback needs-patch removed

#9 @ericlewis
10 years ago

  • Type changed from defect (bug) to enhancement

#10 @wonderboymusic
9 years ago

  • Milestone changed from Awaiting Review to 4.2

This ticket was mentioned in Slack in #core by drew. View the logs.


9 years ago

#12 @DrewAPicture
9 years ago

@wonderboymusic: What are your thoughts on this improvement?

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


9 years ago

#14 @johnbillion
9 years ago

  • Milestone changed from 4.2 to Future Release

#15 @iandunn
6 years ago

Related: r36310 (#19121) added the wp_unique_filename filter.

Note: See TracTickets for help on using tickets.