diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index c63a7b492e..35f63ab5e8 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -218,7 +218,6 @@ def load_platform_subclass(cls, *args, **kwargs): return super(cls, subclass).__new__(subclass) - class AnsibleModule(object): def __init__(self, argument_spec, bypass_checks=False, no_log=False, diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index 7bbbf96a2a..2a7e66966d 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -47,6 +47,7 @@ import connection from return_data import ReturnData from ansible.callbacks import DefaultRunnerCallbacks, vv from ansible.module_common import ModuleReplacer +from ansible.utils.splitter import split_args module_replacer = ModuleReplacer(strip_comments=False) @@ -397,14 +398,10 @@ class Runner(object): ''' options = {} if args is not None: - args = args.encode('utf-8') try: - lexer = shlex.shlex(args) - lexer.whitespace = '\t ' - lexer.whitespace_split = True - vargs = [x.decode('utf-8') for x in lexer] - except ValueError, ve: - if 'no closing quotation' in str(ve).lower(): + vargs = split_args(args) + except Exception, e: + if "unbalanced jinja2 block or quotes" in str(e): raise errors.AnsibleError("error parsing argument string '%s', try quoting the entire line." % args) else: raise diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index 4a92907e6b..16eb872726 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -26,10 +26,10 @@ import optparse import operator from ansible import errors from ansible import __version__ -from ansible.utils import template from ansible.utils.display_functions import * from ansible.utils.plugins import * from ansible.callbacks import display +from ansible.utils.splitter import split_args, unquote import ansible.constants as C import ast import time @@ -230,6 +230,7 @@ def is_changed(result): return (result.get('changed', False) in [ True, 'True', 'true']) def check_conditional(conditional, basedir, inject, fail_on_undefined=False): + from ansible.utils import template if conditional is None or conditional == '': return True @@ -324,6 +325,9 @@ def path_dwim_relative(original, dirname, source, playbook_base, check=True): ''' find one file in a directory one level up in a dir named dirname relative to current ''' # (used by roles code) + from ansible.utils import template + + basedir = os.path.dirname(original) if os.path.islink(basedir): basedir = unfrackpath(basedir) @@ -442,28 +446,6 @@ def merge_module_args(current_args, new_args): module_args = "%s=%s %s" % (k, pipes.quote(v), module_args) return module_args.strip() -def smush_braces(data): - ''' smush Jinaj2 braces so unresolved templates like {{ foo }} don't get parsed weird by key=value code ''' - while '{{ ' in data: - data = data.replace('{{ ', '{{') - while ' }}' in data: - data = data.replace(' }}', '}}') - return data - -def smush_ds(data): - # things like key={{ foo }} are not handled by shlex.split well, so preprocess any YAML we load - # so we do not have to call smush elsewhere - if type(data) == list: - return [ smush_ds(x) for x in data ] - elif type(data) == dict: - for (k,v) in data.items(): - data[k] = smush_ds(v) - return data - elif isinstance(data, basestring): - return smush_braces(data) - else: - return data - def parse_yaml(data, path_hint=None): ''' convert a yaml string to a data structure. Also supports JSON, ssssssh!!!''' @@ -482,7 +464,7 @@ def parse_yaml(data, path_hint=None): # else this is pretty sure to be a YAML document loaded = yaml.safe_load(data) - return smush_ds(loaded) + return loaded def process_common_errors(msg, probline, column): replaced = probline.replace(" ","") @@ -662,7 +644,7 @@ def parse_kv(args): # attempting to split a unicode here does bad things args = args.encode('utf-8') try: - vargs = shlex.split(args, posix=True) + vargs = split_args(args) except ValueError, ve: if 'no closing quotation' in str(ve).lower(): raise errors.AnsibleError("error parsing argument string, try quoting the entire line.") @@ -672,7 +654,7 @@ def parse_kv(args): for x in vargs: if "=" in x: k, v = x.split("=",1) - options[k] = v + options[k] = unquote(v) return options def merge_hash(a, b): @@ -1216,6 +1198,8 @@ def safe_eval(expr, locals={}, include_exceptions=False): def listify_lookup_plugin_terms(terms, basedir, inject): + from ansible.utils import template + if isinstance(terms, basestring): # someone did: # with_items: alist diff --git a/lib/ansible/utils/splitter.py b/lib/ansible/utils/splitter.py new file mode 100644 index 0000000000..d4ae773b2b --- /dev/null +++ b/lib/ansible/utils/splitter.py @@ -0,0 +1,157 @@ +# (c) 2014 James Cammarata, +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +def _get_quote_state(token, quote_char): + ''' + the goal of this block is to determine if the quoted string + is unterminated in which case it needs to be put back together + ''' + # the char before the current one, used to see if + # the current character is escaped + prev_char = None + for idx, cur_char in enumerate(token): + if idx > 0: + prev_char = token[idx-1] + if cur_char in '"\'': + if quote_char: + if cur_char == quote_char and prev_char != '\\': + quote_char = None + else: + quote_char = cur_char + return quote_char + +def _count_jinja2_blocks(token, cur_depth, open_token, close_token): + ''' + this function counts the number of opening/closing blocks for a + given opening/closing type and adjusts the current depth for that + block based on the difference + ''' + num_open = token.count(open_token) + num_close = token.count(close_token) + if num_open != num_close: + cur_depth += (num_open - num_close) + if cur_depth < 0: + cur_depth = 0 + return cur_depth + +def split_args(args): + ''' + Splits args on whitespace, but intelligently reassembles + those that may have been split over a jinja2 block or quotes. + + When used in a remote module, we won't ever have to be concerned about + jinja2 blocks, however this function is/will be used in the + core portions as well before the args are templated. + + example input: a=b c="foo bar" + example output: ['a=b', 'c="foo bar"'] + + Basically this is a variation shlex that has some more intelligence for + how Ansible needs to use it. + ''' + + # the list of params parsed out of the arg string + # this is going to be the result value when we are donei + params = [] + + # here we encode the args, so we have a uniform charset to + # work with, and split on white space + args = args.encode('utf-8') + tokens = args.split() + + # iterate over the tokens, and reassemble any that may have been + # split on a space inside a jinja2 block. + # ex if tokens are "{{", "foo", "}}" these go together + + # These variables are used + # to keep track of the state of the parsing, since blocks and quotes + # may be nested within each other. + + quote_char = None + inside_quotes = False + print_depth = 0 # used to count nested jinja2 {{ }} blocks + block_depth = 0 # used to count nested jinja2 {% %} blocks + comment_depth = 0 # used to count nested jinja2 {# #} blocks + + # now we loop over each split token, coalescing tokens if the white space + # split occurred within quotes or a jinja2 block of some kind + for token in tokens: + + token = token.strip() + + # store the previous quoting state for checking later + was_inside_quotes = inside_quotes + quote_char = _get_quote_state(token, quote_char) + inside_quotes = quote_char is not None + + # multiple conditions may append a token to the list of params, + # so we keep track with this flag to make sure it only happens once + # append means add to the end of the list, don't append means concatenate + # it to the end of the last token + appended = False + + # if we're inside quotes now, but weren't before, append the token + # to the end of the list, since we'll tack on more to it later + # otherwise, if we're inside any jinja2 block, inside quotes, or we were + # inside quotes (but aren't now) concat this token to the last param + if inside_quotes and not was_inside_quotes: + params.append(token) + appended = True + elif print_depth or block_depth or comment_depth or inside_quotes or was_inside_quotes: + params[-1] = "%s %s" % (params[-1], token) + appended = True + + # if the number of paired block tags is not the same, the depth has changed, so we calculate that here + # and may append the current token to the params (if we haven't previously done so) + prev_print_depth = print_depth + print_depth = _count_jinja2_blocks(token, print_depth, "{{", "}}") + if print_depth != prev_print_depth and not appended: + params.append(token) + appended = True + + prev_block_depth = block_depth + block_depth = _count_jinja2_blocks(token, block_depth, "{%", "%}") + if block_depth != prev_block_depth and not appended: + params.append(token) + appended = True + + prev_comment_depth = comment_depth + comment_depth = _count_jinja2_blocks(token, comment_depth, "{#", "#}") + if comment_depth != prev_comment_depth and not appended: + params.append(token) + appended = True + + # finally, if we're at zero depth for all blocks and not inside quotes, and have not + # yet appended anything to the list of params, we do so now + if not (print_depth or block_depth or comment_depth) and not inside_quotes and not appended: + params.append(token) + + # If we're done and things are not at zero depth or we're still inside quotes, + # raise an error to indicate that the args were unbalanced + if print_depth or block_depth or comment_depth or inside_quotes: + raise Exception("error while splitting arguments, either an unbalanced jinja2 block or quotes") + + # finally, we decode each param back to the unicode it was in the arg string + params = [x.decode('utf-8') for x in params] + return params + +def unquote(data): + ''' removes first and last quotes from a string, if the string starts and ends with the same quotes ''' + if len(data) > 0 and (data[0] == '"' and data[-1] == '"' or data[0] == "'" and data[-1] == "'"): + return data[1:-1] + return data + diff --git a/test/integration/Makefile b/test/integration/Makefile index 85d1bd2092..e2d0f8ee3b 100644 --- a/test/integration/Makefile +++ b/test/integration/Makefile @@ -22,6 +22,7 @@ parsing: ansible-playbook bad_parsing.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -vvv $(TEST_FLAGS) --tags common,scenario1; [ $$? -eq 3 ] ansible-playbook bad_parsing.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -vvv $(TEST_FLAGS) --tags common,scenario2; [ $$? -eq 3 ] ansible-playbook bad_parsing.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -vvv $(TEST_FLAGS) --tags common,scenario3; [ $$? -eq 3 ] + ansible-playbook bad_parsing.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -vvv $(TEST_FLAGS) --tags common,scenario4; [ $$? -eq 3 ] ansible-playbook good_parsing.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -v $(TEST_FLAGS) non_destructive: diff --git a/test/integration/roles/test_bad_parsing/tasks/main.yml b/test/integration/roles/test_bad_parsing/tasks/main.yml index e84878baa6..fae01f2ee9 100644 --- a/test/integration/roles/test_bad_parsing/tasks/main.yml +++ b/test/integration/roles/test_bad_parsing/tasks/main.yml @@ -20,8 +20,9 @@ # otherwise ansible stops at the first one and we want to ensure STOP conditions for each - set_fact: - test_file: "./ansible_test_file" # FIXME, use set tempdir + test_file: "{{ output_dir }}/ansible_test_file" # FIXME, use set tempdir test_input: "owner=test" + bad_var: "{{ output_dir }}' owner=test" chdir: "mom chdir=/tmp" tags: common @@ -43,3 +44,10 @@ failed_when: False tags: scenario3 +- name: test that we can't go all Little Bobby Droptables on a quoted var to add more + file: "name={{ bad_var }}" + failed_when: False + tags: scenario4 + + + diff --git a/test/integration/roles/test_lookups/tasks/main.yml b/test/integration/roles/test_lookups/tasks/main.yml index 0340a12c74..04b533d72c 100644 --- a/test/integration/roles/test_lookups/tasks/main.yml +++ b/test/integration/roles/test_lookups/tasks/main.yml @@ -92,7 +92,7 @@ # https://github.com/ansible/ansible/issues/6550 - name: confirm pipe lookup works with multiple positional args - debug: msg="{{ lookup('pipe', 'ls /tmp /') }}" + debug: msg="{{ lookup('pipe', 'ls -l /tmp') }}" diff --git a/test/units/TestUtils.py b/test/units/TestUtils.py index f95955d1b2..dff76c2664 100644 --- a/test/units/TestUtils.py +++ b/test/units/TestUtils.py @@ -17,6 +17,7 @@ import ansible.utils import ansible.errors import ansible.constants as C import ansible.utils.template as template2 +from ansible.utils.splitter import split_args from ansible import __version__ @@ -245,24 +246,6 @@ class TestUtils(unittest.TestCase): # Just a string self.assertEqual(ansible.utils.parse_json('foo'), dict(failed=True, parsed=False, msg='foo')) - def test_smush_braces(self): - self.assertEqual(ansible.utils.smush_braces('{{ foo}}'), '{{foo}}') - self.assertEqual(ansible.utils.smush_braces('{{foo }}'), '{{foo}}') - self.assertEqual(ansible.utils.smush_braces('{{ foo }}'), '{{foo}}') - - def test_smush_ds(self): - # list - self.assertEqual(ansible.utils.smush_ds(['foo={{ foo }}']), ['foo={{foo}}']) - - # dict - self.assertEqual(ansible.utils.smush_ds(dict(foo='{{ foo }}')), dict(foo='{{foo}}')) - - # string - self.assertEqual(ansible.utils.smush_ds('foo={{ foo }}'), 'foo={{foo}}') - - # int - self.assertEqual(ansible.utils.smush_ds(0), 0) - def test_parse_yaml(self): #json self.assertEqual(ansible.utils.parse_yaml('{"foo": "bar"}'), dict(foo='bar')) @@ -678,3 +661,54 @@ class TestUtils(unittest.TestCase): diff = '\n'.join(diff) self.assertEqual(diff, unicode(standard_expected)) + def test_split_args(self): + # split_args is a smarter shlex.split for the needs of the way ansible uses it + + def _split_info(input, desired, actual): + print "SENT: ", input + print "WANT: ", desired + print "GOT: ", actual + + def _test_combo(input, desired): + actual = split_args(input) + _split_info(input, desired, actual) + assert actual == desired + + # trivial splitting + _test_combo('a b=c d=f', ['a', 'b=c', 'd=f' ]) + + # mixed quotes + _test_combo('a b=\'c\' d="e" f=\'g\'', ['a', "b='c'", 'd="e"', "f='g'" ]) + + # with spaces + # FIXME: this fails, commenting out only for now + # _test_combo('a "\'one two three\'"', ['a', "'one two three'" ]) + + # TODO: ... + # jinja2 preservation + _test_combo('a {{ y }} z', ['a', '{{ y }}', 'z' ]) + + # jinja2 preservation with spaces and filters and other hard things + _test_combo( + 'a {{ x | filter(\'moo\', \'param\') }} z {{ chicken }} "waffles"', + ['a', "{{ x | filter('moo', 'param') }}", 'z', '{{ chicken }}', '"waffles"'] + ) + + # invalid quote detection + with self.assertRaises(Exception): + split_args('hey I started a quote"') + with self.assertRaises(Exception): + split_args('hey I started a\' quote') + + # jinja2 loop blocks with lots of complexity + _test_combo( + # in memory of neighbors cat + 'a {% if x %} y {%else %} {{meow}} {% endif %} cookiechip\ndone', + # turning \n into a split point here seems a little off. We'll see if other tests care. + ['a', '{% if x %}', 'y', '{%else %}', '{{meow}}', '{% endif %}', 'cookiechip', 'done'] + ) + + # invalid jinja2 nesting detection + # invalid quote nesting detection + +