From 5baadd1fa18cc45534a1a66a67af7c61d442af3e Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 24 Apr 2020 03:08:31 +0200 Subject: [PATCH 1/5] Be more robust against some situation where archive is corrupted --- locales/en.json | 3 ++- src/yunohost/backup.py | 42 +++++++++++++++++++++++++----------------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/locales/en.json b/locales/en.json index c2c087031..9207b304b 100644 --- a/locales/en.json +++ b/locales/en.json @@ -74,6 +74,8 @@ "backup_archive_name_exists": "A backup archive with this name already exists.", "backup_archive_name_unknown": "Unknown local backup archive named '{name:s}'", "backup_archive_open_failed": "Could not open the backup archive", + "backup_archive_cant_retrieve_info_json": "Could not load infos for archive '{archive}' ... The info.json cannot be retrieved (or is not a valid json).", + "backup_archive_corrupted": "It looks like the backup archive '{archive}' is corrupted : {error}", "backup_archive_system_part_not_available": "System part '{part:s}' unavailable in this backup", "backup_archive_writing_error": "Could not add the files '{source:s}' (named in the archive '{dest:s}') to be backed up into the compressed archive '{archive:s}'", "backup_ask_for_copying_if_needed": "Do you want to perform the backup using {size:s} MB temporarily? (This way is used since some files could not be prepared using a more efficient method.)", @@ -91,7 +93,6 @@ "backup_delete_error": "Could not delete '{path:s}'", "backup_deleted": "Backup deleted", "backup_hook_unknown": "The backup hook '{hook:s}' is unknown", - "backup_invalid_archive": "This is not a backup archive", "backup_method_borg_finished": "Backup into Borg finished", "backup_method_copy_finished": "Backup copy finalized", "backup_method_custom_finished": "Custom backup method '{method:s}' finished", diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 10a232f38..5f24f444f 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -870,7 +870,7 @@ class RestoreManager(): Read the info file from inside an archive Exceptions: - backup_invalid_archive -- Raised if we can't read the info + backup_archive_cant_retrieve_info_json -- Raised if we can't read the info """ # Retrieve backup info info_file = os.path.join(self.work_dir, "info.json") @@ -883,7 +883,7 @@ class RestoreManager(): self.info["system"] = self.info["hooks"] except IOError: logger.debug("unable to load '%s'", info_file, exc_info=1) - raise YunohostError('backup_invalid_archive') + raise YunohostError('backup_archive_cant_retrieve_info_json', archive=self.archive_path) else: logger.debug("restoring from backup '%s' created on %s", self.name, datetime.utcfromtimestamp(self.info['created_at'])) @@ -891,10 +891,6 @@ class RestoreManager(): def _postinstall_if_needed(self): """ Post install yunohost if needed - - Exceptions: - backup_invalid_archive -- Raised if the current_host isn't in the - archive """ # Check if YunoHost is installed if not os.path.isfile('/etc/yunohost/installed'): @@ -906,7 +902,7 @@ class RestoreManager(): logger.debug("unable to retrieve current_host from the backup", exc_info=1) # FIXME include the current_host by default ? - raise YunohostError('backup_invalid_archive') + raise YunohostError("The main domain name cannot be retrieved from inside the archive, and is needed to perform the postinstall", raw_msg=True) logger.debug("executing the post-install...") tools_postinstall(domain, 'Yunohost', True) @@ -1924,6 +1920,12 @@ class TarBackupMethod(BackupMethod): self._archive_file, exc_info=1) raise YunohostError('backup_archive_open_failed') + try: + files_in_archive = tar.getnames() + print(files_in_archive) + except IOError as e: + raise YunohostError("backup_archive_corrupted", archive=self._archive_file, error=str(e)) + # FIXME : Is this really useful to close the archive just to # reopen it right after this with the same options ...? tar.close() @@ -1932,21 +1934,21 @@ class TarBackupMethod(BackupMethod): logger.debug(m18n.n("restore_extracting")) tar = tarfile.open(self._archive_file, "r:gz") - if "info.json" in tar.getnames(): + if "info.json" in files_in_archive: leading_dot = "" tar.extract('info.json', path=self.work_dir) - elif "./info.json" in tar.getnames(): + elif "./info.json" in files_in_archive: leading_dot = "./" tar.extract('./info.json', path=self.work_dir) else: logger.debug("unable to retrieve 'info.json' inside the archive", exc_info=1) tar.close() - raise YunohostError('backup_invalid_archive') + raise YunohostError('backup_archive_cant_retrieve_info_json', archive=self._archive_file) - if "backup.csv" in tar.getnames(): + if "backup.csv" in files_in_archive: tar.extract('backup.csv', path=self.work_dir) - elif "./backup.csv" in tar.getnames(): + elif "./backup.csv" in files_in_archive: tar.extract('./backup.csv', path=self.work_dir) else: # Old backup archive have no backup.csv file @@ -2288,7 +2290,7 @@ def backup_list(with_info=False, human_readable=False): try: d[a] = backup_info(a, human_readable=human_readable) except YunohostError as e: - logger.warning('%s: %s' % (a, e.strerror)) + logger.warning(str(e)) result = d @@ -2325,17 +2327,23 @@ def backup_info(name, with_details=False, human_readable=False): if not os.path.exists(info_file): tar = tarfile.open(archive_file, "r:gz") info_dir = info_file + '.d' + try: - if "info.json" in tar.getnames(): + files_in_archive = tar.getnames() + except IOError as e: + raise YunohostError("backup_archive_corrupted", archive=archive_file, error=str(e)) + + try: + if "info.json" in files_in_archive: tar.extract('info.json', path=info_dir) - elif "./info.json" in tar.getnames(): + elif "./info.json" in files_in_archive: tar.extract('./info.json', path=info_dir) else: raise KeyError except KeyError: logger.debug("unable to retrieve '%s' inside the archive", info_file, exc_info=1) - raise YunohostError('backup_invalid_archive') + raise YunohostError('backup_archive_cant_retrieve_info_json', archive=archive_file) else: shutil.move(os.path.join(info_dir, 'info.json'), info_file) finally: @@ -2348,7 +2356,7 @@ def backup_info(name, with_details=False, human_readable=False): info = json.load(f) except: logger.debug("unable to load '%s'", info_file, exc_info=1) - raise YunohostError('backup_invalid_archive') + raise YunohostError('backup_archive_cant_retrieve_info_json', archive=archive_file) # Retrieve backup size size = info.get('size', 0) From a20fd04955581ce5a261de58bf461ed00b7beb2f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sat, 25 Apr 2020 01:27:20 +0200 Subject: [PATCH 2/5] Remove tmp debug print() Co-Authored-By: Kayou --- src/yunohost/backup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 5f24f444f..5d64ae5d6 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -1922,7 +1922,6 @@ class TarBackupMethod(BackupMethod): try: files_in_archive = tar.getnames() - print(files_in_archive) except IOError as e: raise YunohostError("backup_archive_corrupted", archive=self._archive_file, error=str(e)) From 77e124519f14b2fc50d29f1c12a15f993b736121 Mon Sep 17 00:00:00 2001 From: Kay0u Date: Sat, 25 Apr 2020 01:54:12 +0200 Subject: [PATCH 3/5] add bad archive test --- src/yunohost/tests/test_backuprestore.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/yunohost/tests/test_backuprestore.py b/src/yunohost/tests/test_backuprestore.py index bcba21bb6..b715aad04 100644 --- a/src/yunohost/tests/test_backuprestore.py +++ b/src/yunohost/tests/test_backuprestore.py @@ -574,9 +574,20 @@ def test_restore_archive_with_no_json(mocker): assert "badbackup" in backup_list()["archives"] - with raiseYunohostError(mocker, 'backup_invalid_archive'): + with raiseYunohostError(mocker, 'backup_archive_cant_retrieve_info_json'): backup_restore(name="badbackup", force=True) +@pytest.mark.with_wordpress_archive_from_2p4 +def test_restore_archive_with_bad_archive(mocker): + + # Break the archive + os.system("head -n 1000 /home/yunohost.backup/archives/backup_wordpress_from_2p4.tar.gz > /home/yunohost.backup/archives/backup_wordpress_from_2p4.tar.gz") + + assert "backup_wordpress_from_2p4" in backup_list()["archives"] + + with raiseYunohostError(mocker, 'backup_archive_open_failed'): + backup_restore(name="backup_wordpress_from_2p4", force=True) + def test_backup_binds_are_readonly(mocker, monkeypatch): From 05734dfd7cb221197b065e0cde747721f84bafbe Mon Sep 17 00:00:00 2001 From: Kay0u Date: Sat, 25 Apr 2020 02:28:45 +0200 Subject: [PATCH 4/5] clean tmp backuo dir --- src/yunohost/tests/test_backuprestore.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/yunohost/tests/test_backuprestore.py b/src/yunohost/tests/test_backuprestore.py index b715aad04..c7a4f9016 100644 --- a/src/yunohost/tests/test_backuprestore.py +++ b/src/yunohost/tests/test_backuprestore.py @@ -588,6 +588,8 @@ def test_restore_archive_with_bad_archive(mocker): with raiseYunohostError(mocker, 'backup_archive_open_failed'): backup_restore(name="backup_wordpress_from_2p4", force=True) + clean_tmp_backup_directory() + def test_backup_binds_are_readonly(mocker, monkeypatch): From 311835b1b5ccb8a64f8baa3390153524fe402b80 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 27 Apr 2020 23:23:31 +0200 Subject: [PATCH 5/5] Add name of the exceptions that can be raised to docstring.. --- src/yunohost/backup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 5d64ae5d6..c2d2e276a 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -1909,6 +1909,8 @@ class TarBackupMethod(BackupMethod): Exceptions: backup_archive_open_failed -- Raised if the archive can't be open + backup_archive_corrupted -- Raised if the archive appears corrupted + backup_archive_cant_retrieve_info_json -- If the info.json file can't be retrieved """ super(TarBackupMethod, self).mount(restore_manager)