[wp-trac] [WordPress Trac] #39476: wp_delete_file filter expects both absolute and relative paths

WordPress Trac noreply at wordpress.org
Thu Jan 5 04:38:15 UTC 2017


#39476: wp_delete_file filter expects both absolute and relative paths
--------------------------+-----------------------
 Reporter:  rmccue        |      Owner:
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  4.7.1
Component:  Media         |    Version:  4.2
 Severity:  normal        |   Keywords:  has-patch
  Focuses:                |
--------------------------+-----------------------
 The `wp_delete_file` filter is called in a number of places. In
 particular, it's called in `wp_delete_file()` and
 `wp_delete_attachment()`. Unfortunately, the filter acts inconsistently.

 In `wp_delete_file()`, the result of the filter is passed to `unlink`
 directly (assuming it's not empty). In `wp_delete_attachment()` however,
 the file is combined with the upload base directory using `path_join()`.

 In most normal circumstances, this is not an issue, as `path_join()` no-
 ops when passed an absolute path. However, when using stream wrappers with
 a custom protocol (such as `s3://`), `path_join()` does not recognise
 these as absolute paths and redundantly joins the paths, resulting in a
 broken path. The error suppression asperand (@) hides this invalid path
 error.

 There's two obvious solutions we could choose:

 1. Be consistent with passing paths in. Either always absolute or always
 relative.
 2. Just use `wp_delete_file` in `wp_delete_attachment`. This forces it to
 be consistent. This implies being consistently absolute.

 As the filter is currently inconsistent, I don't think there's a huge
 amount of damage to changing it to be always absolute.

 When `wp_delete_file()` was added in #17864, it was used in most places,
 but not in `wp_delete_attachment()` for Unknown Reasons. Every other
 invocation of the function (and hence filter) is absolute.

 Patch attached.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/39476>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list