Merge pull request #467 from jdufresne/resource-warning

Clean up unclosed files causing  ResourceWarnings in tests

src/saml2/algsupport.py

@@ -36,10 +36,9 @@
 def get_algorithm_support(xmlsec):
     com_list = [xmlsec, '--list-transforms']
     pof = Popen(com_list, stderr=PIPE, stdout=PIPE)
-
-    p_out = pof.stdout.read().decode('utf-8')
-    p_err = pof.stderr.read().decode('utf-8')
-    pof.wait()
+    p_out, p_err = pof.communicate()
+    p_out = p_out.decode('utf-8')
+    p_err = p_err.decode('utf-8')
 
     if not p_err:
         p = p_out.splitlines()

src/saml2/mdstore.py

@@ -665,7 +665,8 @@ def __init__(self, attrc, filename=None, cert=None, **kwargs):
         self.cert = cert
 
     def get_metadata_content(self):
-        return open(self.filename, 'rb').read()
+        with open(self.filename, 'rb') as fp:
+            return fp.read()
 
     def load(self, *args, **kwargs):
         _txt = self.get_metadata_content()
@@ -768,7 +769,9 @@ def __init__(self, attrc, filename, **kwargs):
         self.filename = filename
 
     def load(self, *args, **kwargs):
-        for key, item in json.loads(open(self.filename).read()):
+        with open(self.filename) as fp:
+            data = json.load(fp)
+        for key, item in data:
             self.entity[key] = item
 
 

src/saml2/metadata.py

@@ -701,15 +701,14 @@ def entity_descriptor(confd):
     enc_cert = None
     if confd.cert_file is not None:
         mycert = []
-        mycert.append("".join(open(confd.cert_file).readlines()[1:-1]))
+        mycert.append("".join(read_cert(confd.cert_file)))
         if confd.additional_cert_files is not None:
             for _cert_file in confd.additional_cert_files:
-                mycert.append("".join(open(_cert_file).readlines()[1:-1]))
+                mycert.append("".join(read_cert(_cert_file)))
     if confd.encryption_keypairs is not None:
         enc_cert = []
         for _encryption in confd.encryption_keypairs:
-            enc_cert.append(
-                "".join(open(_encryption["cert_file"]).readlines()[1:-1]))
+            enc_cert.append("".join(read_cert(_encryption["cert_file"])))
 
     entd = md.EntityDescriptor()
     entd.entity_id = confd.entityid
@@ -821,3 +820,9 @@ def sign_entity_descriptor(edesc, ident, secc, sign_alg=None, digest_alg=None):
     xmldoc = secc.sign_statement("%s" % edesc, class_name(edesc))
     edesc = md.entity_descriptor_from_string(xmldoc)
     return edesc, xmldoc
+
+
+def read_cert(path):
+    with open(path) as fp:
+        lines = fp.readlines()
+    return lines[1:-1]

src/saml2/s_utils.py

@@ -194,10 +194,11 @@ def parse_attribute_map(filenames):
     forward = {}
     backward = {}
     for filename in filenames:
-        for line in open(filename).readlines():
-            (name, friendly_name, name_format) = line.strip().split()
-            forward[(name, name_format)] = friendly_name
-            backward[friendly_name] = (name, name_format)
+        with open(filename) as fp:
+            for line in fp:
+                (name, friendly_name, name_format) = line.strip().split()
+                forward[(name, name_format)] = friendly_name
+                backward[friendly_name] = (name, name_format)
 
     return forward, backward
 

src/saml2/sigver.py

@@ -787,8 +787,8 @@ def __init__(self, xmlsec_binary, **kwargs):
     def version(self):
         com_list = [self.xmlsec, "--version"]
         pof = Popen(com_list, stderr=PIPE, stdout=PIPE)
-        content = pof.stdout.read().decode('ascii')
-        pof.wait()
+        content, _ = pof.communicate()
+        content = content.decode('ascii')
         try:
             return content.split(" ")[1]
         except IndexError:
