From ab86cb5962038c14a7e6c2a0b5ce7b125b65589e Mon Sep 17 00:00:00 2001 From: Aleksandr Parfenov Date: Sat, 9 Jun 2018 14:59:35 +0300 Subject: [PATCH] Warning in case of empty/corrupted backup.control file --- src/catalog.c | 21 +++++++++- src/utils/pgut.c | 17 ++++++--- src/utils/pgut.h | 2 +- tests/option_test.py | 5 ++- tests/show_test.py | 91 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 9 deletions(-) diff --git a/src/catalog.c b/src/catalog.c index a0a8e5dc..78630b85 100644 --- a/src/catalog.c +++ b/src/catalog.c @@ -479,6 +479,7 @@ readBackupControlFile(const char *path) char *program_version = NULL; char *server_version = NULL; char *compress_alg = NULL; + int parsed_options; pgut_option options[] = { @@ -508,10 +509,28 @@ readBackupControlFile(const char *path) }; if (access(path, F_OK) != 0) + { + elog(WARNING, "control file \"%s\" doesn't exist", path); + pgBackupFree(backup); return NULL; + } pgBackup_init(backup); - pgut_readopt(path, options, ERROR); + parsed_options = pgut_readopt(path, options, WARNING); + + if (parsed_options == 0) + { + elog(WARNING, "control file \"%s\" is empty", path); + pgBackupFree(backup); + return NULL; + } + + if (backup->start_time == 0) + { + elog(WARNING, "invalid ID/start-time, control file \"%s\" is corrupted", path); + pgBackupFree(backup); + return NULL; + } if (backup_mode) { diff --git a/src/utils/pgut.c b/src/utils/pgut.c index 330f2fa5..6948a0c1 100644 --- a/src/utils/pgut.c +++ b/src/utils/pgut.c @@ -1095,20 +1095,22 @@ key_equals(const char *lhs, const char *rhs) /* * Get configuration from configuration file. + * Return number of parsed options */ -void +int pgut_readopt(const char *path, pgut_option options[], int elevel) { FILE *fp; char buf[1024]; char key[1024]; char value[1024]; + int parsed_options = 0; if (!options) - return; + return parsed_options; if ((fp = pgut_fopen(path, "rt", true)) == NULL) - return; + return parsed_options; while (fgets(buf, lengthof(buf), fp)) { @@ -1127,18 +1129,23 @@ pgut_readopt(const char *path, pgut_option options[], int elevel) { if (opt->allowed < SOURCE_FILE && opt->allowed != SOURCE_FILE_STRICT) - elog(elevel, "option %s cannot specified in file", opt->lname); + elog(elevel, "option %s cannot be specified in file", opt->lname); else if (opt->source <= SOURCE_FILE) + { assign_option(opt, value, SOURCE_FILE); + parsed_options++; + } break; } } if (!options[i].type) - elog(elevel, "invalid option \"%s\"", key); + elog(elevel, "invalid option \"%s\" in file \"%s\"", key, path); } } fclose(fp); + + return parsed_options; } static const char * diff --git a/src/utils/pgut.h b/src/utils/pgut.h index 662f07c1..5e447a07 100644 --- a/src/utils/pgut.h +++ b/src/utils/pgut.h @@ -110,7 +110,7 @@ extern bool in_cleanup; extern bool in_password; /* User prompts password */ extern int pgut_getopt(int argc, char **argv, pgut_option options[]); -extern void pgut_readopt(const char *path, pgut_option options[], int elevel); +extern int pgut_readopt(const char *path, pgut_option options[], int elevel); extern void pgut_getopt_env(pgut_option options[]); extern void pgut_atexit_push(pgut_atexit_callback callback, void *userdata); extern void pgut_atexit_pop(pgut_atexit_callback callback, void *userdata); diff --git a/tests/option_test.py b/tests/option_test.py index a66d346e..34f7cd0f 100644 --- a/tests/option_test.py +++ b/tests/option_test.py @@ -199,7 +199,8 @@ class OptionTest(ProbackupTest, unittest.TestCase): self.add_instance(backup_dir, 'node', node) # invalid option in pg_probackup.conf - with open(os.path.join(backup_dir, "backups", "node", "pg_probackup.conf"), "a") as conf: + pbconf_path = os.path.join(backup_dir, "backups", "node", "pg_probackup.conf") + with open(pbconf_path, "a") as conf: conf.write("TIMELINEID=1\n") try: @@ -209,7 +210,7 @@ class OptionTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertEqual(e.message, - 'ERROR: invalid option "TIMELINEID"\n', + 'ERROR: invalid option "TIMELINEID" in file "{0}"\n'.format(pbconf_path), '\n Unexpected Error Message: {0}\n CMD: {1}'.format(repr(e.message), self.cmd)) # Clean after yourself diff --git a/tests/show_test.py b/tests/show_test.py index 29d0bdb3..931da184 100644 --- a/tests/show_test.py +++ b/tests/show_test.py @@ -110,3 +110,94 @@ class OptionTest(ProbackupTest, unittest.TestCase): # Clean after yourself self.del_test_dir(module_name, fname) + + # @unittest.skip("skip") + def test_no_control_file(self): + """backup.control doesn't exist""" + fname = self.id().split('.')[3] + backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') + node = self.make_simple_node( + base_dir="{0}/{1}/node".format(module_name, fname), + initdb_params=['--data-checksums'], + pg_options={'wal_level': 'replica'} + ) + + self.init_pb(backup_dir) + self.add_instance(backup_dir, 'node', node) + self.set_archiving(backup_dir, 'node', node) + node.start() + + backup_id = self.backup_node(backup_dir, 'node', node) + + # delete backup.control file + file = os.path.join( + backup_dir, "backups", "node", + backup_id, "backup.control") + os.remove(file) + + self.assertIn('control file "{0}" doesn\'t exist'.format(file), self.show_pb(backup_dir, 'node', as_text=True)) + + # Clean after yourself + self.del_test_dir(module_name, fname) + + # @unittest.skip("skip") + def test_empty_control_file(self): + """backup.control is empty""" + fname = self.id().split('.')[3] + backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') + node = self.make_simple_node( + base_dir="{0}/{1}/node".format(module_name, fname), + initdb_params=['--data-checksums'], + pg_options={'wal_level': 'replica'} + ) + + self.init_pb(backup_dir) + self.add_instance(backup_dir, 'node', node) + self.set_archiving(backup_dir, 'node', node) + node.start() + + backup_id = self.backup_node(backup_dir, 'node', node) + + # truncate backup.control file + file = os.path.join( + backup_dir, "backups", "node", + backup_id, "backup.control") + fd = open(file, 'w') + fd.close() + + self.assertIn('control file "{0}" is empty'.format(file), self.show_pb(backup_dir, 'node', as_text=True)) + + # Clean after yourself + self.del_test_dir(module_name, fname) + + # @unittest.skip("skip") + # @unittest.expectedFailure + def test_corrupt_control_file(self): + """backup.control contains invalid option""" + fname = self.id().split('.')[3] + backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') + node = self.make_simple_node( + base_dir="{0}/{1}/node".format(module_name, fname), + initdb_params=['--data-checksums'], + pg_options={'wal_level': 'replica'} + ) + + self.init_pb(backup_dir) + self.add_instance(backup_dir, 'node', node) + self.set_archiving(backup_dir, 'node', node) + node.start() + + backup_id = self.backup_node(backup_dir, 'node', node) + + # corrupt backup.control file + file = os.path.join( + backup_dir, "backups", "node", + backup_id, "backup.control") + fd = open(file, 'a') + fd.write("statuss = OK") + fd.close() + + self.assertIn('invalid option "statuss" in file'.format(file), self.show_pb(backup_dir, 'node', as_text=True)) + + # Clean after yourself + self.del_test_dir(module_name, fname)