In file_unmanaged_save_data() we create a temporary file, then move it into place. However, if the move fails, we end up with stale temp files. These are owned www-data:www-data and have mode 0600 by default, which can cause problems with backup scripts (i.e., Aegir) and such that won't be able to read them.

It's a pretty simple matter to check the return value of file_unmanaged_move(), and delete the temp file if the move failed. This function is identical in D7 and D8, but I'll post patches shortly for both.

CommentFileSizeAuthor
#1 drupal-clean_up_temp_files-2496173-1-d8.patch645 bytesergonlogic
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,160 pass(es). View
#1 drupal-clean_up_temp_files-2496173-1-d7.patch627 bytesergonlogic
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-clean_up_temp_files-2496173-1-d7.patch. Unable to apply patch. See the log in the details link for more information. View
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
CreditAttribution: ergonlogic at Poetic Systems for Camden commented
FileSize
drupal-clean_up_temp_files-2496173-1-d7.patch627 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-clean_up_temp_files-2496173-1-d7.patch. Unable to apply patch. See the log in the details link for more information. View
drupal-clean_up_temp_files-2496173-1-d8.patch645 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,160 pass(es). View

Here are the patches.

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
CreditAttribution: ergonlogic at Poetic Systems for Camden commented
Assigned: ergonlogic » Unassigned
Status: Active » Needs review

The last submitted patch, 1: drupal-clean_up_temp_files-2496173-1-d7.patch, failed testing.

cilefen’s picture

cilefen CreditAttribution: cilefen commented

Is there any way data could be lost?

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
CreditAttribution: ergonlogic at Poetic Systems for Camden commented

Not really. These files are randomly named, so they couldn't really be recovered. This only happens if there's a failure downstream. It just cleans up the temp files.

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
CreditAttribution: ergonlogic at Poetic Systems for Camden commented

I think we might be better off just calling drupal_unlink() directly, rather than file_unmanaged_delete() here.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

I'm curious whether we can test this behaviour here?

dagomar’s picture

dagomar CreditAttribution: dagomar commented
Status: Needs review » Reviewed & tested by the community

I had the same problem, this patch does fix this for me (drupal 7).

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs review

We need an answer @dawehner's question in #7 as to whether this is testable..

+++ b/core/includes/file.inc
@@ -987,7 +987,12 @@ function file_unmanaged_save_data($data, $destination = NULL, $replace = FILE_EX
+    file_unmanaged_delete($temp_name);

I think using file_unmanaged_delete() is fine.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions for Aegir Cooperative commented

Could we test this by calling file_unmanaged_save_data() with a destination that we know does not exist? The problem then is though that we don't know the $temp_name outside this function.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

20th’s picture

20th CreditAttribution: 20th commented
Parent issue: » #2229865: [meta] Modernize File/StreamWrapper API

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

maustyle’s picture

maustyle CreditAttribution: maustyle commented

removed by author.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.