[Freeipa-devel] [PATCH] 0015 Only split CSV strings once (updated)

[Petr Vobornik]

On 02/27/2012 02:01 PM, Petr Viktorin wrote:
> It seems I didn't communicate the problem and my solution clearly
> enough, so let me try again. (Also, I learned from the discussions!)
>
> Currently, both the client and the server parse CSV options. The client
> does *not* re-encode the CSV before sending; the parsing is really done
> twice. This means e.g. that you need 3 backslashes to escape a literal
> comma: after the client-side split, '\\\,' becomes '\,'; which after the
> server-side split becomes ','.
>
>
> Since CSV is specific to the command-line, and the client is responsible
> for translating command-line input to XML-RPC (which has its own syntax
> for lists), the ideal fix will be to move CSV processing entirely to the
> client.
> This will be a rather invasive change, mainly because some parts of the
> UI now expect the server-side parsing (but they don't escape CSV, so
> values containing commas or backslashes are broken). So it won't make it
> to the upcoming release. My patch provides a quick fix: when a call
> comes from the command-line client, disable the server-side parsing.

I investigated all occurrences of CSV creation in Web UI. I removed them 
and UI is working fine. The patch is on the list: pvoborni 099. So your 
patch shouldn't affect UI if my patch is applied.

>
> I can't get away from moving split_csv() (which is not idempotent) out
> of normalize() (which is, and gets called lots of times); this is the
> patch's major change in therms of LOC.
>
>
> I'll note again that this only affects values with backslashes or double
> quotes. Exactly these options are currently broken (=need double
> escaping). The "normal" uses of CSV are completely unaffected.
>
>
> Attaching updated patch; the change vs. the original is that the "don't
> parse again" flag is now only set at the server, when a XMLRPC call is
> received, making the client fully forward-compatible (the flag doesn't
> get sent through the wire).
>
>
> The ticket is https://fedorahosted.org/freeipa/ticket/2227, but this
> patch is only the first step in fixing it.
>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel


-- 
Petr Vobornik