module output is only json objects (#73765) (#73777)

 remove json lists as they are not valid from modules
 fixes #73744

(cherry picked from commit 43300e22798e4c9bd8ec2e321d28c5e8d2018aeb)

9 files changed, 51 insertions, 9 deletions

diff --git a/changelogs/fragments/fix_json_module_parsing.yml b/changelogs/fragments/fix_json_module_parsing.yml
new file mode 100644
index 0000000000..051aab5912
--- /dev/null
+++ b/changelogs/fragments/fix_json_module_parsing.yml
@@ -0,0 +1,2 @@
+bugfixes:
+    - restrict module valid JSON parsed output to objects as lists are not valid responses.
diff --git a/lib/ansible/module_utils/json_utils.py b/lib/ansible/module_utils/json_utils.py
index d5639fa3f8..0e95aa677c 100644
--- a/lib/ansible/module_utils/json_utils.py
+++ b/lib/ansible/module_utils/json_utils.py
@@ -32,7 +32,7 @@ import json
 
 # NB: a copy of this function exists in ../../modules/core/async_wrapper.py. Ensure any
 # changes are propagated there.
-def _filter_non_json_lines(data):
+def _filter_non_json_lines(data, objects_only=False):
     '''
     Used to filter unrelated output around module JSON output, like messages from
     tcagetattr, or where dropbear spews MOTD on every single command (which is nuts).
@@ -50,7 +50,7 @@ def _filter_non_json_lines(data):
         if line.startswith(u'{'):
             endchar = u'}'
             break
-        elif line.startswith(u'['):
+        elif not objects_only and line.startswith(u'['):
             endchar = u']'
             break
     else:
diff --git a/lib/ansible/modules/async_wrapper.py b/lib/ansible/modules/async_wrapper.py
index 5379726ae6..7ba8271ef6 100644
--- a/lib/ansible/modules/async_wrapper.py
+++ b/lib/ansible/modules/async_wrapper.py
@@ -74,7 +74,7 @@ def _filter_non_json_lines(data):
     Used to filter unrelated output around module JSON output, like messages from
     tcagetattr, or where dropbear spews MOTD on every single command (which is nuts).
 
-    Filters leading lines before first line-starting occurrence of '{' or '[', and filter all
+    Filters leading lines before first line-starting occurrence of '{', and filter all
     trailing lines after matching close character (working from the bottom of output).
     '''
     warnings = []
@@ -85,10 +85,6 @@ def _filter_non_json_lines(data):
     for start, line in enumerate(lines):
         line = line.strip()
         if line.startswith(u'{'):
-            endchar = u'}'
-            break
-        elif line.startswith(u'['):
-            endchar = u']'
             break
     else:
         raise ValueError('No start of json char found')
@@ -97,7 +93,7 @@ def _filter_non_json_lines(data):
     lines = lines[start:]
 
     for reverse_end_offset, line in enumerate(reversed(lines)):
-        if line.strip().endswith(endchar):
+        if line.strip().endswith(u'}'):
             break
     else:
         raise ValueError('No end of json char found')
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 4e5e82adb7..84e4b9a9d3 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -1029,7 +1029,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
 
     def _parse_returned_data(self, res):
         try:
-            filtered_output, warnings = _filter_non_json_lines(res.get('stdout', u''))
+            filtered_output, warnings = _filter_non_json_lines(res.get('stdout', u''), objects_only=True)
             for w in warnings:
                 display.warning(w)
 
diff --git a/test/integration/targets/json_cleanup/aliases b/test/integration/targets/json_cleanup/aliases
new file mode 100644
index 0000000000..765b70da79
--- /dev/null
+++ b/test/integration/targets/json_cleanup/aliases
@@ -0,0 +1 @@
+shippable/posix/group2
diff --git a/test/integration/targets/json_cleanup/library/bad_json b/test/integration/targets/json_cleanup/library/bad_json
new file mode 100644
index 0000000000..1df8c725f0
--- /dev/null
+++ b/test/integration/targets/json_cleanup/library/bad_json
@@ -0,0 +1,11 @@
+#!/usr/bin/env bash
+
+set -eu
+
+echo 'this stuff should be ignored'
+
+echo '[ looks like a json list]'
+
+echo '{"changed": false, "failed": false, "msg": "good json response"}'
+
+echo 'moar garbage'
diff --git a/test/integration/targets/json_cleanup/module_output_cleaning.yml b/test/integration/targets/json_cleanup/module_output_cleaning.yml
new file mode 100644
index 0000000000..165352aab8
--- /dev/null
+++ b/test/integration/targets/json_cleanup/module_output_cleaning.yml
@@ -0,0 +1,26 @@
+- name: ensure we clean module output well
+  hosts: localhost
+  gather_facts: false
+  tasks:
+    - name: call module that spews extra stuff
+      bad_json:
+      register: clean_json
+      ignore_errors: true
+
+    - name: all expected is there
+      assert:
+        that:
+          - clean_json is success
+          - clean_json is not changed
+          - "clean_json['msg'] == 'good json response'"
+
+    - name: all non wanted is not there
+      assert:
+        that:
+          - item not in clean_json.values()
+      loop:
+         - this stuff should be ignored
+         - [ looks like a json list]
+         - '[ looks like a json list]'
+         - ' looks like a json list'
+         - moar garbage
diff --git a/test/integration/targets/json_cleanup/runme.sh b/test/integration/targets/json_cleanup/runme.sh
new file mode 100755
index 0000000000..2de3bd0ecb
--- /dev/null
+++ b/test/integration/targets/json_cleanup/runme.sh
@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+set -eux
+
+ansible-playbook module_output_cleaning.yml "$@"
diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt
index f0c2d1ed98..5275f076ed 100644
--- a/test/sanity/ignore.txt
+++ b/test/sanity/ignore.txt
@@ -236,6 +236,7 @@ test/integration/targets/ignore_unreachable/fake_connectors/bad_exec.py future-i
 test/integration/targets/ignore_unreachable/fake_connectors/bad_exec.py metaclass-boilerplate
 test/integration/targets/ignore_unreachable/fake_connectors/bad_put_file.py future-import-boilerplate
 test/integration/targets/ignore_unreachable/fake_connectors/bad_put_file.py metaclass-boilerplate
+test/integration/targets/json_cleanup/library/bad_json shebang
 test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.0/DSCResources/ANSIBLE_xSetReboot/ANSIBLE_xSetReboot.psm1 pslint!skip
 test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.0/DSCResources/ANSIBLE_xTestResource/ANSIBLE_xTestResource.psm1 pslint!skip
 test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.0/xTestDsc.psd1 pslint!skip