Merge pull request #806 from letsencrypt/apache_include_quotes

Apache include quotes

letsencrypt-apache/letsencrypt_apache/parser.py

@@ -241,6 +241,10 @@ class ApacheParser(object):
         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
@@ -315,6 +319,14 @@ class ApacheParser(object):
 
         """
         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:
@@ -390,6 +402,9 @@ class ApacheParser(object):
         #     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 ../

letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py

@@ -32,6 +32,7 @@ class ComplexParserTest(util.ParserTest):
                 "COMPLEX": "",
                 "tls_port": "1234",
                 "fnmatch_filename": "test_fnmatch.conf",
+                "tls_port_str": "1234"
             }
         )
 
@@ -49,6 +50,12 @@ class ComplexParserTest(util.ParserTest):
         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"]
 
@@ -89,6 +96,7 @@ class ComplexParserTest(util.ParserTest):
         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 +109,12 @@ class ComplexParserTest(util.ParserTest):
     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}")
 

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
 
 

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