From 6c953a414f2b8ddb0ba489b6b43b4fea73e9c529 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Mar 29 2018 14:54:02 +0000
Subject: consolidate ConfigParser usage


Fixes: https://pagure.io/koji/issue/865

---

diff --git a/builder/kojid b/builder/kojid
index 726d535..8cf130c 100755
--- a/builder/kojid
+++ b/builder/kojid
@@ -56,7 +56,7 @@ import xmlrpclib
 import zipfile
 import copy
 import Cheetah.Template
-from ConfigParser import ConfigParser
+from ConfigParser import SafeConfigParser
 from fnmatch import fnmatch
 from gzip import GzipFile
 from optparse import OptionParser, SUPPRESS_HELP
@@ -5610,7 +5610,7 @@ def get_options():
         assert False  # pragma: no cover
 
     # load local config
-    config = ConfigParser()
+    config = SafeConfigParser()
     config.read(options.configFile)
     for x in config.sections():
         if x != 'kojid':
diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py
index a6f338a..7c3117b 100644
--- a/cli/koji_cli/commands.py
+++ b/cli/koji_cli/commands.py
@@ -5645,10 +5645,8 @@ def handle_image_build(options, session, args):
         if not os.path.exists(task_options.config):
             parser.error(_("%s not found!" % task_options.config))
         section = 'image-build'
-        config = six.moves.configparser.ConfigParser()
-        conf_fd = open(task_options.config)
-        config.readfp(conf_fd)
-        conf_fd.close()
+        config = six.moves.configparser.SafeConfigParser()
+        config.read(task_options.config)
         if not config.has_section(section):
             parser.error(_("single section called [%s] is required" % section))
         # pluck out the positional arguments first
diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py
index f518da8..677bd44 100644
--- a/hub/kojixmlrpc.py
+++ b/hub/kojixmlrpc.py
@@ -18,7 +18,7 @@
 # Authors:
 #       Mike McLean <mikem@redhat.com>
 
-from ConfigParser import RawConfigParser
+from ConfigParser import SafeConfigParser
 import datetime
 import inspect
 import logging
