Skip to content

fix(calendar): skip malformed ical events #1870

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.

Sign up for GitHub

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

Open
keilogic wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(calendar): skip malformed ical events #1870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
030c545
test(calendar): cover malformed ical event loading
keilogic Jun 10, 2026
0a41dc5
fix(calendar): skip malformed ical events
keilogic Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/app/calendar-app/calendar.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { StorageService } from '../storage.service';
import { RunboxWebmailAPI } from '../rmmapi/rbwebmail';
import { CalendarService } from './calendar.service';
import { RunboxCalendarEvent } from './runbox-calendar-event';
import { HttpErrorResponse } from '@angular/common/http';
import { of } from 'rxjs';
import { take } from 'rxjs/operators';
import moment from 'moment';
Expand Down Expand Up @@ -226,6 +227,25 @@ END:VCALENDAR
}));
});

it('should skip malformed calendar events while loading valid ones', () => {
dav_events['test/broken'] = {
calendar: 'test',
id: 'test/broken',
ical: 'BEGIN:VCALENDAR\nBEGIN:VEVENT\nDESCRIPTION;ALTREP\nDTSTART:20210425T090000\nDTEND:20210425T100000\nSUMMARY:Broken\nEND:VEVENT\nEND:VCALENDAR\n'

@castaway castaway Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put the "broken" ical value not all on one long line (newlines are fine), and document what exactly is "broken" in the test data?

@castaway castaway Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @keilogic could you respond to the requested changes?

};

const errors: HttpErrorResponse[] = [];
sut.errorLog.subscribe(e => errors.push(e));

sut.reloadEvents();

expect(sut.icalevents.length).toBe(2, '2 valid ical events were kept');
expect(sut.events.length).toBe(6, 'valid events were still generated');
expect(errors.length).toBe(1, 'one calendar event error was reported');
expect(errors[0].error.error).toContain('test/broken');
expect(errors[0].error.error).toContain('Invalid parameters');
});

