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

[Petr Viktorin]

On 02/29/2012 07:13 PM, Petr Vobornik wrote:
> 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
>
>

The webUI patch is in, and also I heard this patch is not making it to 
the release anyway, so the workaround makes little sense. I'd like to go 
for the real fix.

Meanwhile I found some other bugs 
(https://fedorahosted.org/freeipa/ticket/2482, 
https://fedorahosted.org/freeipa/ticket/2483) that prevent me from 
testing this throroughly.

Self-NACK for now.

-- 
Petr³