[Core] Fix crash on removing xml attributes. Add unit test.

diff --git a/src/switch_xml.c b/src/switch_xml.c
index 94268da358bc4b3f6932bafc071299063d03ae23..13ac47d8a5818aa071ab902d655648d43c7dd2fe 100644 (file)
--- a/src/switch_xml.c
+++ b/src/switch_xml.c
@@ -2998,12 +2998,16 @@ SWITCH_DECLARE(switch_xml_t) switch_xml_set_attr(switch_xml_t xml, const char *n
                strcpy(xml->attr[l + 3] + c, " ");      /* set name/value as not malloced */
                if (xml->flags & SWITCH_XML_DUP)
                        xml->attr[l + 3][c] = SWITCH_XML_NAMEM;
-       } else if (xml->flags & SWITCH_XML_DUP)
-               free((char *) name);    /* name was strduped */
+               c = l + 2; /* end of attribute list */
+       } else {
+               for (c = l; xml->attr[c]; c += 2);      /* find end of attribute list */
+
+               if (xml->flags & SWITCH_XML_DUP)
+                       free((char*)name);      /* name was strduped */
+               if (xml->attr[c + 1][l / 2] & SWITCH_XML_TXTM)
+                       free(xml->attr[l + 1]); /* old val */
+       }
 
-       for (c = l; xml->attr[c]; c += 2);      /* find end of attribute list */
-       if (xml->attr[c + 1][l / 2] & SWITCH_XML_TXTM)
-               free(xml->attr[l + 1]); /* old val */
        if (xml->flags & SWITCH_XML_DUP)
                xml->attr[c + 1][l / 2] |= SWITCH_XML_TXTM;
        else
@@ -3014,9 +3018,18 @@ SWITCH_DECLARE(switch_xml_t) switch_xml_set_attr(switch_xml_t xml, const char *n
        else {                                          /* remove attribute */
                if (xml->attr[c + 1][l / 2] & SWITCH_XML_NAMEM)
                        free(xml->attr[l]);
-               memmove(xml->attr + l, xml->attr + l + 2, (c - l + 2) * sizeof(char *));
-               xml->attr = (char **) switch_must_realloc(xml->attr, (c + 2) * sizeof(char *));
-               memmove(xml->attr[c + 1] + (l / 2), xml->attr[c + 1] + (l / 2) + 1, (c / 2) - (l / 2)); /* fix list of which name/vals are malloced */
+               c -= 2;
+               if (c > 0) {
+                       memmove(xml->attr + l, xml->attr + l + 2, (c - l + 2) * sizeof(char*));
+                       xml->attr = (char**)switch_must_realloc(xml->attr, (c + 2) * sizeof(char*));
+                       memmove(xml->attr[c + 1] + (l / 2), xml->attr[c + 1] + (l / 2) + 1, (c / 2) - (l / 2)); /* fix list of which name/vals are malloced */
+                       xml->attr[c + 1][c / 2] = '\0';
+               } else {
+                       /* last attribute removed, reset attribute list */
+                       free(xml->attr[3]);
+                       free(xml->attr);
+                       xml->attr = SWITCH_XML_NIL;
+               }
        }
        xml->flags &= ~SWITCH_XML_DUP;  /* clear strdup() flag */
 

diff --git a/tests/unit/switch_core.c b/tests/unit/switch_core.c
index ed84e39c06079746a6a0ced1da2623fd611df93a..56a700c0fb80a8de1a442d7d65fa6fa79efd6c20 100644 (file)
--- a/tests/unit/switch_core.c
+++ b/tests/unit/switch_core.c
@@ -53,6 +53,24 @@ FST_CORE_BEGIN("./conf")
                }
                FST_TEARDOWN_END()
 
+               FST_TEST_BEGIN(test_xml_set_attr)
+               {
+                       switch_xml_t parent_xml = switch_xml_new("xml");
+                       switch_xml_t xml = switch_xml_add_child_d(parent_xml, "test", 1);
+                       switch_xml_set_attr_d(xml, "test1", "1");
+                       switch_xml_set_attr(xml, "a1", "v1");
+                       switch_xml_set_attr_d(xml, "a2", "v2");
+                       switch_xml_set_attr(xml, "test1", NULL);
+                       switch_xml_set_attr_d(xml, "test2", "2");
+                       switch_xml_set_attr_d(xml, "a3", "v3");
+                       switch_xml_set_attr(xml, "test2", NULL);
+                       switch_xml_set_attr(xml, "a1", NULL);
+                       switch_xml_set_attr(xml, "a2", NULL);
+                       switch_xml_set_attr(xml, "a3", NULL);
+                       switch_xml_free(parent_xml);
+               }
+               FST_TEST_END()
+
 #ifdef HAVE_OPENSSL
                FST_TEST_BEGIN(test_md5)
                {