Merge pull request #3699 from ThomasWaldmann/backports-19

Backports 19

src/borg/archiver.py

@@ -1719,6 +1719,46 @@ class Archiver:
     @with_repository(exclusive=True, cache=True, compatibility=(Manifest.Operation.WRITE,))
     def do_config(self, args, repository, manifest, key, cache):
         """get, set, and delete values in a repository or cache config file"""
+
+        def repo_validate(section, name, value=None, check_value=True):
+            if section not in ['repository', ]:
+                raise ValueError('Invalid section')
+            if name in ['segments_per_dir', 'max_segment_size', 'storage_quota', ]:
+                if check_value:
+                    try:
+                        int(value)
+                    except ValueError:
+                        raise ValueError('Invalid value') from None
+                    if name == 'max_segment_size':
+                        if int(value) >= MAX_SEGMENT_SIZE_LIMIT:
+                            raise ValueError('Invalid value: max_segment_size >= %d' % MAX_SEGMENT_SIZE_LIMIT)
+            elif name in ['additional_free_space', ]:
+                if check_value:
+                    try:
+                        parse_file_size(value)
+                    except ValueError:
+                        raise ValueError('Invalid value') from None
+            elif name in ['append_only', ]:
+                if check_value and value not in ['0', '1']:
+                    raise ValueError('Invalid value')
+            elif name in ['id', ]:
+                if check_value:
+                    try:
+                        bin_id = unhexlify(value)
+                    except:
+                        raise ValueError('Invalid value, must be 64 hex digits') from None
+                    if len(bin_id) != 32:
+                        raise ValueError('Invalid value, must be 64 hex digits')
+            else:
+                raise ValueError('Invalid name')
+
+        def cache_validate(section, name, value=None, check_value=True):
+            if section not in ['cache', ]:
+                raise ValueError('Invalid section')
+            # I looked at the cache config and did not see anything a user would want to edit,
+            # so, for now, raise for any key name
+            raise ValueError('Invalid name')
+
         try:
             section, name = args.name.split('.')
         except ValueError:
@@ -1729,16 +1769,20 @@ class Archiver:
             cache.cache_config.load()
             config = cache.cache_config._config
             save = cache.cache_config.save
+            validate = cache_validate
         else:
             config = repository.config
             save = lambda: repository.save_config(repository.path, repository.config)
+            validate = repo_validate
 
         if args.delete:
+            validate(section, name, check_value=False)
             config.remove_option(section, name)
             if len(config.options(section)) == 0:
                 config.remove_section(section)
             save()
         elif args.value:
+            validate(section, name, args.value)
             if section not in config.sections():
                 config.add_section(section)
             config.set(section, name, args.value)

src/borg/constants.py

@@ -36,6 +36,9 @@ MAX_DATA_SIZE = 20971479
 MAX_OBJECT_SIZE = MAX_DATA_SIZE + 41  # see LoggedIO.put_header_fmt.size assertion in repository module
 assert MAX_OBJECT_SIZE == 20 * 1024 * 1024
 
+# repo config max_segment_size value must be below this limit to stay within uint32 offsets:
+MAX_SEGMENT_SIZE_LIMIT = 2 ** 32 - MAX_OBJECT_SIZE
+
 # borg.remote read() buffer size
 BUFSIZE = 10 * 1024 * 1024
 

src/borg/patterns.py

@@ -367,17 +367,18 @@ def parse_inclexcl_command(cmd_line_str, fallback=ShellPattern):
         'P': IECommand.PatternStyle,
         'p': IECommand.PatternStyle,
     }
