Only split CSV in the client, quote instead of escaping

Splitting on commas is not an idempotent operation:
'a,b\,c' -> ('a', 'b,c') -> ('a', 'b', 'c')

That means we can't do it when the call is forwarded, so this is only
done on the CLI. The UI already sends values as a tuple.

Replace escaping in the csv parser with quoting. Quoted strings can have
embedded commas instead of having to escape them. This prevents the csv
parser from eating all escape characters.

Also, document Param's csv arguments, and update tests.

https://fedorahosted.org/freeipa/ticket/2417
https://fedorahosted.org/freeipa/ticket/2227

6 files changed, 25 insertions, 27 deletions

diff --git a/tests/test_ipalib/test_parameters.py b/tests/test_ipalib/test_parameters.py
index ad8d8404..83c33ddf 100644
--- a/tests/test_ipalib/test_parameters.py
+++ b/tests/test_ipalib/test_parameters.py
@@ -635,44 +635,44 @@ class test_Param(ClassChecker):
         assert o._convert_scalar.value is default
         assert o.normalizer.value is default
 
-    def test_csv_normalize(self):
+    def test_split_csv(self):
         """
-        Test the `ipalib.parameters.Param.normalize` method with csv.
+        Test the `ipalib.parameters.Param.split_csv` method with csv.
         """
         o = self.cls('my_list+', csv=True)
-        n = o.normalize('a,b')
+        n = o.split_csv('a,b')
         assert type(n) is tuple
         assert len(n) is 2
 
-        n = o.normalize('bar,   "hi, there",foo')
+        n = o.split_csv('bar,   "hi, there",foo')
         assert type(n) is tuple
         assert len(n) is 3
 
-    def test_csv_normalize_separator(self):
+    def test_split_csv_separator(self):
         """
-        Test the `ipalib.parameters.Param.normalize` method with csv and a separator.
+        Test the `ipalib.parameters.Param.split_csv` method with csv and a separator.
         """
         o = self.cls('my_list+', csv=True, csv_separator='|')
 
-        n = o.normalize('a')
+        n = o.split_csv('a')
         assert type(n) is tuple
         assert len(n) is 1
 
-        n = o.normalize('a|b')
+        n = o.split_csv('a|b')
         assert type(n) is tuple
         assert len(n) is 2
 
-    def test_csv_normalize_skipspace(self):
+    def test_split_csv_skipspace(self):
         """
-        Test the `ipalib.parameters.Param.normalize` method with csv without skipping spaces.
+        Test the `ipalib.parameters.Param.split_csv` method with csv without skipping spaces.
         """
         o = self.cls('my_list+', csv=True, csv_skipspace=False)
 
-        n = o.normalize('a')
+        n = o.split_csv('a')
         assert type(n) is tuple
         assert len(n) is 1
 
-        n = o.normalize('a, "b,c", d')
+        n = o.split_csv('a, "b,c", d')
         assert type(n) is tuple
         # the output w/o skipspace is ['a',' "b','c"',' d']
         assert len(n) is 4
