WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 weeks ago

#17864 new defect (bug)

Small bug when using "wp_delete_file" filter

Reported by: DuGi_dk Owned by: Morten Rugaard
Milestone: Future Release Priority: normal
Severity: minor Version: 3.1.3
Component: Media Keywords: has-patch needs-refresh 4.0-early
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 3 years ago.
first pass, covering wp_delete_attachment()
17864.2.diff (6.3 KB) - added by scribu 3 years ago.
cover all 'wp_delete_file' filter references

Download all attachments as: .zip

Change History (7)

comment:1 scribu3 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 scribu3 years ago

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

In all cases, the callback should receive a path. It seems that in some cases it's relative, while in others it's not.

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.

Last edited 3 years ago by scribu (previous) (diff)

scribu3 years ago

first pass, covering wp_delete_attachment()

comment:3 scribu3 years ago

  • Keywords has-patch added

scribu3 years ago

cover all 'wp_delete_file' filter references

comment:4 nacin3 months ago

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

comment:5 nacin3 weeks 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.

Note: See TracTickets for help on using tickets.