-
-    try:
-        cmd = cmd_prefix_map[cmd_line_str[0]]
-
-        # remaining text on command-line following the command character
-        remainder_str = cmd_line_str[1:].lstrip()
-
-        if not remainder_str:
-            raise ValueError("Missing pattern/information!")
-    except (IndexError, KeyError, ValueError):
-        raise argparse.ArgumentTypeError("Unable to parse pattern/command: {}".format(cmd_line_str))
+    if not cmd_line_str:
+        raise argparse.ArgumentTypeError("A pattern/command must not be empty.")
+
+    cmd = cmd_prefix_map.get(cmd_line_str[0])
+    if cmd is None:
+        raise argparse.ArgumentTypeError("A pattern/command must start with any one of: %s" %
+                                         ', '.join(cmd_prefix_map))
+
+    # remaining text on command-line following the command character
+    remainder_str = cmd_line_str[1:].lstrip()
+    if not remainder_str:
+        raise argparse.ArgumentTypeError("A pattern/command must have a value part.")
 
     if cmd is IECommand.RootPath:
         # TODO: validate string?

src/borg/repository.py

@@ -122,6 +122,9 @@ class Repository:
     class InvalidRepository(Error):
         """{} is not a valid repository. Check repo config."""
 
+    class InvalidRepositoryConfig(Error):
+        """{} does not have a valid configuration. Check repo config [{}]."""
+
     class AtticRepository(Error):
         """Attic repository detected. Please run "borg upgrade {}"."""
 
@@ -278,7 +281,8 @@ class Repository:
                 os.link(config_path, old_config_path)
             except OSError as e:
                 if e.errno in (errno.EMLINK, errno.ENOSYS, errno.EPERM, errno.ENOTSUP):
-                    logger.warning("Hardlink failed, cannot securely erase old config file")
+                    logger.warning("Failed to securely erase old repository config file (hardlinks not supported>). "
+                                   "Old repokey data, if any, might persist on physical storage.")
                 else:
                     raise
 
@@ -383,6 +387,9 @@ class Repository:
             self.close()
             raise self.InvalidRepository(path)
         self.max_segment_size = self.config.getint('repository', 'max_segment_size')
+        if self.max_segment_size >= MAX_SEGMENT_SIZE_LIMIT:
+            self.close()
+            raise self.InvalidRepositoryConfig(path, 'max_segment_size >= %d' % MAX_SEGMENT_SIZE_LIMIT)  # issue 3592
         self.segments_per_dir = self.config.getint('repository', 'segments_per_dir')
         self.additional_free_space = parse_file_size(self.config.get('repository', 'additional_free_space', fallback=0))
         # append_only can be set in the constructor

src/borg/testsuite/archiver.py

@@ -2782,21 +2782,18 @@ id: 2 / e29442 3506da 4e1ea7 / 25f62a 5a3d41 - 02
         self.create_test_files()
         os.unlink('input/flagfile')
         self.cmd('init', '--encryption=repokey', self.repository_location)
-        for cfg_key in {'testkey', 'testsection.testkey'}:
-            self.cmd('config', self.repository_location, cfg_key, exit_code=1)
-            self.cmd('config', self.repository_location, cfg_key, 'testcontents')
+        for cfg_key, cfg_value in [
+            ('additional_free_space', '2G'),
+            ('repository.append_only', '1'),
+        ]:
+            output = self.cmd('config', self.repository_location, cfg_key)
+            assert output == '0' + '\n'
+            self.cmd('config', self.repository_location, cfg_key, cfg_value)
             output = self.cmd('config', self.repository_location, cfg_key)
-            assert output == 'testcontents\n'
+            assert output == cfg_value + '\n'
             self.cmd('config', self.repository_location, '--delete', cfg_key)
             self.cmd('config', self.repository_location, cfg_key, exit_code=1)
 
-            self.cmd('config', self.repository_location, '--cache', cfg_key, exit_code=1)
-            self.cmd('config', self.repository_location, '--cache', cfg_key, 'testcontents')
-            output = self.cmd('config', self.repository_location, '--cache', cfg_key)
-            assert output == 'testcontents\n'
-            self.cmd('config', self.repository_location, '--cache', '--delete', cfg_key)
-            self.cmd('config', self.repository_location, '--cache', cfg_key, exit_code=1)
-
     requires_gnutar = pytest.mark.skipif(not have_gnutar(), reason='GNU tar must be installed for this test.')
     requires_gzip = pytest.mark.skipif(not shutil.which('gzip'), reason='gzip must be installed for this test.')