diff --git a/tests/test_xmlrpc/test_delegation_plugin.py b/tests/test_xmlrpc/test_delegation_plugin.py
index db5f7186..5030f8bc 100644
--- a/tests/test_xmlrpc/test_delegation_plugin.py
+++ b/tests/test_xmlrpc/test_delegation_plugin.py
@@ -87,7 +87,7 @@ class test_delegation(Declarative):
             desc='Create %r' % delegation1,
             command=(
                 'delegation_add', [delegation1], dict(
-                     attrs=u'street,c,l,st,postalCode',
+                     attrs=[u'street', u'c', u'l', u'st', u'postalCode'],
                      permissions=u'write',
                      group=u'editors',
                      memberof=u'admins',
@@ -111,7 +111,7 @@ class test_delegation(Declarative):
             desc='Try to create duplicate %r' % delegation1,
             command=(
                 'delegation_add', [delegation1], dict(
-                     attrs=u'street,c,l,st,postalCode',
+                     attrs=[u'street', u'c', u'l', u'st', u'postalCode'],
                      permissions=u'write',
                      group=u'editors',
                      memberof=u'admins',
diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py
index e310d319..ec656519 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -724,8 +724,9 @@ class test_dns(Declarative):
 
         dict(
             desc='Add NSEC record to %r using dnsrecord_add' % (dnsres1),
-            command=('dnsrecord_add', [dnszone1, dnsres1], {'nsec_part_next': dnszone1,
-                                                            'nsec_part_types' : ['TXT', 'A']}),
+            command=('dnsrecord_add', [dnszone1, dnsres1], {
+                'nsec_part_next': dnszone1,
+                'nsec_part_types' : [u'TXT', u'A']}),
             expected={
                 'value': dnsres1,
                 'summary': None,
diff --git a/tests/test_xmlrpc/test_privilege_plugin.py b/tests/test_xmlrpc/test_privilege_plugin.py
index 58dbff85..94422558 100644
--- a/tests/test_xmlrpc/test_privilege_plugin.py
+++ b/tests/test_xmlrpc/test_privilege_plugin.py
@@ -87,7 +87,7 @@ class test_privilege(Declarative):
             command=(
                 'permission_add', [permission1], dict(
                      type=u'user',
-                     permissions=u'add, delete',
+                     permissions=[u'add', u'delete'],
                 )
             ),
             expected=dict(
diff --git a/tests/test_xmlrpc/test_role_plugin.py b/tests/test_xmlrpc/test_role_plugin.py
index f871e268..6f6de321 100644
--- a/tests/test_xmlrpc/test_role_plugin.py
+++ b/tests/test_xmlrpc/test_role_plugin.py
@@ -47,9 +47,6 @@ privilege1 = u'r,w privilege 1'
 privilege1_dn = DN(('cn', privilege1), DN(api.env.container_privilege),
                    api.env.basedn)
 
-def escape_comma(value):
-    return value.replace(',', '\\,')
-
 class test_role(Declarative):
 
     cleanup_commands = [
@@ -184,7 +181,7 @@ class test_role(Declarative):
         dict(
             desc='Add privilege %r to role %r' % (privilege1, role1),
             command=('role_add_privilege', [role1],
-                dict(privilege=escape_comma(privilege1))
+                dict(privilege=privilege1)
             ),
             expected=dict(
                 completed=1,
@@ -465,7 +462,7 @@ class test_role(Declarative):
         dict(
             desc='Remove privilege %r from role %r' % (privilege1, role1),
             command=('role_remove_privilege', [role1],
-                dict(privilege=escape_comma(privilege1))
+                dict(privilege=privilege1)
             ),
             expected=dict(
                 completed=1,
@@ -486,7 +483,7 @@ class test_role(Declarative):
         dict(
             desc='Remove privilege %r from role %r again' % (privilege1, role1),
             command=('role_remove_privilege', [role1],
-                dict(privilege=escape_comma(privilege1))
+                dict(privilege=privilege1)
             ),
             expected=dict(
                 completed=0,
diff --git a/tests/test_xmlrpc/test_selfservice_plugin.py b/tests/test_xmlrpc/test_selfservice_plugin.py
index 2ddff50e..d457627c 100644
--- a/tests/test_xmlrpc/test_selfservice_plugin.py
+++ b/tests/test_xmlrpc/test_selfservice_plugin.py
@@ -74,8 +74,8 @@ class test_selfservice(Declarative):
             desc='Create %r' % selfservice1,
             command=(
                 'selfservice_add', [selfservice1], dict(
-                     attrs=u'street,c,l,st,postalCode',
-                     permissions=u'write',
+                    attrs=[u'street', u'c', u'l', u'st', u'postalcode'],
+                    permissions=u'write',
                 )
             ),
             expected=dict(
@@ -95,8 +95,8 @@ class test_selfservice(Declarative):
             desc='Try to create duplicate %r' % selfservice1,
             command=(
                 'selfservice_add', [selfservice1], dict(
-                     attrs=u'street,c,l,st,postalCode',
-                     permissions=u'write',
+                    attrs=[u'street', u'c', u'l', u'st', u'postalcode'],
+                    permissions=u'write',
                 ),
             ),
             expected=errors.DuplicateEntry(),