@@ -971,32 +971,30 @@ def _run_xmlsec(self, com_list, extra_args, validate_output=True,
         :param exception: The exception class to raise on errors
         :result: Whatever xmlsec wrote to an --output temporary file
         """
-        ntf = NamedTemporaryFile(suffix=".xml",
-                                 delete=self._xmlsec_delete_tmpfiles)
-        com_list.extend(["--output", ntf.name])
-        com_list += extra_args
+        with NamedTemporaryFile(suffix=".xml", delete=self._xmlsec_delete_tmpfiles) as ntf:
+            com_list.extend(["--output", ntf.name])
+            com_list += extra_args
 
-        logger.debug("xmlsec command: %s", " ".join(com_list))
+            logger.debug("xmlsec command: %s", " ".join(com_list))
 
-        pof = Popen(com_list, stderr=PIPE, stdout=PIPE)
+            pof = Popen(com_list, stderr=PIPE, stdout=PIPE)
+            p_out, p_err = pof.communicate()
+            p_out = p_out.decode('utf-8')
+            p_err = p_err.decode('utf-8')
 
-        p_out = pof.stdout.read().decode('utf-8')
-        p_err = pof.stderr.read().decode('utf-8')
-        pof.wait()
+            if pof.returncode is not None and pof.returncode < 0:
+                logger.error(LOG_LINE, p_out, p_err)
+                raise XmlsecError("%d:%s" % (pof.returncode, p_err))
 
-        if pof.returncode is not None and pof.returncode < 0:
-            logger.error(LOG_LINE, p_out, p_err)
-            raise XmlsecError("%d:%s" % (pof.returncode, p_err))
+            try:
+                if validate_output:
+                    parse_xmlsec_output(p_err)
+            except XmlsecError as exc:
+                logger.error(LOG_LINE_2, p_out, p_err, exc)
+                raise
 
-        try:
-            if validate_output:
-                parse_xmlsec_output(p_err)
-        except XmlsecError as exc:
-            logger.error(LOG_LINE_2, p_out, p_err, exc)
-            raise
-
-        ntf.seek(0)
-        return p_out, p_err, ntf.read()
+            ntf.seek(0)
+            return p_out, p_err, ntf.read()
 
 
 class CryptoBackendXMLSecurity(CryptoBackend):

tests/test_30_mdstore.py

@@ -375,7 +375,8 @@ def test_load_extern_incommon():
 
 def test_load_local():
     # string representation of XML idp definition
-    idp_metadata = open(full_path("metadata.xml")).read()
+    with open(full_path("metadata.xml")) as fp:
+        idp_metadata = fp.read()
 
     saml_config = Config()
 

tests/test_31_config.py

@@ -322,7 +322,11 @@ def test_conf_syslog():
 
     # otherwise the logger setting is not changed
     root_logger.level = logging.NOTSET
-    root_logger.handlers = []
+    while root_logger.handlers:
+        handler = root_logger.handlers[-1]
+        root_logger.removeHandler(handler)
+        handler.flush()
+        handler.close()
 
     print(c.logger)
     c.setup_logger()

tests/test_40_sigver.py

@@ -75,7 +75,8 @@ def _eq(l1, l2):
 
 
 def test_cert_from_instance_1():
-    xml_response = open(SIGNED).read()
+    with open(SIGNED) as fp:
+        xml_response = fp.read()
     response = samlp.response_from_string(xml_response)
     assertion = response.assertion[0]
     certs = sigver.cert_from_instance(assertion)
@@ -87,7 +88,8 @@ def test_cert_from_instance_1():
 @pytest.mark.skipif(not decoder,
                     reason="pyasn1 is not installed")
 def test_cert_from_instance_ssp():
-    xml_response = open(SIMPLE_SAML_PHP_RESPONSE).read()
+    with open(SIMPLE_SAML_PHP_RESPONSE) as fp:
+        xml_response = fp.read()
     response = samlp.response_from_string(xml_response)
     assertion = response.assertion[0]
     certs = sigver.cert_from_instance(assertion)
@@ -150,13 +152,15 @@ def setup_class(self):
         )
 
     def test_verify_1(self):
-        xml_response = open(SIGNED).read()
+        with open(SIGNED) as fp:
+            xml_response = fp.read()
         response = self.sec.correctly_signed_response(xml_response)
         assert response
 
     def test_non_verify_1(self):
         """ unsigned is OK """
-        xml_response = open(UNSIGNED).read()
+        with open(UNSIGNED) as fp:
+            xml_response = fp.read()
         response = self.sec.correctly_signed_response(xml_response)
         assert response
 

tests/test_41_response.py

@@ -94,7 +94,8 @@ def test_2(self):
         assert isinstance(resp, AuthnResponse)
 
     def test_false_sign(self):
-        xml_response = open(FALSE_ASSERT_SIGNED).read()
+        with open(FALSE_ASSERT_SIGNED) as fp:
+            xml_response = fp.read()
         resp = response_factory(
             xml_response, self.conf,
             return_addrs=["http://lingon.catalogix.se:8087/"],
@@ -113,7 +114,8 @@ def test_false_sign(self):
             assert False
 
     def test_other_response(self):
-        xml_response = open(full_path("attribute_response.xml")).read()
+        with open(full_path("attribute_response.xml")) as fp:
+            xml_response = fp.read()
         resp = response_factory(
             xml_response, self.conf,
             return_addrs=['https://myreviewroom.com/saml2/acs/'],

tests/test_44_authnresp.py

@@ -95,7 +95,8 @@ def test_verify_signed_1(self):
         assert self.ar.name_id
 
     def test_parse_2(self):
-        xml_response = open(XML_RESPONSE_FILE).read()
+        with open(XML_RESPONSE_FILE) as fp:
+            xml_response = fp.read()
         ID = "bahigehogffohiphlfmplepdpcohkhhmheppcdie"
         self.ar.outstanding_queries = {ID: "http://localhost:8088/foo"}    
         self.ar.return_addr = "http://xenosmilus.umdc.umu.se:8087/login"