#1250 backend: handle invalid copr-repo calls
Merged 4 years ago by praiskup. Opened 4 years ago by praiskup.

@@ -556,6 +556,10 @@

          if not subdirs:

              return []

          for subdir in subdirs:

+             if subdir is None:

+                 # this should never happen, but better to skip

+                 # this than kill some backend process

+                 continue

              args += [option, subdir]

          return args

      cmd += subdirs('--add', add)

file modified
+19 -2
@@ -44,11 +44,28 @@

      return sp.returncode

  

  

+ def arg_parser_subdir_type(subdir):

+     if not subdir:

+         raise argparse.ArgumentTypeError("subdir can not be empty string")

+     if '..' in subdir:

+         raise argparse.ArgumentTypeError(

+                 "relative '..' in subdir name '{}'".format(subdir))

+     if ' ' in subdir:

+         raise argparse.ArgumentTypeError(

+                 "space character in subdir name '{}'".format(subdir))

+     if '/' in subdir:

+         raise argparse.ArgumentTypeError(

+                 "'/' in subdir name '{}', we support only single-level subdir "

+                 "for now".format(subdir))

+     return subdir

+ 

+ 

  def get_arg_parser():

      parser = argparse.ArgumentParser()

      parser.add_argument('--delete', action='append', metavar='SUBDIR',

-                         default=[])

-     parser.add_argument('--add', action='append', metavar='SUBDIR', default=[])

+                         default=[], type=arg_parser_subdir_type)

+     parser.add_argument('--add', action='append', metavar='SUBDIR', default=[],

+                         type=arg_parser_subdir_type)

      parser.add_argument('--devel', action='store_true', default=False)

      parser.add_argument('--no-appstream-metadata', action='store_true',

                          default=False)

@@ -100,3 +100,24 @@

          # wouldn't be able to reference ../ locations

          assert updated['packages']['prunerepo']['xml:base'] == \

                  'https://example.com/results/john/empty/fedora-rawhide-x86_64'

+ 

+     @pytest.mark.parametrize('add', [

+         ["aaa", ''],

+         ["File 1"],

+         ["slash/in/path"],

+         [".."],

+     ])

+     def test_copr_repo_subdir_validator(self, add, capfd):

+         assert 0 == call_copr_repo('/some/dir', add=add)

+         # not using capsys because pytest/issues/5997

+         _, err = capfd.readouterr()

+         assert 'copr-repo: error: argument' in err

+ 

+     @mock.patch("backend.helpers.subprocess.call")

+     def test_copr_repo_subdir_none_doesnt_raise(self, call):

+         """ check that None is skipped in add (or delete) """

+         call.return_value = 0 # exit status 0

+         assert True == call_copr_repo('/some/dir', add=['xxx', None])

+         assert len(call.call_args_list) == 1

+         call = call.call_args_list[0]

+         assert call[0][0] == ['copr-repo', '/some/dir', '--add', 'xxx']

rebased onto ce71d7f7ab4d2287cf91b50fcf733f9b8eaeb2ba

4 years ago

Why using value as a parameter, only to be copied to subdir and never to use it again?

rebased onto 2ff17a37d2f7a1d56e22a693380f96b88323d806

4 years ago

thanks for the review, ptal

rebased onto e92bfbc

4 years ago

Pull-Request has been merged by praiskup

4 years ago