Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-existing or incorrectly formatted Tutanota attendee causes errors upon sending event updates and deletion #2975

Closed
jowlo opened this issue Apr 23, 2021 · 5 comments · Fixed by #2983
Assignees
Labels
bug broken functionality, usability problems, unexpected errors state:tested We tested it and are about to release it
Milestone

Comments

@jowlo
Copy link
Contributor

jowlo commented Apr 23, 2021

Bug in web app

Describe the bug
When importing a calendar with an event that has an attendee with non-existing @tutanota.com address on it, import works but there is an error when looking at the details/edit view of that event.

To Reproduce
Steps to reproduce the behavior:

  1. Import calendar with event with an non-existant tutanota attendee, for example:
BEGIN:VCALENDAR
PRODID:-//Tutao GmbH//Tutanota 3.83.0//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:PUBLISH
BEGIN:VEVENT
DTSTART:20210423T080000Z
DTEND:20210423T083000Z
DTSTAMP:20210423T105406Z
UID:asdfasdfasdf@tutanota.com
SEQUENCE:2
SUMMARY:testevent
ORGANIZER;EMAIL=test@tutanota.com:mailto:test@tutanota.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;RSVP=TRUE;EMAIL=test@tutanota.com:mailto:test@tutanota.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN="A girl is noone";EMAIL=someonewhodoesnotexist@tutanota.de:mailto:someonewhodoesnotexist@tutanota.de:
END:VEVENT
END:VCALENDAR
  1. Import into calendar
  2. Open edit view of the new event
  3. See error

Expected behavior
No error shown

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
See #2974 for the same error when creating a new event

@jowlo jowlo added the bug broken functionality, usability problems, unexpected errors label Apr 23, 2021
@vaf-hub vaf-hub self-assigned this Apr 26, 2021
@vaf-hub
Copy link
Contributor

vaf-hub commented Apr 26, 2021

The issue with the event you provided as an example is the trailing colon, not the fact that the Tutanota address does not exist:
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN="A girl is noone";EMAIL=someonewhodoesnotexist@tutanota.de:mailto:someonewhodoesnotexist@tutanota.de:

I think this is invalid format. We could handle it when importing the event if there are actual users that try to import such events?
In such a case we can modify parseCalendarEvents() to check that what parseMailtoValue() returned is a valid mail address. However, I have concerns about throwing an error and not parsing the event at all. Maybe we could just discard the attendee or organizer whose mail address is in invalid format. I am not sure about the side effects that might have, though.

If you don't think we should handle the invalid format I would propose to close this as wontfix.

@jowlo
Copy link
Contributor Author

jowlo commented Apr 26, 2021

Actually, its an export from our own calendar, here is the original file without changing anything:

BEGIN:VCALENDAR
PRODID:-//Tutao GmbH//Tutanota 3.83.0//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:PUBLISH
BEGIN:VEVENT
DTSTART:20210423T080000Z
DTEND:20210423T083000Z
DTSTAMP:20210423T105406Z
UID:MXBepxk----01asdf00asdf95@tutanota.com
SEQUENCE:2
SUMMARY:testevent4reimport
ORGANIZER;EMAIL=paypaltest@tutanota.com:mailto:paypaltest@tutanota.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;RSVP=TRUE;EMAIL=paypaltest@tutanota.com:mailto:paypaltest@tutanota.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN="Bedfortest, but with, comma";EMAIL=someonewhodoesnotexist@tutanota.de:mailto:someonewhodoesnotexist@tutanota.de:
END:VEVENT
END:VCALENDAR

It is however a reimport where i changed stuff in the ics file earlier. I guess we shouldn't import it in the first place, catch that error somehow or sanitize the email adresses when importing or exporting.

I don't think won't fix is an option because it does throw an application error after using a normal workflow which imho should never happen. There are a gazillion sources of ics events, pretty sure someone adds malformed mail adresses.

@vaf-hub
Copy link
Contributor

vaf-hub commented Apr 27, 2021

I tried to export and reimport calendars to create an ics files like the one you provided, but there is no trailing colon for the mailto value. So, I am not sure how to reproduce this.

However, I investigated further. There are actually two different errors:

  1. If there is a trailing colon in the imported attendee's mail address, upon opening the edit event dialog we receive BadRequestError due to the invalid mail address.

  2. If the attendee's mail address is a valid Tutanota mail address (without a trailing colon) but does not exist, we do not immediately get an error upon opening the edit event dialog (as the initial report suggests). We do get an error, though, if we try to delete the event or modify it and try to send out updates to the attendees. So this only happens if we are allowed to send out updates (by being the organizer).

I think we should fix part 2, by allowing the deletion of events with non-existing addresses without showing an error dialog. Regarding updates for modifications of the event I think we should show a confirmation dialog and ask to ignore the non-existing address but send updates to all existing addresses.

@jowlo
Copy link
Contributor Author

jowlo commented Apr 27, 2021

I tried to export and reimport calendars to create an ics files like the one you provided, but there is no trailing colon for the mailto value. So, I am not sure how to reproduce this.

You would have to import a faulty event like one of those i provided first and then export it again. I just meant that we also export it with a colon at the end and do not remove it, so we would need to sanitize at some point.

From the rest of your post it sounds like you were able to reproduce fine though. It think we should definitely fix both errors:

  1. If it's not a correctly formatted mail address we shouldn't even do a request to the server. I guess we can import it like we do now but we shouldn't ever expect any calendar event attendee mails to be valid then. Or we deal with this during import (which would be consistent with the fact that we check addresses during creation of a calendar event).
  2. We could somehow show the user that the attendee has an invalid mail-address format, maybe with a 'X' icon where now we show the circle or "checkmark in circle" to signal attendee status.

@vaf-hub vaf-hub changed the title Non-existing Tutanota attendee of event errors in details view Non-existing or incorrectly formatted Tutanota attendee causes errors upon sending event updates and deletion Apr 27, 2021
@vaf-hub vaf-hub added this to the Next release milestone Apr 27, 2021
@vaf-hub
Copy link
Contributor

vaf-hub commented Apr 27, 2021

Test notes:

  • when importing calendars invalid attendee and organizer mail addresses (like the one above with a trailing colon) are ignored and the event is created without them

create an event with both existing and non-existing valid tutanota mail addresses (by importing a modified ics file)

  • verify deletion of the event workls and check that all valid mail addresses do receive an update with the cancellation
  • verify that removing all of the invalid addresses and sending updates from the calendar event edit dialog is possible
  • verify that modifying the event without removing invalid addresses and sending updates shows an error dialog that mentions the invalid addresses

@johnbotris johnbotris modified the milestones: Next release, 3.83.4 Apr 30, 2021
@johnbotris johnbotris self-assigned this May 12, 2021
@johnbotris johnbotris added the state:tested We tested it and are about to release it label May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug broken functionality, usability problems, unexpected errors state:tested We tested it and are about to release it
Projects
None yet
3 participants