@@ -381,16 +381,11 @@ def load_config(environ):
     """
     logger = logging.getLogger("koji")
     #get our config file(s)
-    cf = environ.get('koji.hub.ConfigFile', '/etc/koji-hub/hub.conf')
-    cfdir = environ.get('koji.hub.ConfigDir', '/etc/koji-hub/hub.conf.d')
-    if cfdir:
-        configs = koji.config_directory_contents(cfdir)
-    else:
-        configs = []
-    if cf and os.path.isfile(cf):
-        configs.append(cf)
+    cfs = [environ.get('koji.hub.ConfigFile', '/etc/koji-hub/hub.conf')] or []
+    cfdirs = [environ.get('koji.hub.ConfigDir', '/etc/koji-hub/hub.conf.d')] or []
+    configs = koji.get_config_files(cfdirs, cfs)
     if configs:
-        config = RawConfigParser()
+        config = SafeConfigParser()
         config.read(configs)
     else:
         config = None
diff --git a/koji/__init__.py b/koji/__init__.py
index 24347bd..bff082f 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -1630,6 +1630,34 @@ def config_directory_contents(dir_name):
     return configs
 
 
+def get_config_files(dirs=None, files=None, strict=False):
+    # get list of config files in given directories and files, order is
+    # preserved
+    configs = []
+    if dirs is not None:
+        for d in dirs:
+            cfgs = config_directory_contents(d)
+            if cfgs:
+                configs.extend(cfgs)
+            else:
+                logging.debug("No config files found in directory: %s" % d)
+    if files is not None:
+        configs += files
+
+    # check access
+    result = []
+    for f in configs:
+        if os.path.isfile(f) and os.access(f, os.F_OK):
+            result.append(f)
+        else:
+            if strict:
+                raise ConfigurationError("Config file %s can't be opened." % f)
+            else:
+                logging.warn("Config file %s can't be opened." % f)
+
+    return result
+
+
 def read_config(profile_name, user_config=None):
     config_defaults = {
         'server' : 'http://localhost/kojihub',
@@ -1667,42 +1695,25 @@ def read_config(profile_name, user_config=None):
     #note: later config files override earlier ones
 
     # /etc/koji.conf.d
-    configs = config_directory_contents('/etc/koji.conf.d')
-
-    # /etc/koji.conf
-    if os.access('/etc/koji.conf', os.F_OK):
-        configs.append('/etc/koji.conf')
+    config_dirs = ['/etc/koji.conf.d']
+    config_files = ['/etc/koji.conf']
 
     # User specific configuration
     if user_config:
         # Config file specified on command line
-        fn = os.path.expanduser(user_config)
-        if os.path.isdir(fn):
-            # Specified config is a directory
-            contents = config_directory_contents(fn)
-            if not contents:
-                raise ConfigurationError("No config files found in directory: %s" % fn)
-            configs.extend(contents)
-        else:
-            # Specified config is a file
-            if not os.access(fn, os.F_OK):
-                raise ConfigurationError("No such file: %s" % fn)
-            configs.append(fn)
+        config_dirs.append(os.path.expanduser(user_config))
     else:
         # User config
-        user_config_dir = os.path.expanduser("~/.koji/config.d")
-        configs.extend(config_directory_contents(user_config_dir))
-        fn = os.path.expanduser("~/.koji/config")
-        if os.access(fn, os.F_OK):
-            configs.append(fn)
+        config_dirs.append(os.path.expanduser("~/.koji/config.d"))
+        config_files.append(os.path.expanduser("~/.koji/config"))
+
+    configs = get_config_files(dirs=config_dirs, files=config_files)
 
     # Load the configs in a particular order
     got_conf = False
     for configFile in configs:
-        f = open(configFile)
-        config = six.moves.configparser.ConfigParser()
-        config.readfp(f)
-        f.close()
+        config = six.moves.configparser.SafeConfigParser()
+        config.read(configFile)
         if config.has_section(profile_name):
             got_conf = True
             for name, value in config.items(profile_name):
diff --git a/koji/util.py b/koji/util.py
index eaf7813..f34b6d1 100644
--- a/koji/util.py
+++ b/koji/util.py
@@ -667,11 +667,9 @@ def parse_maven_params(confs, chain=False, scratch=False):
     """
     if not isinstance(confs, (list, tuple)):
         confs = [confs]
-    config = six.moves.configparser.ConfigParser()
-    for conf in confs:
-        conf_fd = open(conf)
-        config.readfp(conf_fd)
-        conf_fd.close()
+    configs = koji.get_config_files(files=confs)
+    config = six.moves.configparser.SafeConfigParser()
+    config.read(configs)
     builds = {}
     for package in config.sections():
         buildtype = 'maven'
diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py
index 85165ba..08f84ea 100644
--- a/plugins/hub/protonmsg.py
+++ b/plugins/hub/protonmsg.py
@@ -270,8 +270,7 @@ def send_queued_msgs(cbtype, *args, **kws):
     global CONFIG
     if not CONFIG:
         conf = ConfigParser.SafeConfigParser()
-        with open(CONFIG_FILE) as conffile:
-            conf.readfp(conffile)
+        conf.read(CONFIG_FILE)
         CONFIG = conf
     urls = CONFIG.get('broker', 'urls').split()
     test_mode = False
diff --git a/plugins/hub/rpm2maven.py b/plugins/hub/rpm2maven.py
index ad78b8f..bcb8448 100644
--- a/plugins/hub/rpm2maven.py
+++ b/plugins/hub/rpm2maven.py
@@ -12,7 +12,6 @@ from koji.util import rmtree
 import ConfigParser
 import fnmatch
 import os
-import shutil
 import subprocess
 
 CONFIG_FILE = '/etc/koji-hub/plugins/rpm2maven.conf'
diff --git a/tests/test_cli/test_image_build.py b/tests/test_cli/test_image_build.py
index e1e0f10..1e9de92 100644
--- a/tests/test_cli/test_image_build.py
+++ b/tests/test_cli/test_image_build.py
@@ -213,10 +213,10 @@ class TestImageBuild(utils.CliTestCase):
             return self.custom_os_path_exists[filepath]
         return self.os_path_exists(filepath)
 
