[wp-trac] [WordPress Trac] #45075: wp_delete_attachment() has unreliable return and wrong process order

[WordPress Trac]

#45075: wp_delete_attachment() has unreliable return and wrong process order
--------------------------+------------------------------
 Reporter:  jave.web      |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Media         |     Version:
 Severity:  critical      |  Resolution:
 Keywords:                |     Focuses:
--------------------------+------------------------------
Changes (by jave.web):

 * severity:  major => critical


Comment:

 Replying to [ticket:45075 jave.web]:
 > When doing permanent deletion, this function should also remove the
 files of the target attachment. However successful return of this function
 does **not** guarantee this even though it still can delete the attachment
 object and its relations.
 >
 > Problems when permanent deletion is in process:
 > 1. {{{wp_delete_attachment()}}} must check for
 {{{wp_delete_attachment_files()}}}'s return and should fail too when
 {{{wp_delete_attachment_files()}}} fails
 >
 > 2. {{{wp_delete_attachment_files()}}} should be called first - what is
 the point of deleting object and it's relations when physical data could
 not be removed...
 >
 >


 I've just investigated more and I find a horrifying thing:

 **WP does not check if the files were really deleted or not at all! **

 {{{wp_delete_attachment_files()}}} - checks for
 {{{wp_delete_file_from_directory()}}} return

 **however: ** {{{wp_delete_file_from_directory()}}} is ''guessing'' its
 return value from filepath comparsion while just calling
 {{{wp_delete_file()}}} to the wind (**agaín, not checking for its
 return**)

 This whole shocker ends in {{{wp_delete_file()}}} which does not bother to
 check if the file was really deleted, altough it is aware of possible
 errors since it silences them with {{{@}}} - which is a good practice -
 but only if you check if it really happened.

 {{{wp_delete_file()}}} - Shockingly, it does not return anything at all
 and this is the place, I think, it all started.

 If you consider user's right to his personal data and the right to delete
 them, leaving the files over the internet can cause some legal issues -
 therefore I am raising the severity to critical. (One should check H-self
 in this case, but still WP core should handle this correctly)

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/45075#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform