324198 - Incorrect line endings with long UID loses SUMMARY information in Apple iCal

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Stefan Sitter [:ssitter]]

Bad line ends when exporting event with long UID

The testcase (extract from http://homepage.mac.com/macklausi/.calendars/GermanSpecials.ics) contains a long UID that is broken up on two lines that looks like:

UID:1027A63F-49CC-11D8-8833-00039386D1C2-D760EFD3-AB81-464D-990C-A41959F\r\n
 71F60\r\n

If you import the testcase into Sunbird and then export/publish to a new iCalendar file the UID looks like:

UID:\r\n
 1027A63F-49CC-11D8-8833-00039386D1C2-D760EFD3-AB81-464D-990C-A41959F71F60\r
\r\n
 \n

There are two line ends mixed up. I did not notice any problem when importing back into Sunbird but other clients might have a problem with this.

Tested with Sunbird/0.3a1+ (20060120) on Windows and Linux.

Comment 1

[Stefan Sitter [:ssitter]]

Attachment: testcase with long UID

Comment 2

[Reed Loden [:reed]]

The bugspam monkeys have struck again. They are currently chewing on default assignees for Calendar. Be afraid for your sanity!

Comment 3

[Matthew (lilmatt) Willis]

(In reply to comment #0)
> UID:1027A63F-49CC-11D8-8833-00039386D1C2-D760EFD3-AB81-464D-990C-A41959F\r\n
>  71F60\r\n
> 
> If you import the testcase into Sunbird and then export/publish to a new
> iCalendar file the UID looks like:
> 
> UID:\r\n
>  1027A63F-49CC-11D8-8833-00039386D1C2-D760EFD3-AB81-464D-990C-A41959F71F60\r
> \r\n
>  \n

According to this mailing list post: http://www.softwarestudio.org/mail-archives/libical/0554.html
libical added handling to try and break lines at colon, semicolon, comma, or space characters, because Outlook apparently had problems with lines like the original UID above.

It appears to be this line:
http://lxr.mozilla.org/mozilla/source/calendar/libical/src/libical/icalparameter.c#272

libical appears to break the line at the colon since it's the only "preferred" character in that line. Bad libical.

I'm not sure how we want to handle this without allegedly busting Outlook compatibility.

Comment 4

[Stefan Sitter [:ssitter]]

(In reply to comment #3)
I was more concerned about "\r\n" being replaced with "\r\r\n \n" and if this might cause troubles. If all application can handle this I'm fine to close the bug.

Comment 5

[Matthew (lilmatt) Willis]

\r\r\n\n is still bad.

I'm wondering if we should hack libical to NOT break at the first colon if this line is "UID:"

It seems really really hacky.

I wonder if the Outlook interop problem still exists in such quantity that we should keep the "break at these chars, not at column 70" hack?

Looking for input from others...

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

After some discussion and peering at RFC 2445, the real problem here is the line endings.  Exactly whether it causes dataloss is going to depend on what other clients do when they try and import it.  At this point, I'm going to assume that this is going to be problematic at least for some clients, and mark it as blocking.  

That said, nothing seems to be marked as a DUP of this, so it's not as though there's a lot of evidence to support that.  I think we need to get this off the blocker list, and the best way to do so would be by fixing it.  However, another way to do so would be to demonstrate that various widely-used clients (e.g. Outlook, iCal, gCal, yCal) are able to import it without losing data.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Given that we don't actually know that this is a problem that's going to effect real-life users, and the data itself would be recoverable if so, this won't block. If someone is able to find one or more of the clients listed that do choke on this, re-nominating for blocking would be reasonable.

Comment 9

[cmtalbert]

http://tinyurl.com/yhwge4(In reply to comment #7)
> Given that we don't actually know that this is a problem that's going to effect
> real-life users, and the data itself would be recoverable if so, this won't
> block. If someone is able to find one or more of the clients listed that do
> choke on this, re-nominating for blocking would be reasonable.
> 

This affects iCal.app (version 2.0.4) -- see bug 371133

Comment 10

[Matthew (lilmatt) Willis]

I'd very much like to see this fixed.  Since the fix for this is likely in libical and won't need string changes, I'm okay to block on this for now.

Comment 11

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: fold before adding newline

libical first adds a newline, then folds. But in this case, the folding happens in between the two characters of the newline (\r and \n). If the id would have been 1 char longer, this would not have happened.
Anyway, the obvious fix is to fold before adding the last newline.

Comment 12

[cmtalbert]

Comment on attachment 258899 [details] [diff] [review]
fold before adding newline

Taking this review from Daniel, since he's on vacation.

>Index: libical/src/libical/icalproperty.c
>===================================================================

>@@ -532,22 +535,21 @@
>+    /* We now use a function to fold the line properly every 75 characters. 
>+       That function also add the newline for us. */
Please remove the trailing space after the word "characters."


Otherwise works well, tested on mac lightning.  r=ctalbert with that nit.

Comment 13

[Matthew (lilmatt) Willis]

also make "add" plural

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in