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

Petr Viktorin pviktori at redhat.com
Thu Feb 23 14:29:45 UTC 2012


https://fedorahosted.org/freeipa/ticket/2227 (Unable to add certain sudo 
commands to groups). What an interesting bug to get :)


One problem with our CSV splitting is that it's not idempotent 
(baskslashes are "eaten" when there are escaped commas), but when we 
forward a call it gets done on both the client and the server. The 
attached patch fixes that in a somewhat hacky way. It's not a complete 
fix for the issue though, for that I need to ask what to do.


Some Params use the CSV format when we just need a list of 
comma-separated values. Our flavor of CSV does escaping in two different 
ways. This is pretty bad for predictability, least-surprise, 
documentation, ...

To recap, the two ways are escaping (escaped commas are ignored, 
backslashes also need to be escaped) and quoting (values are optionally 
enclosed in double quotes, to include a '"' in a quoted string, use '""').
Escaping is good because users are used to it, but currently literal 
backslashes need to be doubled, making multivalue syntax different from 
single-value syntax, even if there are no commas in the value.
Quoting is weird, but complete by itself so it doesn't also need escaping.

Do we need to use both? Is this documented somewhere? Some of our tests 
check only for one, some assume the other. Users are probably even more 
confused.


If we only keep one of those, the fix for #2227 should be quite easy.
If not (backwards compatibility), we need to document this properly, 
test all the corner cases, and fix the UI to handle CSV escaping.
Since it's subtly broken in lots of places, maybe it's best to break 
backwards compatibility and go for a simple solution now.

Anyway, if I want to do a proper fix, CSV handling should be only done 
on the client. Unfortunately turning off CSV handling in the server will 
break the UI, which at places uses `join(',')` (no escaping commas, no 
single place to change it), even though it can just send a list.




-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0015-Only-split-CSV-strings-once.patch
Type: text/x-patch
Size: 8400 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120223/5a503734/attachment.bin>


More information about the Freeipa-devel mailing list