From 51bd07204b7063e8aacf01750d09091f5269f584 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Thu, 27 Jul 2017 18:15:56 -0700 Subject: [PATCH] Revert "Revert "Allow ini plugin to load file using other encoding than utf8." (#27407)" This reverts commit 520696fb3949f708f55bce74ae2f2ab302fc9c21. --- docs/docsite/rst/playbooks_lookups.rst | 1 + lib/ansible/parsing/dataloader.py | 4 +- lib/ansible/plugins/lookup/ini.py | 36 ++++++++--------- .../lookup_properties/lookup-8859-15.ini | 7 ++++ .../targets/lookup_properties/lookup.ini | 1 + .../lookup_properties/lookup.properties | 1 + .../test_lookup_properties.yml | 39 +++++++++++++++++-- test/runner/lib/git.py | 12 +++--- test/runner/lib/util.py | 12 +++--- 9 files changed, 78 insertions(+), 35 deletions(-) create mode 100644 test/integration/targets/lookup_properties/lookup-8859-15.ini diff --git a/docs/docsite/rst/playbooks_lookups.rst b/docs/docsite/rst/playbooks_lookups.rst index 4152a795e1..001faed211 100644 --- a/docs/docsite/rst/playbooks_lookups.rst +++ b/docs/docsite/rst/playbooks_lookups.rst @@ -253,6 +253,7 @@ type ini Type of the file. Can be ini or properties (for java file ansible.ini Name of the file to load section global Default section where to lookup for key. re False The key is a regexp. +encoding utf-8 Text encoding to use. default empty string return value if the key is not in the ini file ========== ============ ========================================================================================= diff --git a/lib/ansible/parsing/dataloader.py b/lib/ansible/parsing/dataloader.py index 2ee53ea484..568cb801a8 100644 --- a/lib/ansible/parsing/dataloader.py +++ b/lib/ansible/parsing/dataloader.py @@ -179,7 +179,7 @@ class DataLoader: except AttributeError: pass # older versions of yaml don't have dispose function, ignore - def _get_file_contents(self, file_name): + def _get_file_contents(self, file_name, encoding='utf-8'): ''' Reads the file contents from the given file name, and will decrypt them if they are found to be vault-encrypted. @@ -194,7 +194,7 @@ class DataLoader: show_content = True try: with open(b_file_name, 'rb') as f: - data = f.read() + data = to_text(f.read(), encoding=encoding) if is_encrypted(data): data = self._vault.decrypt(data, filename=b_file_name) show_content = False diff --git a/lib/ansible/plugins/lookup/ini.py b/lib/ansible/plugins/lookup/ini.py index 14660bc5f5..0eb6459d2a 100644 --- a/lib/ansible/plugins/lookup/ini.py +++ b/lib/ansible/plugins/lookup/ini.py @@ -31,7 +31,7 @@ from ansible.plugins.lookup import LookupBase def _parse_params(term): '''Safely split parameter term to preserve spaces''' - keys = ['key', 'type', 'section', 'file', 're', 'default'] + keys = ['key', 'type', 'section', 'file', 're', 'default', 'encoding'] params = {} for k in keys: params[k] = '' @@ -52,19 +52,6 @@ def _parse_params(term): class LookupModule(LookupBase): - def read_properties(self, filename, key, dflt, is_regexp): - config = StringIO() - current_cfg_file = open(to_bytes(filename, errors='surrogate_or_strict'), 'rb') - - config.write(u'[java_properties]\n' + to_text(current_cfg_file.read(), errors='surrogate_or_strict')) - config.seek(0, os.SEEK_SET) - self.cp.readfp(config) - return self.get_value(key, 'java_properties', dflt, is_regexp) - - def read_ini(self, filename, key, section, dflt, is_regexp): - self.cp.readfp(open(to_bytes(filename, errors='surrogate_or_strict'))) - return self.get_value(key, section, dflt, is_regexp) - def get_value(self, key, section, dflt, is_regexp): # Retrieve all values from a section using a regexp if is_regexp: @@ -79,8 +66,6 @@ class LookupModule(LookupBase): def run(self, terms, variables=None, **kwargs): - basedir = self.get_basedir(variables) - self.basedir = basedir self.cp = configparser.ConfigParser() ret = [] @@ -94,6 +79,7 @@ class LookupModule(LookupBase): 'default': None, 'section': "global", 'type': "ini", + 'encoding': 'utf-8', } # parameters specified? @@ -105,11 +91,23 @@ class LookupModule(LookupBase): except (ValueError, AssertionError) as e: raise AnsibleError(e) + # Retrieve file path path = self.find_file_in_search_path(variables, 'files', paramvals['file']) + + # Create StringIO later used to parse ini + config = StringIO() + # Special case for java properties if paramvals['type'] == "properties": - var = self.read_properties(path, key, paramvals['default'], paramvals['re']) - else: - var = self.read_ini(path, key, paramvals['section'], paramvals['default'], paramvals['re']) + config.write(u'[java_properties]\n') + paramvals['section'] = 'java_properties' + + # Open file using encoding + contents, show_data = self._loader._get_file_contents(path, encoding=paramvals['encoding']) + config.write(contents) + config.seek(0, os.SEEK_SET) + + self.cp.readfp(config) + var = self.get_value(key, paramvals['section'], paramvals['default'], paramvals['re']) if var is not None: if isinstance(var, MutableSequence): for v in var: diff --git a/test/integration/targets/lookup_properties/lookup-8859-15.ini b/test/integration/targets/lookup_properties/lookup-8859-15.ini new file mode 100644 index 0000000000..33f9c29d18 --- /dev/null +++ b/test/integration/targets/lookup_properties/lookup-8859-15.ini @@ -0,0 +1,7 @@ +[global] +# A comment +value1=Text associated with value1 and global section +value2=Same for value2 and global section +value.dot=Properties with dot +field.with.space = another space +field_with_unicode=été indien où à château français ïîôû diff --git a/test/integration/targets/lookup_properties/lookup.ini b/test/integration/targets/lookup_properties/lookup.ini index 16500fd899..5b7cc34b55 100644 --- a/test/integration/targets/lookup_properties/lookup.ini +++ b/test/integration/targets/lookup_properties/lookup.ini @@ -4,6 +4,7 @@ value1=Text associated with value1 and global section value2=Same for value2 and global section value.dot=Properties with dot field.with.space = another space +unicode=été indien où à château français ïîôû [section1] value1=section1/value1 diff --git a/test/integration/targets/lookup_properties/lookup.properties b/test/integration/targets/lookup_properties/lookup.properties index f388d8cfbf..d71ce1217b 100644 --- a/test/integration/targets/lookup_properties/lookup.properties +++ b/test/integration/targets/lookup_properties/lookup.properties @@ -3,3 +3,4 @@ value1=Text associated with value1 value2=Same for value2 value.dot=Properties with dot field.with.space = another space +field.with.unicode = été indien où à château français ïîôû diff --git a/test/integration/targets/lookup_properties/test_lookup_properties.yml b/test/integration/targets/lookup_properties/test_lookup_properties.yml index 4d22ce642c..a8cad9de48 100644 --- a/test/integration/targets/lookup_properties/test_lookup_properties.yml +++ b/test/integration/targets/lookup_properties/test_lookup_properties.yml @@ -9,32 +9,63 @@ test2: "{{lookup('ini', 'value2 type=properties file=lookup.properties')}}" test_dot: "{{lookup('ini', 'value.dot type=properties file=lookup.properties')}}" field_with_space: "{{lookup('ini', 'field.with.space type=properties file=lookup.properties')}}" - - debug: var={{item}} + - assert: + that: "{{item}} is defined" with_items: [ 'test1', 'test2', 'test_dot', 'field_with_space' ] - name: "read ini value" set_fact: value1_global: "{{lookup('ini', 'value1 section=global file=lookup.ini')}}" value2_global: "{{lookup('ini', 'value2 section=global file=lookup.ini')}}" value1_section1: "{{lookup('ini', 'value1 section=section1 file=lookup.ini')}}" + field_with_unicode: "{{lookup('ini', 'unicode section=global file=lookup.ini')}}" - debug: var={{item}} - with_items: [ 'value1_global', 'value2_global', 'value1_section1' ] + with_items: [ 'value1_global', 'value2_global', 'value1_section1', 'field_with_unicode' ] + - assert: + that: + - "field_with_unicode == 'été indien où à château français ïîôû'" + - name: "read ini value from iso8859-15 file" + set_fact: + field_with_unicode: "{{lookup('ini', 'field_with_unicode section=global encoding=iso8859-1 file=lookup-8859-15.ini')}}" + - assert: + that: + - "field_with_unicode == 'été indien où à château français ïîôû'" - name: "read ini value with section and regexp" set_fact: value_section: "{{lookup('ini', 'value[1-2] section=value_section file=lookup.ini re=true')}}" other_section: "{{lookup('ini', 'other[1-2] section=other_section file=lookup.ini re=true')}}" - debug: var={{item}} with_items: [ 'value_section', 'other_section' ] + - assert: + that: + - "value_section == '1,2'" + - "other_section == '4,5'" - name: "Reading unknown value" set_fact: - unknown: "{{lookup('ini', 'value2 default=unknown section=section1 file=lookup.ini')}}" + unknown: "{{lookup('ini', 'unknown default=unknown section=section1 file=lookup.ini')}}" - debug: var=unknown + - assert: + that: + - 'unknown == "unknown"' - name: "Looping over section section1" debug: msg="{{item}}" with_ini: value[1-2] section=section1 file=lookup.ini re=true + register: _ + - assert: + that: + - '_.results.0.item == "section1/value1"' + - '_.results.1.item == "section1/value2"' - name: "Looping over section value_section" debug: msg="{{item}}" with_ini: value[1-2] section=value_section file=lookup.ini re=true + register: _ + - assert: + that: + - '_.results.0.item == "1"' + - '_.results.1.item == "2"' - debug: msg="{{item}}" with_ini: value[1-2] section=section1 file=lookup.ini re=true register: _ - - debug: var=_ + - assert: + that: + - '_.results.0.item == "section1/value1"' + - '_.results.1.item == "section1/value2"' diff --git a/test/runner/lib/git.py b/test/runner/lib/git.py index 5cea4f8c66..c380136c82 100644 --- a/test/runner/lib/git.py +++ b/test/runner/lib/git.py @@ -24,7 +24,7 @@ class Git(object): :rtype: list[str] """ cmd = ['diff'] + args - return self.run_git_split(cmd, '\n') + return self.run_git_split(cmd, '\n', str_errors='replace') def get_diff_names(self, args): """ @@ -76,22 +76,24 @@ class Git(object): except SubprocessError: return False - def run_git_split(self, cmd, separator=None): + def run_git_split(self, cmd, separator=None, str_errors='strict'): """ :type cmd: list[str] :param separator: str | None + :type str_errors: 'strict' | 'replace' :rtype: list[str] """ - output = self.run_git(cmd).strip(separator) + output = self.run_git(cmd, str_errors=str_errors).strip(separator) if not output: return [] return output.split(separator) - def run_git(self, cmd): + def run_git(self, cmd, str_errors='strict'): """ :type cmd: list[str] + :type str_errors: 'strict' | 'replace' :rtype: str """ - return run_command(self.args, [self.git] + cmd, capture=True, always=True)[0] + return run_command(self.args, [self.git] + cmd, capture=True, always=True, str_errors=str_errors)[0] diff --git a/test/runner/lib/util.py b/test/runner/lib/util.py index 4abfdd36d1..7d2e14beed 100644 --- a/test/runner/lib/util.py +++ b/test/runner/lib/util.py @@ -81,7 +81,7 @@ def find_executable(executable, cwd=None, path=None, required=True): def run_command(args, cmd, capture=False, env=None, data=None, cwd=None, always=False, stdin=None, stdout=None, - cmd_verbosity=1): + cmd_verbosity=1, str_errors='strict'): """ :type args: CommonConfig :type cmd: collections.Iterable[str] @@ -93,15 +93,16 @@ def run_command(args, cmd, capture=False, env=None, data=None, cwd=None, always= :type stdin: file | None :type stdout: file | None :type cmd_verbosity: int + :type str_errors: 'strict' | 'replace' :rtype: str | None, str | None """ explain = args.explain and not always return raw_command(cmd, capture=capture, env=env, data=data, cwd=cwd, explain=explain, stdin=stdin, stdout=stdout, - cmd_verbosity=cmd_verbosity) + cmd_verbosity=cmd_verbosity, str_errors=str_errors) def raw_command(cmd, capture=False, env=None, data=None, cwd=None, explain=False, stdin=None, stdout=None, - cmd_verbosity=1): + cmd_verbosity=1, str_errors='strict'): """ :type cmd: collections.Iterable[str] :type capture: bool @@ -112,6 +113,7 @@ def raw_command(cmd, capture=False, env=None, data=None, cwd=None, explain=False :type stdin: file | None :type stdout: file | None :type cmd_verbosity: int + :type str_errors: 'strict' | 'replace' :rtype: str | None, str | None """ if not cwd: @@ -170,8 +172,8 @@ def raw_command(cmd, capture=False, env=None, data=None, cwd=None, explain=False encoding = 'utf-8' data_bytes = data.encode(encoding) if data else None stdout_bytes, stderr_bytes = process.communicate(data_bytes) - stdout_text = stdout_bytes.decode(encoding) if stdout_bytes else u'' - stderr_text = stderr_bytes.decode(encoding) if stderr_bytes else u'' + stdout_text = stdout_bytes.decode(encoding, str_errors) if stdout_bytes else u'' + stderr_text = stderr_bytes.decode(encoding, str_errors) if stderr_bytes else u'' else: process.wait() stdout_text, stderr_text = None, None