-    def mock_builtin_open(self, filepath, *args):
+    def mock_builtin_open(self, filepath, *args, **kwargs):
         if filepath in self.custom_open:
             return self.custom_open[filepath]
-        return self.builtin_open(filepath, *args)
+        return self.builtin_open(filepath, *args, **kwargs)
 
     def setUp(self):
         self.os_path_exists = os.path.exists
@@ -307,12 +307,14 @@ factory_test_ver=1.0
 """
         self.custom_open[config_file] = six.StringIO(fake_config)
 
-        with mock.patch('os.path.exists', new=self.mock_os_path_exists), \
-                mock.patch('koji_cli.commands.open', new=self.mock_builtin_open):
-            handle_image_build(
-                self.options,
-                self.session,
-                ['--config', config_file])
+        if six.PY2:
+            with mock.patch('os.path.exists', new=self.mock_os_path_exists), \
+                    mock.patch('__builtin__.open', new=self.mock_builtin_open):
+                handle_image_build(self.options, self.session, ['--config', config_file])
+        else:
+            with mock.patch('os.path.exists', new=self.mock_os_path_exists), \
+                    mock.patch('builtins.open', new=self.mock_builtin_open):
+                handle_image_build(self.options, self.session, ['--config', config_file])
 
         args, kwargs = build_image_oz_mock.call_args
         self.assertDictEqual(TASK_OPTIONS, args[1].__dict__)
diff --git a/tests/test_lib/test_profiles.py b/tests/test_lib/test_profiles.py
index 0417023..4c23aec 100644
--- a/tests/test_lib/test_profiles.py
+++ b/tests/test_lib/test_profiles.py
@@ -40,6 +40,3 @@ def stress(errors, n):
         return
     else:
         errors[n] = None
-
-
-
diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py
index 226534a..4fd3819 100644
--- a/tests/test_lib/test_utils.py
+++ b/tests/test_lib/test_utils.py
@@ -487,7 +487,7 @@ class MavenUtilTestCase(unittest.TestCase):
         self.assertEqual(cm.exception.args[0], 'total ordering not possible')
 
     def _read_conf(self, cfile):
-        config = six.moves.configparser.ConfigParser()
+        config = six.moves.configparser.SafeConfigParser()
         path = os.path.dirname(__file__)
         with open(path + cfile, 'r') as conf_file:
             config.readfp(conf_file)
diff --git a/util/koji-gc b/util/koji-gc
index 89cc492..6675000 100755
--- a/util/koji-gc
+++ b/util/koji-gc
@@ -114,21 +114,14 @@ def get_options():
 
     defaults = parser.get_default_values()
 
-    config = ConfigParser.ConfigParser()
-    cf = getattr(options, 'config_file', None)
-    if cf:
-        if not os.access(cf, os.F_OK):
-            parser.error(_("No such file: %s") % cf)
-            assert False  # pragma: no cover
-    else:
-        cf = '/etc/koji-gc/koji-gc.conf'
-        if not os.access(cf, os.F_OK):
-            cf = None
-    if not cf:
+    config = ConfigParser.SafeConfigParser()
+    cf = getattr(options, 'config_file', '/etc/koji-gc/koji-gc.conf')
+    configs = koji.get_config_files(files=[cf])
+    if not configs:
         print("no config file")
         config = None
     else:
-        config.read(cf)
+        config.read(configs)
         # List of values read from config file to update default parser values
         cfgmap = [
             # name, alias, type
diff --git a/util/koji-shadow b/util/koji-shadow
index a958c9c..6a2ae3d 100755
--- a/util/koji-shadow
+++ b/util/koji-shadow
@@ -160,21 +160,14 @@ def get_options():
     (options, args) = parser.parse_args()
 
     defaults = parser.get_default_values()
-    config = ConfigParser.ConfigParser()
-    cf = getattr(options, 'config_file', None)
-    if cf:
-        if not os.access(cf, os.F_OK):
-            parser.error(_("No such file: %s") % cf)
-            assert False  # pragma: no cover
-    else:
-        cf = '/etc/koji-shadow/koji-shadow.conf'
-        if not os.access(cf, os.F_OK):
-            cf = None
-    if not cf:
+    config = ConfigParser.SafeConfigParser()
+    cf = getattr(options, 'config_file', '/etc/koji-shadow/koji-shadow.conf')
+    configs = koji.get_config_files(files=[cf])
+    if not configs:
         log("no config file")
         config = None
     else:
-        config.read(cf)
+        config.read(configs)
         #allow config file to update defaults
         for opt in parser.option_list:
             if not opt.dest:
diff --git a/util/kojira b/util/kojira
index bf19894..32f5e9d 100755
--- a/util/kojira
+++ b/util/kojira
@@ -25,7 +25,7 @@ import os
 import koji
 from koji.util import rmtree, parseStatus
 from optparse import OptionParser
-from ConfigParser import ConfigParser
+from ConfigParser import SafeConfigParser
 import errno
 import logging
 import logging.handlers
@@ -811,7 +811,7 @@ def get_options():
     parser.add_option("--logfile", help="Specify logfile")
     (options, args) = parser.parse_args()
 
-    config = ConfigParser()
+    config = SafeConfigParser()
     config.read(options.configFile)
     section = 'kojira'
     for x in config.sections():
diff --git a/vm/kojikamid.py b/vm/kojikamid.py
index 1d40e68..6412e37 100755
--- a/vm/kojikamid.py
+++ b/vm/kojikamid.py
@@ -27,7 +27,7 @@
 # in a cygwin shell.
 
 from optparse import OptionParser
-from ConfigParser import ConfigParser
+from ConfigParser import SafeConfigParser
 import os
 import subprocess
 import sys
@@ -201,7 +201,7 @@ class WindowsBuild(object):
         elif len(specfiles) > 1:
             raise BuildError('Multiple .ini files found')
 
-        conf = ConfigParser()
+        conf = SafeConfigParser()
         conf.read(os.path.join(self.spec_dir, specfiles[0]))
 
         # [naming] section
diff --git a/vm/kojivmd b/vm/kojivmd
index 41789ec..fe3f489 100755
--- a/vm/kojivmd
+++ b/vm/kojivmd
@@ -42,7 +42,7 @@ import base64
 import pwd
 import requests
 import fnmatch
-from ConfigParser import ConfigParser
+from ConfigParser import SafeConfigParser
 from contextlib import closing
 from optparse import OptionParser
 try:
@@ -98,7 +98,7 @@ def get_options():
         assert False  # pragma: no cover
 
     # load local config
-    config = ConfigParser()
+    config = SafeConfigParser()
     config.read(options.configFile)
     for x in config.sections():
         if x != 'kojivmd':
diff --git a/www/kojiweb/wsgi_publisher.py b/www/kojiweb/wsgi_publisher.py
index 778d992..1ed6ee4 100644
--- a/www/kojiweb/wsgi_publisher.py
+++ b/www/kojiweb/wsgi_publisher.py
@@ -29,7 +29,7 @@ import pprint
 import sys
 import traceback
 
-from ConfigParser import RawConfigParser
+from ConfigParser import SafeConfigParser
 from koji.server import ServerError, ServerRedirect
 from koji.util import dslice
 
@@ -123,16 +123,11 @@ class Dispatcher(object):
             - all PythonOptions (except koji.web.ConfigFile) are now deprecated and
               support for them will disappear in a future version of Koji
         """
-        cf = environ.get('koji.web.ConfigFile', '/etc/kojiweb/web.conf')
-        cfdir = environ.get('koji.web.ConfigDir', '/etc/kojiweb/web.conf.d')
-        if cfdir:
-            configs = koji.config_directory_contents(cfdir)
-        else:
-            configs = []
-        if cf and os.path.isfile(cf):
-            configs.append(cf)
+        cfs = [environ.get('koji.web.ConfigFile', '/etc/kojiweb/web.conf')] or []
+        cfdirs = [environ.get('koji.web.ConfigDir', '/etc/kojiweb/web.conf.d')] or []]
+        configs = koji.get_config_files(dirs=cfdirs, files=cfs)
         if configs:
-            config = RawConfigParser()
+            config = SafeConfigParser()
             config.read(configs)
         else:
             raise koji.GenericError("Configuration missing")