Add module_utils and filters for quoting and unquoting (#53)

* Move splitting code to own file.

* Move list to dictionary code to quoting as well.

* Add quoting functionality.

* Add quoting filters.

* Add integration tests to CI.

* Fix bugs, increase coverage.

* Make parsing more strict.

* Extract function parse_argument_value from split_routeros_command to make proper parsing of WHERE possible.

* Adjust expected error message in integration tests.

* Simplify code and improve coverage.

* Add changelog fragment for WHERE strictness in api module.

* Add documenation.

* Fix typo.

* Add documentation references.

* Add example to api module which uses quote_argument_value.

* Fix bug, and add tests which prevent this in the future.

* Add more escape sequence tests.

* Make sure all control characters are quoted.

plugins/filter/quoting.py

@@ -0,0 +1,113 @@
+# -*- coding: utf-8 -*-
+
+# Copyright: (c) 2021, Felix Fontein <felix@fontein.de>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+from ansible.errors import AnsibleFilterError
+from ansible.module_utils.common.text.converters import to_text
+
+from ansible_collections.community.routeros.plugins.module_utils.quoting import (
+    ParseError,
+    convert_list_to_dictionary,
+    join_routeros_command,
+    quote_routeros_argument,
+    quote_routeros_argument_value,
+    split_routeros_command,
+)
+
+
+def wrap_exception(fn, *args, **kwargs):
+    try:
+        return fn(*args, **kwargs)
+    except ParseError as e:
+        raise AnsibleFilterError(to_text(e))
+
+
+def split(line):
+    '''
+    Split a command into arguments.
+
+    Example:
+        'add name=wrap comment="with space"'
+    is converted to:
+        ['add', 'name=wrap', 'comment=with space']
+    '''
+    return wrap_exception(split_routeros_command, line)
+
+
+def quote_argument_value(argument):
+    '''
+    Quote an argument value.
+
+    Example:
+        'with "space"'
+    is converted to:
+        r'"with \"space\""'
+    '''
+    return wrap_exception(quote_routeros_argument_value, argument)
+
+
+def quote_argument(argument):
+    '''
+    Quote an argument.
+
+    Example:
+        'comment=with "space"'
+    is converted to:
+        r'comment="with \"space\""'
+    '''
+    return wrap_exception(quote_routeros_argument, argument)
+
+
+def join(arguments):
+    '''
+    Join a list of arguments to a command.
+
+    Example:
+        ['add', 'name=wrap', 'comment=with space']
+    is converted to:
+        'add name=wrap comment="with space"'
+    '''
+    return wrap_exception(join_routeros_command, arguments)
+
+
+def list_to_dict(string_list, require_assignment=True, skip_empty_values=False):
+    '''
+    Convert a list of arguments to a list of dictionary.
+
+    Example:
+        ['foo=bar', 'comment=with space', 'additional=']
+    is converted to:
+        {'foo': 'bar', 'comment': 'with space', 'additional': ''}
+
+    If require_assignment is True (default), arguments without assignments are
+    rejected. (Example: in ['add', 'name=foo'], 'add' is an argument without
+    assignment.) If it is False, these are given value None.
+
+    If skip_empty_values is True, arguments with empty value are removed from
+    the result. (Example: in ['name='], 'name' has an empty value.)
+    If it is False (default), these are kept.
+
+    '''
+    return wrap_exception(
+        convert_list_to_dictionary,
+        string_list,
+        require_assignment=require_assignment,
+        skip_empty_values=skip_empty_values,
+    )
+
+
+class FilterModule(object):
+    '''Ansible jinja2 filters for RouterOS command quoting and unquoting'''
+
+    def filters(self):
+        return {
+            'split': split,
+            'quote_argument': quote_argument,
+            'quote_argument_value': quote_argument_value,
+            'join': join,
+            'list_to_dict': list_to_dict,
+        }

plugins/module_utils/quoting.py

@@ -0,0 +1,206 @@
+# -*- coding: utf-8 -*-
+
+# Copyright: (c) 2021, Felix Fontein (@felixfontein) <felix@fontein.de>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+
+import sys
+
+from ansible.module_utils.common.text.converters import to_native, to_bytes
+
+
+class ParseError(Exception):
+    pass
+
+
+ESCAPE_SEQUENCES = {
+    b'"': b'"',
+    b'\\': b'\\',
+    b'?': b'?',
+    b'$': b'$',
+    b'_': b'_',
+    b'a': b'\a',
+    b'b': b'\b',
+    b'f': b'\xFF',
+    b'n': b'\n',
+    b'r': b'\r',
+    b't': b'\t',
+    b'v': b'\v',
+}
+
+ESCAPE_SEQUENCE_REVERSED = dict([(v, k) for k, v in ESCAPE_SEQUENCES.items()])
+
+ESCAPE_DIGITS = b'0123456789ABCDEF'
+
+
+if sys.version_info[0] < 3:
+    _int_to_byte = chr
+else:
+    def _int_to_byte(value):
+        return bytes((value, ))
+
+
+def parse_argument_value(line, start_index=0, must_match_everything=True):
+    '''
+    Parse an argument value (quoted or not quoted) from ``line``.
+
+    Will start at offset ``start_index``. Returns pair ``(parsed_value,
+    end_index)``, where ``end_index`` is the first character after the
+    attribute.
+
+    If ``must_match_everything`` is ``True`` (default), will fail if
+    ``end_index < len(line)``.
+    '''
+    line = to_bytes(line)
+    length = len(line)
+    index = start_index
+    if index == length:
+        raise ParseError('Expected value, but found end of string')
+    quoted = False
+    if line[index:index + 1] == b'"':
+        quoted = True
+        index += 1
+    current = []
+    while index < length:
+        ch = line[index:index + 1]
+        index += 1
+        if not quoted and ch == b' ':
+            index -= 1
+            break
+        elif ch == b'"':
+            if quoted:
+                quoted = False
+                if line[index:index + 1] not in (b'', b' '):
+                    raise ParseError('Ending \'"\' must be followed by space or end of string')
+                break
+            raise ParseError('\'"\' must not appear in an unquoted value')
+        elif ch == b'\\':
+            if not quoted:
+                raise ParseError('Escape sequences can only be used inside double quotes')
+            if index == length:
+                raise ParseError('\'\\\' must not be at the end of the line')
+            ch = line[index:index + 1]
+            index += 1
+            if ch in ESCAPE_SEQUENCES:
+                current.append(ESCAPE_SEQUENCES[ch])
+            else:
+                d1 = ESCAPE_DIGITS.find(ch)
+                if d1 < 0:
+                    raise ParseError('Invalid escape sequence \'\\{0}\''.format(to_native(ch)))
+                if index == length:
+                    raise ParseError('Hex escape sequence cut off at end of line')
+                ch2 = line[index:index + 1]
+                d2 = ESCAPE_DIGITS.find(ch2)
+                index += 1
+                if d2 < 0:
+                    raise ParseError('Invalid hex escape sequence \'\\{0}\''.format(to_native(ch + ch2)))
+                current.append(_int_to_byte(d1 * 16 + d2))
+        else:
+            if not quoted and ch in (b"'", b'=', b'(', b')', b'$', b'[', b'{', b'`'):
+                raise ParseError('"{0}" can only be used inside double quotes'.format(to_native(ch)))
+            if ch == b'?':
+                raise ParseError('"{0}" can only be used in escaped form'.format(to_native(ch)))
+            current.append(ch)
+    if quoted:
+        raise ParseError('Unexpected end of string during escaped parameter')
+    if must_match_everything and index < length:
+        raise ParseError('Unexpected data at end of value')
+    return to_native(b''.join(current)), index
+
+
+def split_routeros_command(line):
+    line = to_bytes(line)
+    result = []
+    current = []
+    index = 0
+    length = len(line)
+    parsing_attribute_name = False
+    while index < length:
+        ch = line[index:index + 1]
+        index += 1
+        if ch == b' ':
+            if parsing_attribute_name:
+                parsing_attribute_name = False
+                result.append(b''.join(current))
+                current = []
+        elif ch == b'=' and parsing_attribute_name:
+            current.append(ch)
+            value, index = parse_argument_value(line, start_index=index, must_match_everything=False)
+            current.append(to_bytes(value))
+            parsing_attribute_name = False
+            result.append(b''.join(current))
+            current = []
+        elif ch in (b'"', b'\\', b"'", b'=', b'(', b')', b'$', b'[', b'{', b'`', b'?'):
+            raise ParseError('Found unexpected "{0}"'.format(to_native(ch)))
+        else:
+            current.append(ch)
+            parsing_attribute_name = True
+    if parsing_attribute_name and current:
+        result.append(b''.join(current))
+    return [to_native(part) for part in result]
+
+
+def quote_routeros_argument_value(argument):
+    argument = to_bytes(argument)
+    result = []
+    quote = False
+    length = len(argument)
+    index = 0
+    while index < length:
+        letter = argument[index:index + 1]
+        index += 1
+        if letter in ESCAPE_SEQUENCE_REVERSED:
+            result.append(b'\\%s' % ESCAPE_SEQUENCE_REVERSED[letter])
+            quote = True
+            continue
+        elif ord(letter) < 32:
+            v = ord(letter)
+            v1 = v % 16
+            v2 = v // 16
+            result.append(b'\\%s%s' % (ESCAPE_DIGITS[v2:v2 + 1], ESCAPE_DIGITS[v1:v1 + 1]))
+            quote = True
+            continue
+        elif letter in (b' ', b'=', b';', b"'"):
+            quote = True
+        result.append(letter)
+    argument = to_native(b''.join(result))
+    if quote or not argument:
+        argument = '"%s"' % argument
+    return argument
+
+
+def quote_routeros_argument(argument):
+    def check_attribute(attribute):
+        if ' ' in attribute:
+            raise ParseError('Attribute names must not contain spaces')
+        return attribute
+
+    if '=' not in argument:
+        check_attribute(argument)
+        return argument
+
+    attribute, value = argument.split('=', 1)
+    check_attribute(attribute)
+    value = quote_routeros_argument_value(value)
+    return '%s=%s' % (attribute, value)
+
+
+def join_routeros_command(arguments):
+    return ' '.join([quote_routeros_argument(argument) for argument in arguments])
+
+
+def convert_list_to_dictionary(string_list, require_assignment=True, skip_empty_values=False):
+    dictionary = {}
+    for p in string_list:
+        if '=' not in p:
+            if require_assignment:
+                raise ParseError("missing '=' after '%s'" % p)
+            dictionary[p] = None
+            continue
+        p = p.split('=', 1)
+        if not skip_empty_values or p[1]:
+            dictionary[p[0]] = p[1]
+    return dictionary

plugins/modules/api.py

@@ -119,6 +119,11 @@ options:
       - See also I(validate_cert_hostname). Only used when I(tls=true) and I(validate_certs=true).
     type: path
     version_added: 1.2.0
+seealso:
+  - ref: ansible_collections.community.routeros.docsite.api-guide
+    description: How to connect to RouterOS devices with the RouterOS API
+  - ref: ansible_collections.community.routeros.docsite.quoting
+    description: How to quote and unquote commands and arguments
 '''
 
 EXAMPLES = '''
@@ -190,7 +195,10 @@ EXAMPLES = '''
         password: "{{ password }}"
         username: "{{ username }}"
         path: "{{ path }}"
-        update: ".id={{ query_id }} address={{ ip3 }}"
+        update: >-
+            .id={{ query_id }}
+            address={{ ip3 }}
+            comment={{ 'A comment with spaces' | community.routeros.quote_argument_value }}
       register: updateout
 
     - name: Dump "Update" output
@@ -258,8 +266,16 @@ message:
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.basic import missing_required_lib
-from ansible.module_utils.common.text.converters import to_native, to_bytes
+from ansible.module_utils.common.text.converters import to_native
 
+from ansible_collections.community.routeros.plugins.module_utils.quoting import (
+    ParseError,
+    convert_list_to_dictionary,
+    parse_argument_value,
+    split_routeros_command,
+)
+
+import re
 import ssl
 import traceback
 
@@ -274,95 +290,6 @@ except Exception as e:
     LIB_IMP_ERR = traceback.format_exc()
 
 
-class ParseError(Exception):
-    pass
-
-
-ESCAPE_SEQUENCES = {
-    b'"': b'"',
-    b'\\': b'\\',
-    b'?': b'?',
-    b'$': b'$',
-    b'_': b'_',
-    b'a': b'\a',
-    b'b': b'\b',
-    b'f': b'\xFF',
-    b'n': b'\n',
-    b'r': b'\r',
-    b't': b'\t',
-    b'v': b'\v',
-}
-
-ESCAPE_DIGITS = b'0123456789ABCDEF'
-
-
-def split_routeros(line):
-    line = to_bytes(line)
-    result = []
-    current = []
-    index = 0
-    length = len(line)
-    # States:
-    #   0 = outside param
-    #   1 = param before '='
-    #   2 = param after '=' without quote
-    #   3 = param after '=' with quote
-    state = 0
-    while index < length:
-        ch = line[index:index + 1]
-        index += 1
-        if state == 0 and ch == b' ':
-            pass
-        elif state in (1, 2) and ch == b' ':
-            state = 0
-            result.append(b''.join(current))
-            current = []
-        elif ch == b'=' and state == 1:
-            state = 2
-            current.append(ch)
-            if index + 1 < length and line[index:index + 1] == b'"':
-                state = 3
-                index += 1
-        elif ch == b'"':
-            if state == 3:
-                state = 0
-                result.append(b''.join(current))
-                current = []
-                if index + 1 < length and line[index:index + 1] != b' ':
-                    raise ParseError('Ending \'"\' must be followed by space or end of string')
-            else:
-                raise ParseError('\'"\' must follow \'=\'')
-        elif ch == b'\\':
-            if index + 1 == length:
-                raise ParseError('\'\\\' must not be at the end of the line')
-            ch = line[index:index + 1]
-            index += 1
-            if ch in ESCAPE_SEQUENCES:
-                current.append(ch)
-            else:
-                d1 = ESCAPE_DIGITS.find(ch)
-                if d1 < 0:
-                    raise ParseError('Invalid escape sequence \'\\{0}\''.format(ch))
-                if index + 1 == length:
-                    raise ParseError('Hex escape sequence cut off at end of line')
-                ch2 = line[index:index + 1]
-                d2 = ESCAPE_DIGITS.find(ch2)
-                index += 1
-                if d2 < 0:
-                    raise ParseError('Invalid hex escape sequence \'\\{0}{1}\''.format(ch, ch2))
-                result.append(chr(d1 * 16 + d2))
-        else:
-            current.append(ch)
-            if state == 0:
-                state = 1
-    if state in (1, 2):
-        if current:
-            result.append(b''.join(current))
-    elif state == 3:
-        raise ParseError('Unexpected end of string during escaped parameter')
-    return [to_native(part) for part in result]
-
-
 class ROS_api_module:
     def __init__(self):
         module_args = dict(
@@ -410,7 +337,24 @@ class ROS_api_module:
         self.where = None
         self.query = self.module.params['query']
         if self.query:
-            self.query = self.list_remove_empty(self.split_params(self.query))
+            where_index = self.query.find(' WHERE ')
+            if where_index < 0:
+                self.query = self.split_params(self.query)
+            else:
+                where = self.query[where_index + len(' WHERE '):]
+                self.query = self.split_params(self.query[:where_index])
+                # where must be of the format '<attribute> <operator> <value>'
+                m = re.match(r'^\s*([^ ]+)\s+([^ ]+)\s+(.*)$', where)
+                if not m:
+                    self.errors("invalid syntax for 'WHERE %s'" % where)
+                try:
+                    self.where = [
+                        m.group(1),  # attribute
+                        m.group(2),  # operator
+                        parse_argument_value(m.group(3).rstrip())[0],  # value
+                    ]
+                except ParseError as exc:
+                    self.errors("invalid syntax for 'WHERE %s': %s" % (where, exc))
             try:
                 idx = self.query.index('WHERE')
                 self.where = self.query[idx + 1:]
@@ -439,26 +383,14 @@ class ROS_api_module:
         else:
             self.api_get_all()
 
-    def list_remove_empty(self, check_list):
-        while("" in check_list):
-            check_list.remove("")
-        return check_list
-
     def list_to_dic(self, ldict):
-        dict = {}
-        for p in ldict:
-            if '=' not in p:
-                self.errors("missing '=' after '%s'" % p)
-            p = p.split('=', 1)
-            if p[1]:
-                dict[p[0]] = p[1]
-        return dict
+        return convert_list_to_dictionary(ldict, skip_empty_values=True, require_assignment=True)
 
     def split_params(self, params):
         if not isinstance(params, str):
             raise AssertionError('Parameters can only be a string, received %s' % type(params))
         try:
-            return split_routeros(params)
+            return split_routeros_command(params)
         except ParseError as e:
             self.module.fail_json(msg=to_native(e))
 
@@ -512,11 +444,6 @@ class ROS_api_module:
             keys[k] = Key(k)
         try:
             if self.where:
-                if len(self.where) < 3:
-                    self.errors("invalid syntax for 'WHERE %s'"
-                                % ' '.join(self.where))
-
-                where = []
                 if self.where[1] == '==':
                     select = self.api_path.select(*keys).where(keys[self.where[0]] == self.where[2])
                 elif self.where[1] == '!=':
@@ -528,11 +455,10 @@ class ROS_api_module:
                 else:
                     self.errors("'%s' is not operator for 'where'"
                                 % self.where[1])
-                for row in select:
-                    self.result['message'].append(row)
             else:
-                for row in self.api_path.select(*keys):
-                    self.result['message'].append(row)
+                select = self.api_path.select(*keys)
+            for row in select:
+                self.result['message'].append(row)
             if len(self.result['message']) < 1:
                 msg = "no results for '%s 'query' %s" % (' '.join(self.path),
                                                          ' '.join(self.query))

plugins/modules/command.py

@@ -58,6 +58,11 @@ options:
         conditions, the interval indicates how long to wait before
         trying the command again.
     default: 1
+seealso:
+  - ref: ansible_collections.community.routeros.docsite.ssh-guide
+    description: How to connect to RouterOS devices with SSH
+  - ref: ansible_collections.community.routeros.docsite.quoting
+    description: How to quote and unquote commands and arguments
 '''
 
 EXAMPLES = """

plugins/modules/facts.py

@@ -27,6 +27,9 @@ options:
         not be collected.
     required: false
     default: '!config'
+seealso:
+  - ref: ansible_collections.community.routeros.docsite.ssh-guide
+    description: How to connect to RouterOS devices with SSH
 '''
 
 EXAMPLES = """