"The file was not deleted, because it does not exist."

The above notice occurs every time drupal_unmanaged_delete_recursive() is called with an invalid or nonexistent URI.

drupal_realpath() is used to convert the streamwrapper URI argument to a text string, but drupal_realpath() may also return FALSE on error. This error condition is never checked, so the following code calls drupal_unmanaged_delete(FALSE):


 function file_unmanaged_delete_recursive($path) {
   // Resolve streamwrapper URI to local path.
   $path = drupal_realpath($path);
  if (is_dir($path)) {
    // ...
  }
  return file_unmanaged_delete($path);
}

A trivial patch to fix the problem follows:

--- includes/file.inc
+++ includes/file.inc
@@ -1305,6 +1305,9 @@ function file_unmanaged_delete($path) {
 function file_unmanaged_delete_recursive($path) {
   // Resolve streamwrapper URI to local path.
   $path = drupal_realpath($path);
+  if ($path === FALSE) {
+    return;
+  }
   if (is_dir($path)) {
     $dir = dir($path);
     while (($entry = $dir->read()) !== FALSE) {
CommentFileSizeAuthor
#6 file.inc_.patch889 bytespillarsdotnet
#21 includes_file.inc_.patch1.23 KBpillarsdotnet
#20 includes_file.inc_.patch1.25 KBpillarsdotnet
#4 file.inc_.patch949 bytespillarsdotnet
#3 file.inc_.patch957 bytespillarsdotnet
#1 file_unmanaged_delete_recursive.patch462 bytespillarsdotnet
file_unmanaged_delete_recursive.patch457 bytespillarsdotnet

Comments

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new file_unmanaged_delete_recursive.patch462 bytes

Corrected patch: The function documentation promises to return TRUE if the path does not exist.

--- includes/file.inc                                                                              
+++ includes/file.inc                                                                              
@@ -1305,6 +1305,9 @@ function file_unmanaged_delete($path) {
 function file_unmanaged_delete_recursive($path) {                                                 
   // Resolve streamwrapper URI to local path.                                                     
   $path = drupal_realpath($path);                                                                 
+  if ($path === FALSE) {                                                                          
+    return TRUE;                                                                                  
+  }                                                                                               
   if (is_dir($path)) {                                                                            
     $dir = dir($path);                                                                            
     while (($entry = $dir->read()) !== FALSE) {

There probably should be a test case for this, but I haven't learn how to write them yet.

pillarsdotnet’s picture

pillarsdotnet commented

Bah. Wish it were possible to edit the main text of an issue report -- I see where I called it "drupal_unmanaged_delete" instead of "file_unmanaged_delete". At least the patch has it correct.

EDIT: Still getting a few "The file was not deleted..." notices. Is it possible for drupal_realpath to return NULL or an empty string?

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new file.inc_.patch957 bytes

Okay, this version kills all the notices.

pillarsdotnet’s picture

pillarsdotnet commented
Title: file_unmanaged_delete() and file_unmanaged_delete_recursive() should check the return value of drupal_realpath() before using it » The file was not deleted, because it does not exist.
Priority: Normal » Minor
StatusFileSize
new file.inc_.patch949 bytes

And without the prefixes which no doubt will confuse the testbot...

EDIT: removed invalid code patch.

Status: Needs review » Needs work

The last submitted patch, file.inc_.patch, failed testing.

pillarsdotnet’s picture

pillarsdotnet commented
Title: file_unmanaged_delete() and file_unmanaged_delete_recursive() should check the return value of drupal_realpath() before using it » The file was not deleted, because it does not exist.
Status: Needs review » Needs work
Issue tags: -Quick fix, -quickfix
StatusFileSize
new file.inc_.patch889 bytes

Corrected:

pillarsdotnet’s picture

pillarsdotnet commented
Priority: Minor » Normal
Status: Needs work » Needs review
pillarsdotnet’s picture

pillarsdotnet commented
Title: The file was not deleted, because it does not exist. » file_unmanaged_delete() and file_unmanaged_delete_recursive() should check the return value of drupal_realpath() before using it

Better title.

adrinux’s picture

adrinux commented
Title: The file was not deleted, because it does not exist. » file_unmanaged_delete() and file_unmanaged_delete_recursive() should check the return value of drupal_realpath() before using it
Priority: Minor » Normal

sub

SilviaT’s picture

SilviaT commented

subscribe

hailaz’s picture

hailaz commented

+1

hailaz’s picture

hailaz commented

This is happening on /admin/modules/list/confirm in Drupal 7.0

Any ideas?

pillarsdotnet’s picture

pillarsdotnet commented

Yeah. Apply the patch in #6 and see if it fixes the problem for you.

  • If it does, post a comment saying so.
  • If it doesn't, post a comment saying so and change status to "needs work"
  • If you don't know how to apply the patch, see http://drupal.org/patch/apply for instructions.
hailaz’s picture

hailaz commented

I'm on a mac and don't have snow leopard so can't download and use Xcode. I'm not a developer, it's way over my head, so to speak.

Will this cause issues on a live site before it's fixed?

I need to get this site up asap and want to use 7.

Thanks in advance and apologies for my lack of programming knowledge. I'd love to contribute but I'm kinda stuck at this point.

pillarsdotnet’s picture

pillarsdotnet commented
Priority: Normal » Minor

This is actually a minor issue. Leaving it "broken" will have no effect other than extra messages in your logs.

hailaz’s picture

hailaz commented
Priority: Minor » Normal

Thanks for that!

mchelen’s picture

mchelen
he/him
commented

Patch from #6 fixed the error messages for me.

pillarsdotnet’s picture

pillarsdotnet commented
Priority: Normal » Minor
Issue tags: +Quick fix, +quickfix
pillarsdotnet’s picture

pillarsdotnet commented

#6: file.inc_.patch queued for re-testing.

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new includes_file.inc_.patch1.25 KB

Slightly different logic, and includes a commented-out watchdog line for those who want to debug the issue.

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new includes_file.inc_.patch1.23 KB

Bah. Typo. Still recovering from the Great Git Migration....

Uncommented the watchdog.

magnusk’s picture

magnusk commented

applied patch #6, the problem appears to be fixed. Thanks.

magnusk’s picture

magnusk commented
Status: Needs review » Needs work

I spoke too soon. Cron jobs run every hour on two sites hosted with two distinct servers. The message came back during the third cron run after patch on one server, and during the second cron run on another server.

The notice relates to the "Chaos tools" module:

The file /.../files/ctools/css was not deleted, because it does not exist.
pillarsdotnet’s picture

pillarsdotnet commented
Status: Needs work » Needs review

@magnusk:
The originally-reported issue was an error message saying "The file was not deleted, because it does not exist."

Note the lack of a filename between the words "file" and "was". This makes troubleshooting the problem very difficult.

You have a totally different (and much more informative) error message. Obviously, something related to Chaos tools is passing a filepath with three consecutive dots in it. You should file an issue against the Chaos tools module accordingly.

magnusk’s picture

magnusk commented
Title: The file was not deleted, because it does not exist. » file_unmanaged_delete() and file_unmanaged_delete_recursive() should check the return value of drupal_realpath() before using it
Status: Needs work » Needs review
Issue tags: +Quick fix, +quickfix

The "..." was just me removing the specific path on my server, sorry if that was confusing. I've opened an issue in the ctools module. It seems like there are multiple issues involved?

pillarsdotnet’s picture

pillarsdotnet commented

Dunno. The patch in #21 definitely helps track down the real problem.

Mac Ryan’s picture

Mac Ryan commented

Subscribe

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Closed (duplicate)

Actually those calls to drupal_realpath() needs to go away.

Marking as a duplicate of #1083982: Fix support for remote streamwrappers.

damien tournoud’s picture

damien tournoud commented
Issue tags: -Quick fix, -quickfix

Removing tags.

OldAccount’s picture

OldAccount commented

Patch #6 is working for me, thank you!