[Freeassociation-devel] [FA-commits] SF.net SVN: freeassociation:[989] trunk/libical/src/test/regression.c

[Arnout Engelen]

The regression test now reads:

   is("test encoding of 'Timezone\\nwith a newline'",
     icalproperty_as_ical_string(prop), "DTSTART;VALUE=DATE,TZID=Timezone\\nwith a newline:20090126");

I do not think this is what should be expected: the newline seems to be 
'escaped', but the spec only specifies 'escaping' for 'text' values, not for
'paramtext' values. Because of that, in a paramtext, '\n' should be 
interpreted as simply 'a backslash and the letter n', not 'an escaped newline'.

As the only whitespace allowed by the specs are spaces and tabs, it would be
reasonable to replace the newline with a space:

   is("test encoding of 'Timezone\\nwith a newline'",
     icalproperty_as_ical_string(prop), "DTSTART;VALUE=DATE,TZID=Timezone with a newline:20090126");

On Fri, Jan 08, 2010 at 12:46:46AM +0100, Wilfried Goesgens wrote:
> icalmemory_append_string() is used to append that, which basicaly uses
> strcpy, so no escaping in there.  
> 
> icalparameter_as_ical_string_r() builds the surrounding string, its called
> recursively.  

Well, right now icalparameter_as_ical_string_r uses icalmemory_append_string,
which *also* basically uses strcpy. There is no escaping at all here, 
currently.

It would be reasonable for libical to try its best to make sure its output is 
filtered to contain only valid characters. It would be desirable to perform 
any escaping/filtering from icalproperty_as_ical_string.

Attached is a patch that does this - it replaces any invalid characters with
spaces. Seems like an improvement to me.


Kind regards,

Arnout
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libical-filterparametervalue.patch
Type: text/x-diff
Size: 3837 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libical-devel/attachments/20100110/d32f6328/attachment.bin>