From 9aa93c05c1a4b88ab939660737db3d5e70d1b86d Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 22 Dec 2016 15:35:29 -0800 Subject: [PATCH] Simplify the ocsp_revoked() return type - we weren't reacting to None, so call it False instead --- certbot/ocsp.py | 12 ++++++------ certbot/tests/ocsp_test.py | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/certbot/ocsp.py b/certbot/ocsp.py index a5df5743d..2e0514a44 100644 --- a/certbot/ocsp.py +++ b/certbot/ocsp.py @@ -37,17 +37,17 @@ class RevocationChecker(object): :param str cert_path: Path to certificate :param str chain_path: Path to intermediate cert :rtype bool or None: - :returns: False if valid; True if revoked; None if check itself failed + :returns: True if revoked; False if valid or the check failed """ if self.broken: - return None + return False logger.debug("Querying OCSP for %s", cert_path) url, host = self.determine_ocsp_server(cert_path) if not host: - return None + return False # jdkasten thanks "Bulletproof SSL and TLS - Ivan Ristic" for documenting this! cmd = ["openssl", "ocsp", "-no_nonce", @@ -62,7 +62,7 @@ class RevocationChecker(object): except errors.SubprocessError as e: logger.info("OCSP check failed for %s (are we offline?)", cert_path) logger.debug("Command was:\n%s\nError was:\n%s", " ".join(cmd), e) - return None + return False return _translate_ocsp_query(cert_path, output, err) @@ -98,12 +98,12 @@ def _translate_ocsp_query(cert_path, ocsp_output, ocsp_errors): if not "Response verify OK" in ocsp_errors: logger.info("Revocation status for %s is unknown", cert_path) logger.debug("Uncertain ouput:\n%s\nstderr:\n%s", ocsp_output, ocsp_errors) - return None + return False if cert_path + ": good" in ocsp_output: return False elif cert_path + ": revoked" in ocsp_output: return True else: logger.warn("Unable to properly parse OCSP output: %s", ocsp_output) - return None + return False diff --git a/certbot/tests/ocsp_test.py b/certbot/tests/ocsp_test.py index c4517174d..ff79bb01e 100644 --- a/certbot/tests/ocsp_test.py +++ b/certbot/tests/ocsp_test.py @@ -59,17 +59,17 @@ class OCSPTest(unittest.TestCase): def test_ocsp_revoked(self, mock_run, mock_determine): self.checker.broken = True mock_determine.return_value = ("", "") - self.assertEqual(self.checker.ocsp_revoked("x", "y"), None) + self.assertEqual(self.checker.ocsp_revoked("x", "y"), False) self.checker.broken = False mock_run.return_value = tuple(openssl_happy[1:]) - self.assertEqual(self.checker.ocsp_revoked("x", "y"), None) + self.assertEqual(self.checker.ocsp_revoked("x", "y"), False) self.assertEqual(mock_run.call_count, 0) mock_determine.return_value = ("http://x.co", "x.co") self.assertEqual(self.checker.ocsp_revoked("blah.pem", "chain.pem"), False) mock_run.side_effect = errors.SubprocessError("Unable to load certificate launcher") - self.assertEqual(self.checker.ocsp_revoked("x", "y"), None) + self.assertEqual(self.checker.ocsp_revoked("x", "y"), False) self.assertEqual(mock_run.call_count, 2) @@ -97,10 +97,10 @@ class OCSPTest(unittest.TestCase): mock_run.return_value = openssl_confused from certbot import ocsp self.assertEqual(ocsp._translate_ocsp_query(*openssl_happy), False) - self.assertEqual(ocsp._translate_ocsp_query(*openssl_confused), None) + self.assertEqual(ocsp._translate_ocsp_query(*openssl_confused), False) self.assertEqual(mock_log.debug.call_count, 1) self.assertEqual(mock_log.warn.call_count, 0) - self.assertEqual(ocsp._translate_ocsp_query(*openssl_broken), None) + self.assertEqual(ocsp._translate_ocsp_query(*openssl_broken), False) self.assertEqual(mock_log.warn.call_count, 1) self.assertEqual(ocsp._translate_ocsp_query(*openssl_revoked), True)