Changes to convert to unicode at the borders

The module docs and vault changes solve issues where tracebacks can
happen.  The galaxy changes are mostly refactoring to be more pythonic
with a small chance that a unicode traceback could have occurred there
without the changes.  The change in __init__.py when we actually call
the pager makes things more robust but could hide places where we had
bytes coming in already so I didn't want to change that without auditing
where the text was coming from.

Fixes #14178

4 files changed, 31 insertions, 20 deletions

diff --git a/lib/ansible/cli/__init__.py b/lib/ansible/cli/__init__.py
index 12ba8f8900..a91c9557b0 100644
--- a/lib/ansible/cli/__init__.py
+++ b/lib/ansible/cli/__init__.py
@@ -459,7 +459,7 @@ class CLI(object):
             os.environ['LESS'] = CLI.LESS_OPTS
         try:
             cmd = subprocess.Popen(cmd, shell=True, stdin=subprocess.PIPE, stdout=sys.stdout)
-            cmd.communicate(input=text.encode(sys.stdout.encoding))
+            cmd.communicate(input=to_bytes(text))
         except IOError:
             pass
         except KeyboardInterrupt:
diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py
index 672d891915..81c89b540c 100644
--- a/lib/ansible/cli/galaxy.py
+++ b/lib/ansible/cli/galaxy.py
@@ -39,6 +39,7 @@ from ansible.galaxy.role import GalaxyRole
 from ansible.galaxy.login import GalaxyLogin
 from ansible.galaxy.token import GalaxyToken
 from ansible.playbook.role.requirement import RoleRequirement
+from ansible.utils.unicode import to_unicode
 
 try:
     from __main__ import display
@@ -161,8 +162,8 @@ class GalaxyCLI(CLI):
 
     def _display_role_info(self, role_info):
 
-        text = "\nRole: %s \n" % role_info['name']
-        text += "\tdescription: %s \n" % role_info.get('description', '')
+        text = [u"", u"Role: %s" % to_unicode(role_info['name'])]
+        text.append(u"\tdescription: %s" % role_info.get('description', ''))
 
         for k in sorted(role_info.keys()):
 
@@ -171,14 +172,15 @@ class GalaxyCLI(CLI):
 
             if isinstance(role_info[k], dict):
                 text += "\t%s: \n" % (k)
+                text.append(u"\t%s:" % (k))
                 for key in sorted(role_info[k].keys()):
                     if key in self.SKIP_INFO_KEYS:
                         continue
-                    text += "\t\t%s: %s\n" % (key, role_info[k][key])
+                    text.append(u"\t\t%s: %s" % (key, role_info[k][key]))
             else:
-                text += "\t%s: %s\n" % (k, role_info[k])
+                text.append(u"\t%s: %s" % (k, role_info[k]))
 
-        return text
+        return u'\n'.join(text)
 
 ############################
 # execute actions
@@ -322,9 +324,11 @@ class GalaxyCLI(CLI):
             if role_spec:
                 role_info.update(role_spec)
 
-            data += self._display_role_info(role_info)
+            data = self._display_role_info(role_info)
+            ### FIXME: This is broken in both 1.9 and 2.0 as
+            # _display_role_info() always returns something
             if not data:
-                data += "\n- the role %s was not found" % role
+                data = u"\n- the role %s was not found" % role
 
         self.pager(data)
 
@@ -518,24 +522,25 @@ class GalaxyCLI(CLI):
             display.display("No roles match your search.", color="yellow")
             return True
 
-        data = ''
+        data = [u'']
 
         if response['count'] > page_size:
-            data += ("\nFound %d roles matching your search. Showing first %s.\n" % (response['count'], page_size))
+            data.append(u"Found %d roles matching your search. Showing first %s." % (response['count'], page_size))
         else:
-            data += ("\nFound %d roles matching your search:\n" % response['count'])
+            data.append(u"Found %d roles matching your search:" % response['count'])
 
         max_len = []
         for role in response['results']:
             max_len.append(len(role['username'] + '.' + role['name']))
         name_len = max(max_len)
-        format_str = " %%-%ds %%s\n" % name_len
-        data +='\n'
-        data += (format_str % ("Name", "Description"))
-        data += (format_str % ("----", "-----------"))
+        format_str = u" %%-%ds %%s" % name_len
+        data.append(u'')
+        data.append(format_str % (u"Name", u"Description"))
+        data.append(format_str % (u"----", u"-----------"))
         for role in response['results']:
-            data += (format_str % (role['username'] + '.' + role['name'],role['description']))
+            data.append(format_str % (u'%s.%s' % (role['username'], role['name']), role['description']))
 
+        data = u'\n'.join(data)
         self.pager(data)
 
         return True
diff --git a/lib/ansible/cli/vault.py b/lib/ansible/cli/vault.py
index 9908f17e57..85bdac6993 100644
--- a/lib/ansible/cli/vault.py
+++ b/lib/ansible/cli/vault.py
@@ -26,6 +26,7 @@ from ansible.errors import AnsibleError, AnsibleOptionsError
 from ansible.parsing.dataloader import DataLoader
 from ansible.parsing.vault import VaultEditor
 from ansible.cli import CLI
+from ansible.utils.unicode import to_unicode
 
 try:
     from __main__ import display
@@ -157,7 +158,12 @@ class VaultCLI(CLI):
     def execute_view(self):
 
         for f in self.args:
-            self.pager(self.editor.plaintext(f))
+            # Note: vault should return byte strings because it could encrypt
+            # and decrypt binary files.  We are responsible for changing it to
+            # unicode here because we are displaying it and therefore can make
+            # the decision that the display doesn't have to be precisely what
+            # the input was (leave that to decrypt instead)
+            self.pager(to_unicode(self.editor.plaintext(f)))
 
     def execute_rekey(self):
         for f in self.args:
diff --git a/lib/ansible/utils/module_docs.py b/lib/ansible/utils/module_docs.py
index 4a90c3caca..fe74c18574 100755
--- a/lib/ansible/utils/module_docs.py
+++ b/lib/ansible/utils/module_docs.py
@@ -24,7 +24,7 @@ __metaclass__ = type
 import os
 import sys
 import ast
-import yaml
+from ansible.parsing.yaml.loader import AnsibleLoader
 import traceback
 
 from collections import MutableMapping, MutableSet, MutableSequence
@@ -71,7 +71,7 @@ def get_docstring(filename, verbose=False):
                         continue
 
                     if 'DOCUMENTATION' in theid:
-                        doc = yaml.safe_load(child.value.s)
+                        doc = AnsibleLoader(child.value.s, file_name=filename).get_single_data()
                         fragments = doc.get('extends_documentation_fragment', [])
 
                         if isinstance(fragments, basestring):
@@ -91,7 +91,7 @@ def get_docstring(filename, verbose=False):
                             assert fragment_class is not None
 
                             fragment_yaml = getattr(fragment_class, fragment_var, '{}')
-                            fragment = yaml.safe_load(fragment_yaml)
+                            fragment = AnsibleLoader(fragment_yaml, file_name=filename).get_single_data()
 
                             if fragment.has_key('notes'):
                                 notes = fragment.pop('notes')