From f7078c1f8f7deb71676840a8b370ea240402c6a9 Mon Sep 17 00:00:00 2001
From: Rick Elrod <rick@elrod.me>
Date: Wed, 1 Jul 2020 09:28:48 -0500
Subject: [PATCH] Throw a prettier error in m_u.basic syslog (#70312)

Change:
- In certain situations, such as when the input string contains null
  bytes (\0), syslog.syslog will throw a TypeError. Handle that and
  fail_json instead.

Test Plan:
- New test
- ansible-test --docker centos[68] (for py2 and py3 respectively)

Tickets:
- Refs #70269

Signed-off-by: Rick Elrod <rick@elrod.me>
---
 .../fragments/syslog-prettier-error.yml       |  2 +
 lib/ansible/module_utils/basic.py             | 17 +++++++--
 .../module_utils/module_utils_test.yml        | 37 +++++++++++++++++++
 .../integration/targets/module_utils/runme.sh |  2 +-
 4 files changed, 53 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/fragments/syslog-prettier-error.yml

diff --git a/changelogs/fragments/syslog-prettier-error.yml b/changelogs/fragments/syslog-prettier-error.yml
new file mode 100644
index 00000000000..821cde88ef6
--- /dev/null
+++ b/changelogs/fragments/syslog-prettier-error.yml
@@ -0,0 +1,2 @@
+minor_changes:
+  - The logging functionality in module_utils.basic now returns a nicer error when it falls back to syslog but ends up getting a TypeError thrown back.
diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py
index db90dafd13b..872ee4300fd 100644
--- a/lib/ansible/module_utils/basic.py
+++ b/lib/ansible/module_utils/basic.py
@@ -1879,10 +1879,19 @@ class AnsibleModule(object):
 
     def _log_to_syslog(self, msg):
         if HAS_SYSLOG:
-            module = 'ansible-%s' % self._name
-            facility = getattr(syslog, self._syslog_facility, syslog.LOG_USER)
-            syslog.openlog(str(module), 0, facility)
-            syslog.syslog(syslog.LOG_INFO, msg)
+            try:
+                module = 'ansible-%s' % self._name
+                facility = getattr(syslog, self._syslog_facility, syslog.LOG_USER)
+                syslog.openlog(str(module), 0, facility)
+                syslog.syslog(syslog.LOG_INFO, msg)
+            except TypeError as e:
+                self.fail_json(
+                    msg='Failed to log to syslog (%s). To proceed anyway, '
+                        'disable syslog logging by setting no_target_syslog '
+                        'to True in your Ansible config.' % to_native(e),
+                    exception=traceback.format_exc(),
+                    msg_to_log=msg,
+                )
 
     def debug(self, msg):
         if self._debug:
diff --git a/test/integration/targets/module_utils/module_utils_test.yml b/test/integration/targets/module_utils/module_utils_test.yml
index e29039f9018..81302c5ec8b 100644
--- a/test/integration/targets/module_utils/module_utils_test.yml
+++ b/test/integration/targets/module_utils/module_utils_test.yml
@@ -60,3 +60,40 @@
       that:
         - result.deprecations[0].msg == "Alias 'baz' is deprecated. See the module docs for more information"
         - result.deprecations[0].version == '9.99'
+
+  - block:
+      - name: Get a string with a \0 in it
+        command: echo -e 'hi\0foo'
+        register: string_with_null
+
+      - name: Use the null string as a module parameter
+        lineinfile:
+          path: "{{ output_dir }}/nulltest"
+          line: "{{ string_with_null.stdout }}"
+          create: yes
+        ignore_errors: yes
+        register: nulltest
+
+      - name: See if the file exists
+        stat:
+          path: "{{ output_dir }}/nulltest"
+        register: nullstat
+
+      - assert:
+          that:
+            - nulltest is failed
+            - nulltest.msg_to_log.startswith('Invoked ')
+            - nulltest.msg.startswith('Failed to log to syslog')
+        # Conditionalize this, because when we log with something other than
+        # syslog, it's probably successful and these assertions will fail.
+        when: nulltest is failed
+
+      # Ensure we fail out early and don't actually run the module if logging
+      # failed.
+      - assert:
+          that:
+            - nullstat.stat.exists == nulltest is successful
+    always:
+      - file:
+          path: "{{ output_dir }}/nulltest"
+          state: absent
diff --git a/test/integration/targets/module_utils/runme.sh b/test/integration/targets/module_utils/runme.sh
index e1a0e7c9535..8ae2e9cca2d 100755
--- a/test/integration/targets/module_utils/runme.sh
+++ b/test/integration/targets/module_utils/runme.sh
@@ -8,6 +8,6 @@ ANSIBLE_ROLES_PATH=../ ansible-playbook module_utils_basic_setcwd.yml -i ../../i
 # doesn't traceback with unicode in the custom module_utils directory path.
 ansible-playbook module_utils_vvvvv.yml -i ../../inventory -vvvvv "$@"
 
-ansible-playbook module_utils_test.yml -i ../../inventory -v "$@"
+ansible-playbook module_utils_test.yml -i ../../inventory -e output_dir="$OUTPUT_DIR" -v "$@"
 
 ANSIBLE_MODULE_UTILS=other_mu_dir ansible-playbook module_utils_envvar.yml -i ../../inventory -v "$@"