diff --git a/src/libical/icalcomponent.c b/src/libical/icalcomponent.c index 9c2def669..d5f1d722f 100644 --- a/src/libical/icalcomponent.c +++ b/src/libical/icalcomponent.c @@ -2484,6 +2484,15 @@ static int prop_compare(void *a, void *b) return r; } +static inline int compare_nullptr(const void *a, const void *b) +{ + if (!a == !b) + return 0; + + // non-NULL sorts before NULL + return a ? -1 : 1; +} + static int comp_compare(void *a, void *b) { icalcomponent *c1 = (icalcomponent *)a; @@ -2517,17 +2526,25 @@ static int comp_compare(void *a, void *b) ICAL_TRIGGER_PROPERTY); p2 = icalcomponent_get_first_property(c2, ICAL_TRIGGER_PROPERTY); - r = strcmp(icalproperty_get_value_as_string(p1), - icalproperty_get_value_as_string(p2)); - - if (r == 0) { - p1 = icalcomponent_get_first_property(c1, - ICAL_ACTION_PROPERTY); - p2 = icalcomponent_get_first_property(c2, - ICAL_ACTION_PROPERTY); + if (p1 && p2) { r = strcmp(icalproperty_get_value_as_string(p1), icalproperty_get_value_as_string(p2)); + if (r == 0) { + p1 = icalcomponent_get_first_property(c1, + ICAL_ACTION_PROPERTY); + p2 = icalcomponent_get_first_property(c2, + ICAL_ACTION_PROPERTY); + if (p1 && p2) { + r = strcmp(icalproperty_get_value_as_string(p1), + icalproperty_get_value_as_string(p2)); + } else { + r = compare_nullptr(p1, p2); + } + } + } else { + r = compare_nullptr(p1, p2); } + break; case ICAL_VTIMEZONE_COMPONENT: @@ -2535,8 +2552,12 @@ static int comp_compare(void *a, void *b) ICAL_TZID_PROPERTY); p2 = icalcomponent_get_first_property(c2, ICAL_TZID_PROPERTY); - r = strcmp(icalproperty_get_value_as_string(p1), - icalproperty_get_value_as_string(p2)); + if (p1 && p2) { + r = strcmp(icalproperty_get_value_as_string(p1), + icalproperty_get_value_as_string(p2)); + } else { + r = compare_nullptr(p1, p2); + } break; case ICAL_XSTANDARD_COMPONENT: @@ -2545,8 +2566,13 @@ static int comp_compare(void *a, void *b) ICAL_DTSTART_PROPERTY); p2 = icalcomponent_get_first_property(c2, ICAL_DTSTART_PROPERTY); - r = strcmp(icalproperty_get_value_as_string(p1), - icalproperty_get_value_as_string(p2)); + + if (p1 && p2) { + r = strcmp(icalproperty_get_value_as_string(p1), + icalproperty_get_value_as_string(p2)); + } else { + r = compare_nullptr(p1, p2); + } break; case ICAL_VVOTER_COMPONENT: @@ -2554,8 +2580,13 @@ static int comp_compare(void *a, void *b) ICAL_VOTER_PROPERTY); p2 = icalcomponent_get_first_property(c2, ICAL_VOTER_PROPERTY); - r = strcmp(icalproperty_get_value_as_string(p1), - icalproperty_get_value_as_string(p2)); + + if (p1 && p2) { + r = strcmp(icalproperty_get_value_as_string(p1), + icalproperty_get_value_as_string(p2)); + } else { + r = compare_nullptr(p1, p2); + } break; case ICAL_XVOTE_COMPONENT: @@ -2563,8 +2594,13 @@ static int comp_compare(void *a, void *b) ICAL_POLLITEMID_PROPERTY); p2 = icalcomponent_get_first_property(c2, ICAL_POLLITEMID_PROPERTY); - r = strcmp(icalproperty_get_value_as_string(p1), - icalproperty_get_value_as_string(p2)); + + if (p1 && p2) { + r = strcmp(icalproperty_get_value_as_string(p1), + icalproperty_get_value_as_string(p2)); + } else { + r = compare_nullptr(p1, p2); + } break; default: diff --git a/src/test/regression.c b/src/test/regression.c index 246473368..16c9b0a1c 100644 --- a/src/test/regression.c +++ b/src/test/regression.c @@ -4913,6 +4913,141 @@ void test_icalcomponent_normalize(void) str_is("Normalized components match", calStr1, calStr2); } +void test_icalcomponent_normalize_missing_mandatory_props(void) +{ + // This test asserts that normalize does not crash when comparing + // components which do not have mandatory properties set. If one + // component has the property set and the other does not, then + // the component having the property sorts first. + + struct testcase { + const char *calStr; + const char *expect; + } tests[] = {{.calStr = + "BEGIN:VEVENT\r\n" + "BEGIN:VALARM\r\n" + // no ACTION + // no TRIGGER + "END:VALARM\r\n" + "BEGIN:VALARM\r\n" + "ACTION:DISPLAY\r\n" + "TRIGGER:PT0S\r\n" + "END:VALARM\r\n" + "END:VEVENT\r\n", + .expect = + "BEGIN:VEVENT\r\n" + "BEGIN:VALARM\r\n" + "ACTION:DISPLAY\r\n" + "TRIGGER:PT0S\r\n" + "END:VALARM\r\n" + "BEGIN:VALARM\r\n" + // no ACTION + // no TRIGGER + "END:VALARM\r\n" + "END:VEVENT\r\n"}, + {.calStr = + "BEGIN:VEVENT\r\n" + "BEGIN:VALARM\r\n" + // no ACTION + "TRIGGER:PT0S\r\n" + "END:VALARM\r\n" + "BEGIN:VALARM\r\n" + "ACTION:DISPLAY\r\n" + "TRIGGER:PT0S\r\n" + "END:VALARM\r\n" + "END:VEVENT\r\n", + .expect = + "BEGIN:VEVENT\r\n" + "BEGIN:VALARM\r\n" + "ACTION:DISPLAY\r\n" + "TRIGGER:PT0S\r\n" + "END:VALARM\r\n" + "BEGIN:VALARM\r\n" + // no ACTION + "TRIGGER:PT0S\r\n" + "END:VALARM\r\n" + "END:VEVENT\r\n"}, + { + .calStr = + "BEGIN:VCALENDAR\r\n" + "BEGIN:VTIMEZONE\r\n" + // no TZID + "BEGIN:STANDARD\r\n" + "DTSTART:16010101T000000\r\n" + "TZOFFSETFROM:-0200\r\n" + "TZOFFSETTO:-0200\r\n" + "END:STANDARD\r\n" + "END:VTIMEZONE\r\n" + "BEGIN:VTIMEZONE\r\n" + "TZID:/test\r\n" + "BEGIN:STANDARD\r\n" + "DTSTART:16010101T000000\r\n" + "TZOFFSETFROM:-0200\r\n" + "TZOFFSETTO:-0200\r\n" + "END:STANDARD\r\n" + "END:VTIMEZONE\r\n" + "END:VCALENDAR\r\n", + .expect = + "BEGIN:VCALENDAR\r\n" + "BEGIN:VTIMEZONE\r\n" + "TZID:/test\r\n" + "BEGIN:STANDARD\r\n" + "DTSTART:16010101T000000\r\n" + "TZOFFSETFROM:-0200\r\n" + "TZOFFSETTO:-0200\r\n" + "END:STANDARD\r\n" + "END:VTIMEZONE\r\n" + "BEGIN:VTIMEZONE\r\n" + // no TZID + "BEGIN:STANDARD\r\n" + "DTSTART:16010101T000000\r\n" + "TZOFFSETFROM:-0200\r\n" + "TZOFFSETTO:-0200\r\n" + "END:STANDARD\r\n" + "END:VTIMEZONE\r\n" + "END:VCALENDAR\r\n", + }, + {.calStr = + "BEGIN:VTIMEZONE\r\n" + "BEGIN:STANDARD\r\n" + // no DTSTART + "TZOFFSETFROM:-0200\r\n" + "TZOFFSETTO:-0200\r\n" + "END:STANDARD\r\n" + "BEGIN:STANDARD\r\n" + "DTSTART:16010101T000000\r\n" + "TZOFFSETFROM:-0200\r\n" + "TZOFFSETTO:-0200\r\n" + "END:STANDARD\r\n" + "END:VTIMEZONE\r\n", + .expect = + "BEGIN:VTIMEZONE\r\n" + "BEGIN:STANDARD\r\n" + "DTSTART:16010101T000000\r\n" + "TZOFFSETFROM:-0200\r\n" + "TZOFFSETTO:-0200\r\n" + "END:STANDARD\r\n" + "BEGIN:STANDARD\r\n" + // no DTSTART + "TZOFFSETFROM:-0200\r\n" + "TZOFFSETTO:-0200\r\n" + "END:STANDARD\r\n" + "END:VTIMEZONE\r\n"}, + {NULL, NULL}}; + + for (const struct testcase *tc = tests; tc->calStr; tc++) { + icalcomponent *ical = icalcomponent_new_from_string(tc->calStr); + icalcomponent_normalize(ical); + const char *s = icalcomponent_as_ical_string(ical); + icalcomponent_free(ical); + + if (VERBOSE) { + printf("iCal:\n%s\n\n", s); + } + str_is("Normalized component matches", s, tc->expect); + } +} + static void test_builtin_compat_tzid(void) { struct _cases { @@ -5592,6 +5727,7 @@ int main(int argc, char *argv[]) test_run("Test icalarray_sort", test_icalarray_sort, do_test, do_header); test_run("Test icalcomponent_normalize", test_icalcomponent_normalize, do_test, do_header); + test_run("Test icalcomponent_normalize_missing_mandatory_props", test_icalcomponent_normalize_missing_mandatory_props, do_test, do_header); test_run("Test builtin compat TZID", test_builtin_compat_tzid, do_test, do_header); test_run("Test VCC vCard parse", test_vcc_vcard_parse, do_test, do_header); test_run("Test implicit DTEND and DURATION for VEVENT and VTODO", test_implicit_dtend_duration, do_test, do_header);