diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php index 6dbe0731701..7439b27a71d 100644 --- a/backup/moodle2/backup_stepslib.php +++ b/backup/moodle2/backup_stepslib.php @@ -1714,9 +1714,22 @@ class backup_zip_contents extends backup_execution_step implements file_progress $zippacker = get_file_packer('application/vnd.moodle.backup'); // Zip files - $zippacker->archive_to_pathname($files, $zipfile, true, $this); + $result = $zippacker->archive_to_pathname($files, $zipfile, true, $this); - // If any progress happened, end it. + // Something went wrong. + if ($result === false) { + @unlink($zipfile); + throw new backup_step_exception('error_zip_packing', '', 'An error was encountered while trying to generate backup zip'); + } + // Read to make sure it is a valid backup. Refer MDL-37877 . Delete it, if found not to be valid. + try { + backup_general_helper::get_backup_information_from_mbz($zipfile); + } catch (backup_helper_exception $e) { + @unlink($zipfile); + throw new backup_step_exception('error_zip_packing', '', $e->debuginfo); + } + + // If any progress happened, end it. if ($this->startedprogress) { $this->task->get_progress()->end_progress(); } diff --git a/backup/util/helper/backup_cron_helper.class.php b/backup/util/helper/backup_cron_helper.class.php index a49ee7bebad..949565a8415 100644 --- a/backup/util/helper/backup_cron_helper.class.php +++ b/backup/util/helper/backup_cron_helper.class.php @@ -377,7 +377,11 @@ abstract class backup_cron_automated_helper { $outcome = self::BACKUP_STATUS_OK; $config = get_config('backup'); - $bc = new backup_controller(backup::TYPE_1COURSE, $course->id, backup::FORMAT_MOODLE, backup::INTERACTIVE_NO, backup::MODE_AUTOMATED, $userid); + $dir = $config->backup_auto_destination; + $storage = (int)$config->backup_auto_storage; + + $bc = new backup_controller(backup::TYPE_1COURSE, $course->id, backup::FORMAT_MOODLE, backup::INTERACTIVE_NO, + backup::MODE_AUTOMATED, $userid); try { @@ -402,27 +406,28 @@ abstract class backup_cron_automated_helper { } } - // Set the default filename + // Set the default filename. $format = $bc->get_format(); $type = $bc->get_type(); $id = $bc->get_id(); $users = $bc->get_plan()->get_setting('users')->get_value(); $anonymised = $bc->get_plan()->get_setting('anonymize')->get_value(); - $bc->get_plan()->get_setting('filename')->set_value(backup_plan_dbops::get_default_backup_filename($format, $type, $id, $users, $anonymised)); + $bc->get_plan()->get_setting('filename')->set_value(backup_plan_dbops::get_default_backup_filename($format, $type, + $id, $users, $anonymised)); $bc->set_status(backup::STATUS_AWAITING); $bc->execute_plan(); $results = $bc->get_results(); $outcome = self::outcome_from_results($results); - $file = $results['backup_destination']; // may be empty if file already moved to target location - $dir = $config->backup_auto_destination; - $storage = (int)$config->backup_auto_storage; + $file = $results['backup_destination']; // May be empty if file already moved to target location. if (!file_exists($dir) || !is_dir($dir) || !is_writable($dir)) { $dir = null; } - if ($file && !empty($dir) && $storage !== 0) { - $filename = backup_plan_dbops::get_default_backup_filename($format, $type, $course->id, $users, $anonymised, !$config->backup_shortname); + // Copy file only if there was no error. + if ($file && !empty($dir) && $storage !== 0 && $outcome != self::BACKUP_STATUS_ERROR) { + $filename = backup_plan_dbops::get_default_backup_filename($format, $type, $course->id, $users, $anonymised, + !$config->backup_shortname); if (!$file->copy_content_to($dir.'/'.$filename)) { $outcome = self::BACKUP_STATUS_ERROR; } @@ -438,6 +443,20 @@ abstract class backup_cron_automated_helper { $outcome = self::BACKUP_STATUS_ERROR; } + // Delete the backup file immediately if something went wrong. + if ($outcome === self::BACKUP_STATUS_ERROR) { + + // Delete the file from file area if exists. + if (!empty($file)) { + $file->delete(); + } + + // Delete file from external storage if exists. + if ($storage !== 0 && !empty($filename) && file_exists($dir.'/'.$filename)) { + @unlink($dir.'/'.$filename); + } + } + $bc->destroy(); unset($bc);