Petr Viktorin wrote:
This depends on my patch 0015.
Since CSV escaping was entirely broken before that patch (however we
decide to fix the problem), let's also fix the escaping syntax itself,
without worrying about backwards compatibility.

I tried to solve this according to Rob's comment on
https://fedorahosted.org/freeipa/ticket/2227:
 > Whatever we come up with we need a more consistent way to handle
 > escape characters so it works the same on single and multi-value
 > attributes.

My goal is that it will just do the right thing for 99.9% of uses, and
the 0.1% (or people who want to write robust scripts┬╣) should find a
short, clear paragraph in the documentation.

Of course, values without backslashes, double quotes and newlines are
again completely unaffected.



Commit message:

Instead of the non-obvious semantics of Python's CSV module, just split
on commas that aren't escaped by a backslash.
Then, replace any commas that *are* escaped by backslashes by just
commas, and remove initial whitespace as needed.


This simpler approach fits our use case better: most importantly, it
doesn't require literal backslashes to be escaped, and doesn't "eat"
backslashes (unless they precede a comma). This means the CSV multivalue
format is, except for commas, the same as for single values.
It also does away with CSV's double-quoting syntax, making the format
more predictable (=scriptable).

A drawback is that this format doesn't allow values that end with a
backslash, unless they're given at the end. The workaround is to either
give them at the end, or if more are needed,just use multiple options:
--someopt='val1,val2\' --someopt='val3\'

Values that end with backslashes are so rare, this shouldn't be a
problem in practice.


Also, update the tests.


Fixed as a side effect:
https://fedorahosted.org/freeipa/ticket/2402
https://fedorahosted.org/freeipa/ticket/2417


I think I'd prefer to see this rebased into patch 15 so we have one cohesive thing to review.

I'm not sure we can arbitrarily say that strings ending with \ are rare. That is very close to "who will need more than 640k of RAM?"

It doesn't look this handle someone purposely wanting an escaped comma in the string?

The reason we do the parsing on both sides is to save a round trip in case of errors. If we do it only one one side then it must be done only on the server side. We can't assume well-formed data coming from any client.

rob

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to