From 2d9d1762babbb583b10350ce98b428c864e7005f Mon Sep 17 00:00:00 2001
From: Colin Chan <colinchan@lumeh.org>
Date: Wed, 22 Feb 2017 22:41:38 -0800
Subject: [PATCH] Improve parsing of 'systemctl show' output

---
 lib/ansible/modules/system/systemd.py     | 55 ++++++++++++++---------
 test/units/modules/system/test_systemd.py | 51 +++++++++++++++++++++
 2 files changed, 86 insertions(+), 20 deletions(-)
 create mode 100644 test/units/modules/system/test_systemd.py

diff --git a/lib/ansible/modules/system/systemd.py b/lib/ansible/modules/system/systemd.py
index 8834b62803..a35d2bccb5 100644
--- a/lib/ansible/modules/system/systemd.py
+++ b/lib/ansible/modules/system/systemd.py
@@ -259,6 +259,40 @@ def is_running_service(service_status):
 def request_was_ignored(out):
     return '=' not in out and 'ignoring request' in out
 
+def parse_systemctl_show(lines):
+    # The output of 'systemctl show' can contain values that span multiple lines. At first glance it
+    # appears that such values are always surrounded by {}, so the previous version of this code
+    # assumed that any value starting with { was a multi-line value; it would then consume lines
+    # until it saw a line that ended with }. However, it is possible to have a single-line value
+    # that starts with { but does not end with } (this could happen in the value for Description=,
+    # for example), and the previous version of this code would then consume all remaining lines as
+    # part of that value. Cryptically, this would lead to Ansible reporting that the service file
+    # couldn't be found.
+    #
+    # To avoid this issue, the following code only accepts multi-line values for keys whose names
+    # start with Exec (e.g., ExecStart=), since these are the only keys whose values are known to
+    # span multiple lines.
+    parsed = {}
+    multival = []
+    k = None
+    for line in lines:
+        if k is None:
+            if '=' in line:
+                k, v = line.split('=', 1)
+                if k.startswith('Exec') and v.lstrip().startswith('{'):
+                    if not v.rstrip().endswith('}'):
+                        multival.append(v)
+                        continue
+                parsed[k] = v.strip()
+                k = None
+        else:
+            multival.append(line)
+            if line.rstrip().endswith('}'):
+                parsed[k] = '\n'.join(multival).strip()
+                multival = []
+                k = None
+    return parsed
+
 
 # ===========================================
 # Main control flow
@@ -320,27 +354,8 @@ def main():
 
         elif rc == 0:
             # load return of systemctl show into dictionary for easy access and return
-            multival = []
             if out:
-                k = None
-                for line in to_native(out).split('\n'): # systemd can have multiline values delimited with {}
-                    if line.strip():
-                        if k is None:
-                            if '=' in line:
-                                k,v = line.split('=', 1)
-                                if v.lstrip().startswith('{'):
-                                    if not v.rstrip().endswith('}'):
-                                        multival.append(line)
-                                        continue
-                                result['status'][k] = v.strip()
-                                k = None
-                        else:
-                            if line.rstrip().endswith('}'):
-                                result['status'][k] = '\n'.join(multival).strip()
-                                multival = []
-                                k = None
-                            else:
-                                multival.append(line)
+                result['status'] = parse_systemctl_show(to_native(out).split('\n'))
 
                 is_systemd = 'LoadState' in result['status'] and result['status']['LoadState'] != 'not-found'
 
diff --git a/test/units/modules/system/test_systemd.py b/test/units/modules/system/test_systemd.py
new file mode 100644
index 0000000000..255db9e68e
--- /dev/null
+++ b/test/units/modules/system/test_systemd.py
@@ -0,0 +1,51 @@
+import os
+import tempfile
+
+from ansible.compat.tests import unittest
+from ansible.modules.system.systemd import parse_systemctl_show
+
+
+class ParseSystemctlShowTestCase(unittest.TestCase):
+
+    def test_simple(self):
+        lines = [
+            'Type=simple',
+            'Restart=no',
+            'Requires=system.slice sysinit.target',
+            'Description=Blah blah blah',
+        ]
+        parsed = parse_systemctl_show(lines)
+        self.assertEqual(parsed, {
+            'Type': 'simple',
+            'Restart': 'no',
+            'Requires': 'system.slice sysinit.target',
+            'Description': 'Blah blah blah',
+        })
+
+    def test_multiline_exec(self):
+        # This was taken from a real service that specified "ExecStart=/bin/echo foo\nbar"
+        lines = [
+            'Type=simple',
+            'ExecStart={ path=/bin/echo ; argv[]=/bin/echo foo',
+            'bar ; ignore_errors=no ; start_time=[n/a] ; stop_time=[n/a] ; pid=0 ; code=(null) ; status=0/0 }',
+            'Description=blah',
+        ]
+        parsed = parse_systemctl_show(lines)
+        self.assertEqual(parsed, {
+            'Type': 'simple',
+            'ExecStart': '{ path=/bin/echo ; argv[]=/bin/echo foo\nbar ; ignore_errors=no ; start_time=[n/a] ; stop_time=[n/a] ; pid=0 ; code=(null) ; status=0/0 }',
+            'Description': 'blah',
+        })
+
+    def test_single_line_with_brace(self):
+        lines = [
+            'Type=simple',
+            'Description={ this is confusing',
+            'Restart=no',
+        ]
+        parsed = parse_systemctl_show(lines)
+        self.assertEqual(parsed, {
+            'Type': 'simple',
+            'Description': '{ this is confusing',
+            'Restart': 'no',
+        })