From 5e87aee968082c1eb3f4c8ae2fd1903718e1feb5 Mon Sep 17 00:00:00 2001
From: Yaroslav Halchenko <debian@onerussian.com>
Date: Thu, 12 Aug 2021 17:57:24 -0400
Subject: [PATCH] BF: apache cfg parsing - relax assumption that value cannot
 contain = (#8930)

* BF: apache cfg parsing - relax assumption that value cannot contain =

* Remove failing test_update_runtime_vars_bad_output

* Add test Define statements: with = in value, and an empty value

* update CHANGELOG

Co-authored-by: Alex Zorin <alex@zorin.id.au>
---
 .../certbot_apache/_internal/apache_util.py      | 11 ++++-------
 certbot-apache/tests/parser_test.py              | 16 ++++++----------
 certbot/CHANGELOG.md                             |  2 +-
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/certbot-apache/certbot_apache/_internal/apache_util.py b/certbot-apache/certbot_apache/_internal/apache_util.py
index 424d10fcf..3d06814b6 100644
--- a/certbot-apache/certbot_apache/_internal/apache_util.py
+++ b/certbot-apache/certbot_apache/_internal/apache_util.py
@@ -153,13 +153,10 @@ def parse_defines(apachectl):
         return {}
 
     for match in matches:
-        if match.count("=") > 1:
-            logger.error("Unexpected number of equal signs in "
-                         "runtime config dump.")
-            raise errors.PluginError(
-                "Error parsing Apache runtime variables")
-        parts = match.partition("=")
-        variables[parts[0]] = parts[2]
+        # Value could also contain = so split only once
+        parts = match.split('=', 1)
+        value = parts[1] if len(parts) == 2 else ''
+        variables[parts[0]] = value
 
     return variables
 
diff --git a/certbot-apache/tests/parser_test.py b/certbot-apache/tests/parser_test.py
index 5ee64d3fd..61855fdb7 100644
--- a/certbot-apache/tests/parser_test.py
+++ b/certbot-apache/tests/parser_test.py
@@ -188,6 +188,8 @@ class BasicParserTest(util.ParserTest):
             'Define: DUMP_RUN_CFG\n'
             'Define: U_MICH\n'
             'Define: TLS=443\n'
+            'Define: WITH_ASSIGNMENT=URL=http://example.com\n'
+            'Define: EMPTY=\n'
             'Define: example_path=Documents/path\n'
             'User: name="www-data" id=33 not_used\n'
             'Group: name="www-data" id=33 not_used\n'
@@ -266,7 +268,10 @@ class BasicParserTest(util.ParserTest):
         mock_cfg.side_effect = mock_get_vars
 
         expected_vars = {"TEST": "", "U_MICH": "", "TLS": "443",
-                         "example_path": "Documents/path"}
+                         "example_path": "Documents/path",
+                         "WITH_ASSIGNMENT": "URL=http://example.com",
+                         "EMPTY": "",
+                         }
 
         self.parser.modules = {}
         with mock.patch(
@@ -301,15 +306,6 @@ class BasicParserTest(util.ParserTest):
             # path derived from root configuration Include statements
             self.assertEqual(mock_parse.call_count, 1)
 
-    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
-    def test_update_runtime_vars_bad_output(self, mock_cfg):
-        mock_cfg.return_value = "Define: TLS=443=24"
-        self.parser.update_runtime_variables()
-
-        mock_cfg.return_value = "Define: DUMP_RUN_CFG\nDefine: TLS=443=24"
-        self.assertRaises(
-            errors.PluginError, self.parser.update_runtime_variables)
-
     @mock.patch("certbot_apache._internal.apache_util.subprocess.run")
     def test_update_runtime_vars_bad_ctl(self, mock_run):
         mock_run.side_effect = OSError
diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index 198a4a153..bd43fe10d 100644
--- a/certbot/CHANGELOG.md
+++ b/certbot/CHANGELOG.md
@@ -14,7 +14,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
-*
+* Fixed parsing of `Define`d values in the Apache plugin to allow for `=` in the value.
 
 More details about these changes can be found on our GitHub repo.