it('should be possible to import an .ics file', () => {
sut.importFromIcal(undefined,
`BEGIN:VCALENDAR
Expand Down
118 changes: 72 additions & 46 deletions src/app/calendar-app/calendar.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ export class CalendarService implements OnDestroy {
this.syncTokens[cal.id] = cal.syncToken;
}
const runboxevents = cache['events'].map((e: any) => {
const ievent = this.importFromIcal(e.id, e.ical);
return this.generateEvents(undefined, undefined, [ievent]);
const ievent = this.importFromIcalSafely(e.id, e.ical);
return ievent ? this.generateEvents(undefined, undefined, [ievent]) : [];
});
runboxevents.forEach((events) => this.events = this.events.concat(events));
this.calendarSubject.next(this.calendars);
Expand Down Expand Up @@ -182,6 +182,16 @@ export class CalendarService implements OnDestroy {
this.errorLog.next(e);
}

calendarEventErrorHandler(id: string, e: unknown): void {
const message = e instanceof Error ? e.message : String(e);
const eventId = id ? ` ${id}` : '';
this.apiErrorHandler(new HttpErrorResponse({
error: { error: `Could not load calendar event${eventId}: ${message}` },
status: 0,
statusText: 'Calendar event error',
}));
}

deleteCalendar(id: string) {
this.activities.begin(Activity.DeletingCalendar);
this.rmmapi.deleteCalendar(id).subscribe(() => {
Expand Down Expand Up @@ -351,6 +361,15 @@ export class CalendarService implements OnDestroy {
return { 'id': id, 'event': vevents[0] };
}

importFromIcalSafely(id: string, ical: string, keep = true): {id: string, event: ICAL.Event} | undefined {
try {
return this.importFromIcal(id, ical, keep);
} catch (e) {
this.calendarEventErrorHandler(id, e);
return undefined;
}
}

// generate RBE events from ICAL.Events (inc exceptions)
// default to "now", plus 1mo, and "events stored by fromIcal previously
// iterator will quite happily generate events before the ICAL startdate
Expand All @@ -370,55 +389,62 @@ export class CalendarService implements OnDestroy {
// ICAL.RecurExpansion iterator
const events: RunboxCalendarEvent[] = [];
event_list.forEach((ievent) => {
if (!ievent) {
return;
}
const id = ievent['id'];
let icalevent = ievent['event'];

if (!icalevent.isRecurring()) {
if (!startDate ||
(moment(startDate).isSameOrBefore(icalevent.endDate.toJSDate())
&& moment(endDate).isSameOrAfter(icalevent.startDate.toJSDate()) )) {
events.push(new RunboxCalendarEvent(id, icalevent, icalevent.startDate, icalevent.endDate, this.me.timezone));
try {
if (!icalevent.isRecurring()) {
if (!startDate ||
(moment(startDate).isSameOrBefore(icalevent.endDate.toJSDate())
&& moment(endDate).isSameOrAfter(icalevent.startDate.toJSDate()) )) {
events.push(new RunboxCalendarEvent(id, icalevent, icalevent.startDate, icalevent.endDate, this.me.timezone));
}
return;
}
// start defaults to event dtstart if undefined
// don't start before the event itself starts
// (after end is checked below)
const start_date_ICAL = startDate && moment(startDate).isSameOrAfter(icalevent.startDate.toJSDate())
? ICAL.Time.fromJSDate(startDate) : undefined;

// in case we just got passed a generic date:
if (start_date_ICAL) {
start_date_ICAL.isDate = icalevent.startDate.isDate;
}
return;
}
// start defaults to event dtstart if undefined
// don't start before the event itself starts
// (after end is checked below)
const start_date_ICAL = startDate && moment(startDate).isSameOrAfter(icalevent.startDate.toJSDate())
? ICAL.Time.fromJSDate(startDate) : undefined;

// in case we just got passed a generic date:
if (start_date_ICAL) {
start_date_ICAL.isDate = icalevent.startDate.isDate;
}

// improve RRULEs if underspecified (then save!?)
icalevent = this.fixICALEvent(icalevent);

// Now recheck the start_date_ICAL against the known
// start/end/until dates
// And actually generate some events!
let next_time;
let iterator = icalevent.iterator(start_date_ICAL);
if (iterator.ruleIterators[0].rule.isByCount()
&& icalevent.component.parent.hasProperty('X-RUNBOX-CALCULATED-DTEND')) {
const calc_end_date = ICAL.Time.fromString(icalevent.component.parent
.getFirstPropertyValue('X-RUNBOX-CALCULATED-DTEND')).toJSDate();
if (startDate && moment(startDate).isAfter(calc_end_date)) {
// console.log('Skipping generation as already ended');
return;
// improve RRULEs if underspecified (then save!?)
icalevent = this.fixICALEvent(icalevent);

// Now recheck the start_date_ICAL against the known
// start/end/until dates
// And actually generate some events!
let next_time;
let iterator = icalevent.iterator(start_date_ICAL);
if (iterator.ruleIterators[0].rule.isByCount()
&& icalevent.component.parent.hasProperty('X-RUNBOX-CALCULATED-DTEND')) {
const calc_end_date = ICAL.Time.fromString(icalevent.component.parent
.getFirstPropertyValue('X-RUNBOX-CALCULATED-DTEND')).toJSDate();
if (startDate && moment(startDate).isAfter(calc_end_date)) {
// console.log('Skipping generation as already ended');
return;
}
// on COUNT, generate all
iterator = icalevent.iterator();
} else if (!end_date_Moment) {
end_date_Moment = moment().startOf('month').add(2, 'month');
}
// on COUNT, generate all
iterator = icalevent.iterator();
} else if (!end_date_Moment) {
end_date_Moment = moment().startOf('month').add(2, 'month');
}
// keep going until we pass the end date:
while ( ( next_time = iterator.next() )
&& (end_date_Moment ? end_date_Moment.isAfter(next_time.toJSDate()) : true) ) {
const details = icalevent.getOccurrenceDetails(next_time);
events.push(
new RunboxCalendarEvent(id, details.item, details.startDate, details.endDate, this.me.timezone));
// keep going until we pass the end date:
while ( ( next_time = iterator.next() )
&& (end_date_Moment ? end_date_Moment.isAfter(next_time.toJSDate()) : true) ) {
const details = icalevent.getOccurrenceDetails(next_time);
events.push(
new RunboxCalendarEvent(id, details.item, details.startDate, details.endDate, this.me.timezone));
}
} catch (e) {
this.calendarEventErrorHandler(id, e);
}
});
return events;
Expand Down Expand Up @@ -505,7 +531,7 @@ export class CalendarService implements OnDestroy {
this.icalevents = [];
events.forEach((e: any) => {
// store events into this.icalevents
this.importFromIcal(e.id, e.ical);
this.importFromIcalSafely(e.id, e.ical);
});

// generate RBE events for just this set, for current month+1:
Expand Down
Loading