[Freeipa-devel] [PATCH] 0018 - Simplify CSV escaping syntax

[Rob Crittenden]

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