From 46052f826c31b07bb3fad9e7343cf770ccce87ab Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 3 Oct 2017 02:18:37 +0300 Subject: [PATCH] Handle NoneType from Augeas better in Apache parser get_arg (#5135) * Fix #4245 * Simpler, more accurate test * Do not add empty values to parser modules * Py26 fix --- certbot-apache/certbot_apache/parser.py | 16 ++++++++++++---- .../certbot_apache/tests/parser_test.py | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py index db690f547..b15608d61 100644 --- a/certbot-apache/certbot_apache/parser.py +++ b/certbot-apache/certbot_apache/parser.py @@ -123,9 +123,14 @@ class ApacheParser(object): for match_name, match_filename in six.moves.zip( iterator, iterator): - self.modules.add(self.get_arg(match_name)) - self.modules.add( - os.path.basename(self.get_arg(match_filename))[:-2] + "c") + mod_name = self.get_arg(match_name) + mod_filename = self.get_arg(match_filename) + if mod_name and mod_filename: + self.modules.add(mod_name) + self.modules.add(os.path.basename(mod_filename)[:-2] + "c") + else: + logger.debug("Could not read LoadModule directive from " + + "Augeas path: {0}".format(match_name[6:])) def update_runtime_variables(self): """" @@ -371,7 +376,10 @@ class ApacheParser(object): # Note: normal argument may be a quoted variable # e.g. strip now, not later - value = value.strip("'\"") + if not value: + return None + else: + value = value.strip("'\"") variables = ApacheParser.arg_var_interpreter.findall(value) diff --git a/certbot-apache/certbot_apache/tests/parser_test.py b/certbot-apache/certbot_apache/tests/parser_test.py index b4283aa32..38c5b29c7 100644 --- a/certbot-apache/certbot_apache/tests/parser_test.py +++ b/certbot-apache/certbot_apache/tests/parser_test.py @@ -66,6 +66,10 @@ class BasicParserTest(util.ParserTest): for i, match in enumerate(matches): self.assertEqual(self.parser.aug.get(match), str(i + 1)) + def test_empty_arg(self): + self.assertEquals(None, + self.parser.get_arg("/files/whatever/nonexistent")) + def test_add_dir_to_ifmodssl(self): """test add_dir_to_ifmodssl. @@ -114,6 +118,16 @@ class BasicParserTest(util.ParserTest): self.assertEqual(results["default"], results["listen"]) self.assertEqual(results["default"], results["name"]) + @mock.patch("certbot_apache.parser.ApacheParser.find_dir") + @mock.patch("certbot_apache.parser.ApacheParser.get_arg") + def test_init_modules_bad_syntax(self, mock_arg, mock_find): + mock_find.return_value = ["1", "2", "3", "4", "5", "6", "7", "8"] + mock_arg.return_value = None + with mock.patch("certbot_apache.parser.logger") as mock_logger: + self.parser.init_modules() + # Make sure that we got None return value and logged the file + self.assertTrue(mock_logger.debug.called) + @mock.patch("certbot_apache.parser.ApacheParser._get_runtime_cfg") def test_update_runtime_variables(self, mock_cfg): mock_cfg.return_value = (