From b2e791d1539d81ff8d171fa490da4a64fce59b33 Mon Sep 17 00:00:00 2001
From: Ankit Agarwal <ankit@moodle.com>
Date: Wed, 9 Oct 2013 11:40:53 +0800
Subject: [PATCH] MDL-37877 backup: Fix issues with zipping of large files

Files bigger than 4GB is not supported at this point. Zip archiver doesn't know about it and always returns true. We need to verify if it is a valid file or not and delete if not
Also delete the backup file immediately if anything goes wrong anywhere in backup. We don't want to use up space
---
 backup/moodle2/backup_stepslib.php            | 17 +++++++--
 .../util/helper/backup_cron_helper.class.php  | 35 ++++++++++++++-----
 2 files changed, 42 insertions(+), 10 deletions(-)

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);