icalcomponent.c, icalproperty.c: Rewrite of normalization code to (hopefully) fix crashes and leaks

2 files changed, 46 insertions, 48 deletions

diff --git a/src/libical/icalcomponent.c b/src/libical/icalcomponent.c
index 70c1e342..c16bb6dc 100644
--- a/src/libical/icalcomponent.c
+++ b/src/libical/icalcomponent.c
@@ -2650,55 +2650,52 @@ void icalcomponent_normalize(icalcomponent *comp)
     pvl_list sorted_props = pvl_newlist();
     pvl_list sorted_comps = pvl_newlist();
     icalproperty *prop;
-    icalcomponent *mycomp;
+    icalcomponent *sub;
 
+    /* Normalize properties into sorted list */
     while ((prop = pvl_pop(comp->properties)) != 0) {
-        int nparams = icalproperty_count_parameters(prop);
+        int nparams, remove = 0;
 
-        icalproperty_set_parent(prop, 0);
+        icalproperty_normalize(prop);
+
+        nparams = icalproperty_count_parameters(prop);
 
-        /* Skip unparameterized properties having default values */
+        /* Remove unparameterized properties having default values */
         if (nparams == 0) {
             switch (icalproperty_isa(prop)) {
             case ICAL_CALSCALE_PROPERTY:
                 if (strcmp("GREGORIAN", icalproperty_get_calscale(prop)) == 0) {
-                    icalproperty_free(prop);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
             case ICAL_CLASS_PROPERTY:
                 if (icalproperty_get_class(prop) == ICAL_CLASS_PUBLIC) {
-                    icalproperty_free(prop);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
             case ICAL_PRIORITY_PROPERTY:
                 if (icalproperty_get_priority(prop) == 0) {
-                    icalproperty_free(prop);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
             case ICAL_TRANSP_PROPERTY:
                 if (icalproperty_get_transp(prop) == ICAL_TRANSP_OPAQUE) {
-                    icalproperty_free(prop);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
             case ICAL_REPEAT_PROPERTY:
                 if (icalproperty_get_repeat(prop) == 0) {
-                    icalproperty_free(prop);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
             case ICAL_SEQUENCE_PROPERTY:
                 if (icalproperty_get_sequence(prop) == 0) {
-                    icalproperty_free(prop);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
@@ -2707,16 +2704,22 @@ void icalcomponent_normalize(icalcomponent *comp)
             }
         }
 
-        icalproperty_normalize(prop);
-        pvl_insert_ordered(sorted_props, prop_compare, prop);
+        if (remove) {
+            icalproperty_set_parent(prop, 0); // MUST NOT have a parent to free
+            icalproperty_free(prop);
+        }
+        else {
+            pvl_insert_ordered(sorted_props, prop_compare, prop);
+        }
     }
 
     pvl_free(comp->properties);
     comp->properties = sorted_props;
 
-    while ((mycomp = pvl_pop(comp->components)) != 0) {
-        icalcomponent_normalize(mycomp);
-        pvl_insert_ordered(sorted_comps, comp_compare, mycomp);
+    /* Normalize sub-components into sorted list */
+    while ((sub = pvl_pop(comp->components)) != 0) {
+        icalcomponent_normalize(sub);
+        pvl_insert_ordered(sorted_comps, comp_compare, sub);
     }
 
     pvl_free(comp->components);
diff --git a/src/libical/icalproperty.c b/src/libical/icalproperty.c
index 7a6ca7f4..5485a55e 100644
--- a/src/libical/icalproperty.c
+++ b/src/libical/icalproperty.c
@@ -966,18 +966,18 @@ void icalproperty_normalize(icalproperty *prop)
     pvl_list sorted_params = pvl_newlist();
     icalparameter *param;
 
+    /* Normalize parameters into sorted list */
     while ((param = pvl_pop(prop->parameters)) != 0) {
-        icalparameter_set_parent(param, 0);
+        int remove = 0;
 
-        /* Skip parameters having default values */
+        /* Remove parameters having default values */
         switch (icalparameter_isa(param)) {
         case ICAL_VALUE_PARAMETER:
             /* Skip VALUE parameters for default property value types */
             switch (prop_kind) {
             case ICAL_ATTACH_PROPERTY:
                 if (icalparameter_get_value(param) == ICAL_VALUE_URI) {
-                    icalparameter_free(param);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
@@ -988,15 +988,13 @@ void icalproperty_normalize(icalproperty *prop)
             case ICAL_RDATE_PROPERTY:
             case ICAL_RECURRENCEID_PROPERTY:
                 if (icalparameter_get_value(param) == ICAL_VALUE_DATETIME) {
-                    icalparameter_free(param);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
             case ICAL_DURATION_PROPERTY:
                 if (icalparameter_get_value(param) == ICAL_VALUE_DURATION) {
-                    icalparameter_free(param);
-                    continue;
+                    remove = 1;
                 }
                 break;
 
@@ -1007,64 +1005,55 @@ void icalproperty_normalize(icalproperty *prop)
 
         case ICAL_CUTYPE_PARAMETER:
             if (icalparameter_get_cutype(param) == ICAL_CUTYPE_INDIVIDUAL) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
         case ICAL_ENCODING_PARAMETER:
             if (icalparameter_get_encoding(param) == ICAL_ENCODING_8BIT) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
         case ICAL_FBTYPE_PARAMETER:
             if (icalparameter_get_fbtype(param) == ICAL_FBTYPE_BUSY) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
         case ICAL_PARTSTAT_PARAMETER:
             if (icalparameter_get_partstat(param) == ICAL_PARTSTAT_NEEDSACTION) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
         case ICAL_RELATED_PARAMETER:
             if (icalparameter_get_related(param) == ICAL_RELATED_START) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
         case ICAL_RELTYPE_PARAMETER:
             if (icalparameter_get_reltype(param) == ICAL_RELTYPE_PARENT) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
         case ICAL_ROLE_PARAMETER:
             if (icalparameter_get_role(param) == ICAL_ROLE_REQPARTICIPANT) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
         case ICAL_RSVP_PARAMETER:
             if (icalparameter_get_rsvp(param) == ICAL_RSVP_FALSE) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
         case ICAL_SCHEDULEAGENT_PARAMETER:
             if (icalparameter_get_scheduleagent(param) == ICAL_SCHEDULEAGENT_SERVER) {
-                icalparameter_free(param);
-                continue;
+                remove = 1;
             }
             break;
 
@@ -1072,7 +1061,13 @@ void icalproperty_normalize(icalproperty *prop)
             break;
         }
 
-        pvl_insert_ordered(sorted_params, param_compare, param);
+        if (remove) {
+            icalparameter_set_parent(param, 0); // MUST NOT have a parent to free
+            icalparameter_free(param);
+        }
+        else {
+            pvl_insert_ordered(sorted_params, param_compare, param);
+        }
     }
 
     pvl_free(prop->parameters);