module output is only json objects (#73765)

* module output is only json objects

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

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.

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:

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')

lib/ansible/plugins/action/__init__.py

@@ -1148,7 +1148,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)
 

test/integration/targets/json_cleanup/aliases

@@ -0,0 +1 @@
+shippable/posix/group2

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'

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

test/integration/targets/json_cleanup/runme.sh

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+set -eux
+
+ansible-playbook module_output_cleaning.yml "$@"

test/sanity/ignore.txt

@@ -167,6 +167,7 @@ test/integration/targets/collections_relative_imports/collection_root/ansible_co
 test/integration/targets/gathering_facts/library/bogus_facts shebang
 test/integration/targets/gathering_facts/library/facts_one shebang
 test/integration/targets/gathering_facts/library/facts_two shebang
+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