WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 43 hours ago

#17864 reopened defect (bug)

Small bug when using "wp_delete_file" filter

Reported by: DuGi_dk Owned by: Morten Rugaard
Milestone: 4.2 Priority: normal
Severity: minor Version: 3.1.3
Component: Media Keywords: has-patch
Focuses: Cc:

Description

Hi,

I came acrossed this little annoying bugger, when I was playing around with upload filters.

When I added the "wp_delete_file", I kept getting this error:

Notice: Undefined index: path in /volume1/web/dev/favola_dk/wp-includes/post.php on line 3757

I then digged down in the line, where the error was occuring and I find this little bugger (the bold line is line 3757):

// remove intermediate and backup images if there are any
foreach ( get_intermediate_image_sizes() as $size ) {
        if ( $intermediate = image_get_intermediate_size($post_id, $size) ) {
                $intermediate_file = apply_filters('wp_delete_file', $intermediate['path']);
                @ unlink( path_join($uploadpath['basedir'], $intermediate_file) );
        }
}

I noticed when I var_dump() the $size, that "path" is not within the array at any time. And the funny thing is, if you go down the next block in the code you'll see this:

if ( is_array($backup_sizes) ) {
        foreach ( $backup_sizes as $size ) {
                $del_file = path_join( dirname($meta['file']), $size['file'] );
                $del_file = apply_filters('wp_delete_file', $del_file);
                @ unlink( path_join($uploadpath['basedir'], $del_file) );
        }
}

Here it uses the index "file", which is the one I was expecting to recieve in the former block.

Attachments (2)

17864.diff (3.3 KB) - added by scribu 4 years ago.
first pass, covering wp_delete_attachment()
17864.2.diff (6.3 KB) - added by scribu 4 years ago.
cover all 'wp_delete_file' filter references

Download all attachments as: .zip

Change History (13)

comment:1 @scribu4 years ago

  • Keywords reporter-feedback added; needs-patch 2nd-opinion removed

Please paste your code, that which you hook to 'wp_delete_file'.

comment:2 @scribu4 years ago

  • Component changed from General to Media
  • Milestone changed from Awaiting Review to Future Release

In all cases, the callback should receives a path.

I noticed that there are too many places where the 'wp_delete_file' filter is called manually.

We should make a wp_delete_file() helper function. Patch incoming.

Version 0, edited 4 years ago by scribu (next)

@scribu4 years ago

first pass, covering wp_delete_attachment()

comment:3 @scribu4 years ago

  • Keywords has-patch added

@scribu4 years ago

cover all 'wp_delete_file' filter references

comment:4 @nacin14 months ago

  • Keywords reporter-feedback removed
  • Milestone changed from Future Release to 3.9

comment:5 @nacin12 months ago

  • Keywords needs-refresh 4.0-early added
  • Milestone changed from 3.9 to Future Release

Sorry, we didn't get to this in 3.9, but I'm gonna try to consolidate this again in 4.0.

comment:6 @SergeyBiryukov9 months ago

#28828 was marked as a duplicate.

comment:7 @wonderboymusic4 weeks ago

  • Keywords needs-refresh 4.0-early removed
  • Milestone changed from Future Release to 4.2

comment:8 @wonderboymusic4 weeks ago

  • Resolution set to fixed
  • Status changed from new to closed

In 31575:

Make a new function, wp_delete_file(). Use it.

Props scribu, wonderboymusic.
Fixes #17864.

comment:9 @wonderboymusic4 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:10 @ocean9043 hours ago

@wonderboymusic Anything left here? Inline docs?

comment:11 @slackbot43 hours ago

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

Note: See TracTickets for help on using tickets.