343792 - Freeze (hang) after import of .ics file which has INTERVAL=0 in RRULE

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Chuck Tuffli]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla Thunderbird version 3 alpha 1 (20060706) with Lightning 0.1+ (build 2006070507)

Exported an Outlook calendar to an .ics file with Outport (http://outport.sourceforge.net) and tried to import the resulting file. Lightning  started the import but became unresponsive (100% CPU utilization) before the import completed. Killing Thunderbird and restarting caused the application to again go into an unresponsive state and using nearly 100% of the CPU cycles. The only recovery is to delete the profile and restart Thunderbird.

Narrowed down  the problem to a single line:
RRULE:FREQ=WEEKLY;INTERVAL=0

Changing the INTERVAL value to 1 causes the import to suceed. The RFC specifies "positive integer" values, so it seems like Outport is outputing an incorrect RRULE, but it would be great if Lightning didn't hang.

Reproducible: Always

Steps to Reproduce:
1. Calendar > Import ...
2. Select the .ics file and click Open
Actual Results:  
Application hangs and the CPU utilization approaches 100%

Expected Results:  
Import the file and/or flag the error

Comment 1

[Chuck Tuffli]

Attachment: iCalendar file causing the hang

This is a sanatized subset of the original file that causes Lightning to hang.

Comment 2

[Matthew Rasnake]

I have been having a similar issue, but in subscribing to a remote calendar. Unfortunately, my import problems went beyond the simple INTERVAL=0 issue, and I found that, in fact, semicolons were the problem, not the INTERVAL=0...

By removing all semicolons from the RRULE: line, the import of my test file succeded. Alternately, one of the failing lines from my test file imported successfully when I placed a semicolon at the end of the line (when other semi's were already present). Oddly, some RRULE's imported just fine with semicolons in the line, but without one on the end.

Also, I found that the single instance of INTERVAL=0 in my file imported correctly with the semicolons removed.

So, effectively, the RRULE parser just needs to be improved a bit to not hang on the presence or absence semicolons.

I'm running:
Thunderbird version 2 beta 1 (20061013)
Lightning 0.3

Comment 3

[Joey Minta]

(In reply to comment #2)
Adding/removing semicolons changes the meaning of the RRULE in significant and unexpected ways.  Doing so is not considered a good solution to the problem.

Comment 4

[Matthew Rasnake]

bottom line is that Lightning shouldn't hang thunderbird in the event of a malformed file.

Comment 5

[Omar Bajraszewski]

blocking 0.5?

Comment 6

[Stefan Sitter [:ssitter]]

Sunbird is also affected. Updating severity to critical because the only way to get rid of the problem is by removing entire storage.sdb file -> dataloss.

Comment 7

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

Attachment: fool-proof libical

This patch makes libical more foolproof, by ignoring interval=0.

Comment 8

[Daniel Boelzle [:dbo]]

Comment on attachment 247727 [details] [diff] [review]
fool-proof libical

>+DEFINES += -DHAVE_CONFIG_H -DDEBUG
>+CFLAGS += -g

>+#define ICAL_ERRORS_ARE_FATAL  1

leftovers from your debugging session?
(If not, I don't think we should compile with debug or abort the app on ical errors.)

>-	    parser.rt.interval = (short)atoi(value);
>+	    short v = atoi(value);
missing cast from int to short here, avoiding a compiler warning.

>+	    if (v) {
>+		parser.rt.interval = (short)atoi(value);
>+	    }
better just
		parser.rt.interval = v;

>-    icaltimezone_parse_zone_tab ();
>+//    icaltimezone_parse_zone_tab ();
Why this?

Comment 9

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

Attachment: patch v2

Patch now without the leftover stuff in other files. Sorry for the mess.
Also made the check a little bit more strict: also ignore negative values. Just to be sure.

Comment 10

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

patch checked in

Comment 11

[Stefan Sitter [:ssitter]]

No freeze during import using Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.1) Gecko/20061215 Thunderbird/2.0b1 and Lightning/0.4a1 (2006121603)

No freeze during import using Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20061217 Calendar/0.4a1

-> VERIFIED

Comment 12

[Stefan Sitter [:ssitter]]

If possible there should be a regression test case in Litmus for this issue.

Comment 13

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

imo, there should be a unit-test instead. This can perfectly be tested automatically. No need for manual litmus labour.

Comment 14

[Stefan Sitter [:ssitter]]

Attachment: xpcshell testcase

Comment 15

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 374584 [details] [diff] [review]
xpcshell testcase

Looks good, r=philipp

Comment 16

[Stefan Sitter [:ssitter]]

Comment on attachment 374584 [details] [diff] [review]
xpcshell testcase

Pushed to comm-central: <https://hg.mozilla.org/comm-central/rev/9149aacc9a0b>

Comment 17

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Seems this is not in libical 0.46 yet, we should make sure this goes upstream