From fa9822df0f7a9067b6feae9d8c187f860aae1a29 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 28 Jan 2016 10:50:20 -0800 Subject: [PATCH] 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 --- lib/ansible/cli/__init__.py | 2 +- lib/ansible/cli/galaxy.py | 35 ++++++++++++++++++-------------- lib/ansible/cli/vault.py | 8 +++++++- lib/ansible/utils/module_docs.py | 6 +++--- 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 08488154e2..17f06409bb 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=C.COLOR_ERROR) 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 14a5d03056..e647da0014 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')