From 6e4faac9c07297beeb56c83a5239eea3fed767a2 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 08:15:11 -0700 Subject: [PATCH 1/5] Allow single/double quotes around Include dirs --- letsencrypt-apache/letsencrypt_apache/parser.py | 3 +++ .../letsencrypt_apache/tests/complex_parsing_test.py | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 823d9794bec..e70d22d4ebf 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -390,6 +390,9 @@ def _get_include_path(self, arg): # logger.error("Error: Invalid regexp characters in %s", arg) # return [] + # Remove beginning and ending quotes + arg = arg.strip("'\"") + # Standardize the include argument based on server root if not arg.startswith("/"): # Normpath will condense ../ diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index 64ecaa3219f..a373b97664c 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -78,7 +78,7 @@ def test_load_modules(self): # This is in an IfDefine self.assertTrue("ssl_module" in self.parser.modules) self.assertTrue("mod_ssl.c" in self.parser.modules) - + # def verify_fnmatch(self, arg, hit=True): """Test if Include was correctly parsed.""" from letsencrypt_apache import parser @@ -89,6 +89,7 @@ def verify_fnmatch(self, arg, hit=True): else: self.assertFalse(self.parser.find_dir("FNMATCH_DIRECTIVE")) + # NOTE: Only run one test per function otherwise you will have inf recursion def test_include(self): self.verify_fnmatch("test_fnmatch.?onf") @@ -101,6 +102,12 @@ def test_include_fullpath(self): def test_include_fullpath_trailing_slash(self): self.verify_fnmatch(self.config_path + "//") + def test_include_single_quotes(self): + self.verify_fnmatch("'" + self.config_path + "'") + + def test_include_double_quotes(self): + self.verify_fnmatch('"' + self.config_path + '"') + def test_include_variable(self): self.verify_fnmatch("../complex_parsing/${fnmatch_filename}") From 19d65c3e2f919ddc14cfc1bd34dc9b29a5c725fd Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 08:56:26 -0700 Subject: [PATCH 2/5] Add variable quote parsing --- letsencrypt-apache/letsencrypt_apache/parser.py | 8 ++++++++ .../letsencrypt_apache/tests/complex_parsing_test.py | 7 +++++++ .../tests/testdata/complex_parsing/apache2.conf | 2 ++ .../tests/testdata/complex_parsing/test_variables.conf | 1 + 4 files changed, 18 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index e70d22d4ebf..0ba438e6543 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -315,6 +315,14 @@ def get_arg(self, match): """ value = self.aug.get(match) + + # No need to strip quotes for variables, as apache2ctl already does this + # but we do need to strip quotes for all normal arguments. + + # Note: normal argument may be a quoted variable + # e.g. strip now, not later + value = value.strip("'\"") + variables = ApacheParser.arg_var_interpreter.findall(value) for var in variables: diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index a373b97664c..37c0208c13b 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -32,6 +32,7 @@ def setup_variables(self): "COMPLEX": "", "tls_port": "1234", "fnmatch_filename": "test_fnmatch.conf", + "tls_port_str": "1234" } ) @@ -49,6 +50,12 @@ def test_basic_variable_parsing(self): self.assertEqual(len(matches), 1) self.assertEqual(self.parser.get_arg(matches[0]), "1234") + def test_basic_variable_parsing_quotes(self): + matches = self.parser.find_dir("TestVariablePortStr") + + self.assertEqual(len(matches), 1) + self.assertEqual(self.parser.get_arg(matches[0]), "1234") + def test_invalid_variable_parsing(self): del self.parser.variables["tls_port"] diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/apache2.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/apache2.conf index 26bf47263ab..14cf95f9ec3 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/apache2.conf +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/apache2.conf @@ -46,6 +46,8 @@ IncludeOptional sites-enabled/*.conf Define COMPLEX Define tls_port 1234 +Define tls_port_str "1234" + Define fnmatch_filename test_fnmatch.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/test_variables.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/test_variables.conf index a3819183702..1a9edff74ad 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/test_variables.conf +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/test_variables.conf @@ -1,4 +1,5 @@ TestVariablePort ${tls_port} +TestVariablePortStr "${tls_port_str}" LoadModule status_module modules/mod_status.so From 202b21f260cecbb9354e03831497ac4c38134bed Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 08:58:02 -0700 Subject: [PATCH 3/5] Remove extra # --- .../letsencrypt_apache/tests/complex_parsing_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index 37c0208c13b..bb0ff6af91a 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -85,7 +85,7 @@ def test_load_modules(self): # This is in an IfDefine self.assertTrue("ssl_module" in self.parser.modules) self.assertTrue("mod_ssl.c" in self.parser.modules) - # + def verify_fnmatch(self, arg, hit=True): """Test if Include was correctly parsed.""" from letsencrypt_apache import parser From e922a82277b52203ebfc74787e28bbd605d7d9e7 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 09:06:53 -0700 Subject: [PATCH 4/5] letsencrypt-apache/ --- letsencrypt-apache/letsencrypt_apache/parser.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 0ba438e6543..0a3643064ce 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -241,6 +241,10 @@ def find_dir(self, directive, arg=None, start=None, exclude=True): Directives should be in the form of a case insensitive regex currently .. todo:: arg should probably be a list + .. todo:: arg search currently only supports direct matching. It does + not handle the case of variables or quoted arguments. This should + be adapted to use a generic search for the directive and then do a + case-insensitive self.get_arg filter Note: Augeas is inherently case sensitive while Apache is case insensitive. Augeas 1.0 allows case insensitive regexes like From d4d71a73a55990d127dfb7d531f634b8b4219116 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 09:16:49 -0700 Subject: [PATCH 5/5] Remove trailing whitespace --- .../letsencrypt_apache/tests/complex_parsing_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index bb0ff6af91a..7099c388fce 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -85,7 +85,7 @@ def test_load_modules(self): # This is in an IfDefine self.assertTrue("ssl_module" in self.parser.modules) self.assertTrue("mod_ssl.c" in self.parser.modules) - + def verify_fnmatch(self, arg, hit=True): """Test if Include was correctly parsed.""" from letsencrypt_apache import parser