WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 4 months ago

#16849 new enhancement

Add a filter for $overrides in wp_handle_upload()

Reported by: iandunn Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.1
Component: Upload Keywords: has-patch dev-feedback
Focuses: Cc:

Description

I'm writing a plugin that needs to set a $unique_filename_callback in wp_handle_upload() during a normal post attachment upload (not a custom upload form), but there's no way to do it without a filter like this one:

$overrides = apply_filters( 'wp_handle_upload_overrides', $overrides );

For now I just modded my /wp-admin/includes/file.php to add it, but obviously that's not an ideal solution, especially since I plan on adding the plugin to the repository for others to use.

Attachments (3)

16849.patch (513 bytes) - added by jakub.tyrcha 7 years ago.
16849.2.patch (843 bytes) - added by iandunn 5 years ago.
16489.3.patch (804 bytes) - added by Mte90 17 months ago.
refreshed patch

Download all attachments as: .zip

Change History (27)

#1 @mikeschinkel
7 years ago

  • Cc mikeschinkel@… added

#2 @jakub.tyrcha
7 years ago

  • Keywords has-patch needs-testing added

Sounds reasonable, doesn't it?

#3 @iandunn
7 years ago

Hey Jakub, thanks for writing the patch. I'm not sure it's correct, though. I think the filter should actually go before if ( is_array( $overrides ) ), shouldn't it?

We want to let a plugin modify/overwrite $overrides before $overrides gets used. The way the patch is written now, the filter could only be used if $overrides was already passed in, which won't always be the case.

Also, you forgot to wrap the if block in brackets, so the extract() statement would get called even if the condition was false.

I think below $file = apply_filters( 'wp_handle_upload_prefilter', $file ); would be a good place for it. It doesn't need to be wrapped in an if statement or anything, so it would just be

$file = apply_filters( 'wp_handle_upload_prefilter', $file );
$overrides = apply_filters( 'wp_handle_upload_overrides', $overrides );

#4 follow-up: @jakub.tyrcha
7 years ago

Ah, I'm deeply sorry - it must be the pre-gsoc rush. As for the place in code - I thought it was needed only when there are some existing overrides - I moved it to the suggested place now

@jakub.tyrcha
7 years ago

#5 in reply to: ↑ 4 @iandunn
7 years ago

Looks great, thanks :)

#6 @DrewAPicture
6 years ago

  • Keywords dev-feedback added

#7 @jcow
6 years ago

This would be a really useful addition for something so simple. I almost always prefer the renaming of files as it stands in Wordpress, but now and then I need the ability to control a filename, and this would come in handy. I hope this is being considered for addition. Thanks.

#8 @mordauk
6 years ago

I see this was originally slated for 3.1, any chance we can get this pushed into 3.5?

#9 @nacin
6 years ago

  • Keywords commit added; needs-testing dev-feedback removed
  • Milestone changed from Awaiting Review to 3.5

We should also add an overrides filter to wp_handle_sideload(). But since it doesn't have a prefilter filter, I'm fine with just this for now.

#10 @iandunn
6 years ago

Awesome, thanks nacin :)

#11 @nacin
6 years ago

  • Keywords 3.6-early added

#12 @nacin
6 years ago

  • Milestone changed from 3.5 to Future Release

#13 @martey
5 years ago

  • Cc martey added

@iandunn
5 years ago

#14 @iandunn
5 years ago

I refreshed the patch and added filters for $file and $overrides in wp_handle_sideload().

Would this be a good candidate for 3.7? It seems like a decent number of people think it'd be useful, and it's required for the Overwrite Uploads plugin to work without having to mod Core.

#15 @SergeyBiryukov
5 years ago

  • Milestone changed from Future Release to 3.7

#16 @nacin
5 years ago

We really, really need to change how this works. Deliberately allowing people to specify variables so we can EXTR_OVERWRITE them is a recipe for disaster — or at least, in this case, inflexibility. This function really scares me because it can encourage bad or insecure code. That, and it doesn't follow a design pattern we use almost everywhere else.

Is this something we can convert to a more standard $defaults and wp_parse_args() situation? Then we can do an apply_filters() on that, followed by extract() with EXTR_SKIP.

#17 @wonderboymusic
5 years ago

  • Keywords needs-docs added; commit removed

There's a new filter

#18 @jeremyfelt
5 years ago

  • Keywords 3.6-early removed
  • Milestone changed from 3.7 to Future Release

Punting to future release for now. Seems like something that should have a plan early in the cycle. See concerns in Nacin's last comment.

#19 @nacin
4 years ago

  • Component changed from Plugins to Upload
  • Keywords needs-patch added; has-patch needs-docs removed

#21 @wonderboymusic
4 years ago

_wp_handle_upload() is now a mashup of the two, the overrides are gross, but it is physically impossible to change the value of them without a filter because media_handle_upload() never passes that argument.

#22 @chriscct7
3 years ago

  • Keywords needs-refresh has-patch added; needs-patch removed
  • Severity changed from minor to normal

@Mte90
17 months ago

refreshed patch

#23 @Mte90
17 months ago

  • Keywords dev-feedback added; needs-refresh removed

I refreshed the patch with the latest version.

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


4 months ago

Note: See TracTickets for help on using tickets.