1394524 - Mail with ICS attachment which contains invalid characters and causes "[calIICSService.parseICS] calIcsParser.js:145" stays always "Unread" unless marked "as read" manually

Status: RESOLVED FIXED

(Calendar :: E-mail based Scheduling (iTIP/iMIP), defect)

Description

[Martin Mai]

Attachment: Appointment Confirmation.eml

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3100.0 Iron Safari/537.36

Steps to reproduce:

Open the attached Mail and send it to yourself.


Actual results:

The mail list entry stays bold after read.


Expected results:

After reading, the mail should change status from "unread" to "read".

Comment 1

[Jorg K (CEST = GMT+2)]

I edited the message "as new" and sent it to myself. Upon receipt, it looks unread and opening it or displaying it in the preview pane, doesn't make it read. However, marking it as read works, but when marking it at unread, the same original effect occurs. In fact, you don't need to go though sending the message, just import (drag into a folder or the Trash) and mark it Unread.

I've changed the MIME header of the text/calendar attachment to text/plain and the problem goes away.

The problem is that Calendar/Lightning runs into an error which is logged on the console:
Component returned failure code: 0x804a0100 [calIICSService.parseICS] calIcsParser.js:145

So basically the ICS file is invalid and the system chokes on it and doesn't mark the message read. The decoded content of the ICS file is:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//ince/events//NONSGML v1.0//EN
BEGIN:VEVENT
DTSTART:20170828T060000Z
DTEND:20170828T061500Z
DESCRIPTION:Tesla Service Appointment Confirmation
SUMMARY:Tesla Service Appointment Confirmation
LOCATION:Nuremberg-Thumenberger
DTSTAMP:20170808T142343Z
SEQUENCE:1
UID:1592763
END:VEVENT
BEGIN:VALARM
TRIGGER:-PT30M
REPEAT:2
DURATION:PT15M
ACTION:DISPLAY
END:VALARM
END:VCALENDAR


So there is an encoding problem and there are invalid characters in the file. That throws the ICS decoder which doesn't appear to handle the case gracefully and the message doesn't get marked as read when reading it.

It's a bug but not the most severe one we have on file ;-)

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: Appointment Confirmation.eml - Repaired: invalid characters removed, all working

Comment 3

[[:MakeMyDay]]

In what version of Lightning do you use? Do you calendar.icaljs enabled in the advanced preferences? Does this issue also occur for you, if you flip this pref (requires restart of TB afterwards)?

Comment 4

[Martin Mai]

I was using Thunderbird 52.3.0 (32-Bit)

Today there was an update to 52.4.0
There is Lightning 5.4.4 included.
The reported bug is still here.

My setting is standard:
Name            / Status   / Type    / Value
calendar.icaljs / Standard / boolean / false

After set to true and restart of TB the reported bug is still here.
Now I'm switching back to false.

Comment 5

[[:MakeMyDay]]

Attachment: FailGracefullyOnIcaljsParsing-V1.diff

This patch adds the capability to catch errors in the ics parser to deal with the issue in the first place.

Additionally, ical.js and libical are dealing differently with invalid ics. Libical is more gracefully atm and wrapps invalid properties/params in an X-LIC-ERROR property/param. I added some code to get icaljs to do the same - before I go further and make this ready for an upstream pr including tests, I would like to get some feedback on whether this is the road you would like to take for that. Or would we need a config option to make this behaviour optionally to not break other users of ical.js relying on the current behaviour?

Comment 6

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

Comment on attachment 8921601 [details] [diff] [review]
FailGracefullyOnIcaljsParsing-V1.diff

Honestly, I think X-LIC-ERROR should DIAF. I'm not sure what the best alternative is, either just ignoring the line completely or erroring out and catching the error somewhere. If it is on the parameter level then of course the same for just the parameter.

Comment 7

[[:MakeMyDay]]

Attachment: FailGracefullyOnIcsParsing-V1.diff

Fien for me. However, we should implement a more gracefully error handling in icaljs compared to its current behaviour of throwing - from a user's perspective, this looks like a shortcoming of icaljs, because libical is able to parsse also invalid ics so these can be displayed.

But I deferr this to bug 1411530 and go here with just catching and logging errors in the ics parser. As long as X-LIC exists in our code base, I still would like to make use of it for logging purposses here.

Comment 8

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

Comment on attachment 8921816 [details] [diff] [review]
FailGracefullyOnIcsParsing-V1.diff

Review of attachment 8921816 [details] [diff] [review]:
-----------------------------------------------------------------

The ical.js changes need to go upstream before pushing here. Please send a PR for this change there.

Comment 9

[[:MakeMyDay]]

Attachment: FailGracefullyOnIcsParsing-V2.diff

Huh, the patch was only cleaned up half way before upload. Obviously, I didn't remove the ical.js changes, which was intended due to comment 6.

This version just removes these useless changes and retains the code catching the exceptions, so I'm carrying forward the r+.

Philipp, if you have changed your mind regarding adding X-LIC props to ical.js, please leave a comment or file a follow-up bug to do so.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8c2e2bb95746
Fail gracefully when parsing invalid ics data. r=philipp

Comment 11

[Martin Mai]

Now I'm using Thunderbird 52.9.1 (32-Bit).
I see the issue again: Got an email with attached "Appointment.ics" and after selecting that mail it stays bold.

After reading, the mail should change status from "unread" to "read".
If needed, I can upload/forward that mail with attachment to someone.

Comment 12

[Martin Mai]

Attachment: Bestätigung eines Tesla Service-Termins.eml

Example to explain bug in Thunderbird 52.9.1 (32-Bit)

Comment 13

[Stefan Sitter [:ssitter]]

This bug is fixed in Lightning 6.1 and newer. You are using Lightning 5.4 and therefore you will still see the problem until new major release is done or unless you switch to Beta channel.

Comment 14

[Martin Mai]

Thank You very much.

Comment 15

[[:MakeMyDay]]

There is no point to reopen this if you see the issue in a not fixed version, so setting this back to fixed. ESR650 is up to be releast soon. If you're experiencing the issue in that version, too, feel free to file a new bug.

Comment 16

[Jorg K (CEST = GMT+2)]

That's TB 60 ESR. You can get a preview here:
https://archive.mozilla.org/pub/thunderbird/candidates/60.0-candidates/build4/
Remove your installed Calendar, reset pref extensions.installedDistroAddon.{e2fda1a4-762b-4020-b5ad-a41df1933103} and restart TB.