Don't include packages with forbidden control chars in repodata + some warning fixes

src/koji.c

@@ -104,7 +104,6 @@ koji_stuff_prepare(struct KojiMergedReposStuff **koji_stuff_ptr,
     GSList *element;
     int repoid;
     int result;
-    GError *tmp_err = NULL;
 
     // Pointers to elements in the koji_stuff_ptr
     GHashTable *include_srpms = NULL; // XXX

src/misc.c

@@ -999,15 +999,13 @@ cr_log_fn(const gchar *log_domain,
 void
 cr_slist_free_full(GSList *list, GDestroyNotify free_f)
 {
-    g_slist_foreach(list, (GFunc) free_f, NULL);
-    g_slist_free(list);
+    g_slist_free_full(list, free_f);
 }
 
 
 void cr_queue_free_full(GQueue *queue, GDestroyNotify free_f)
 {
-    g_queue_foreach(queue, (GFunc) free_f, NULL);
-    g_queue_free(queue);
+    g_queue_free_full(queue, free_f);
 }
 
 

src/package.c

@@ -92,53 +92,43 @@ cr_package_free(cr_Package *package)
  */
 
     if (package->requires) {
-        g_slist_foreach (package->requires, (GFunc) g_free, NULL);
-        g_slist_free (package->requires);
+        g_slist_free_full(package->requires, g_free);
     }
 
     if (package->provides) {
-        g_slist_foreach (package->provides, (GFunc) g_free, NULL);
-        g_slist_free (package->provides);
+        g_slist_free_full(package->provides, g_free);
     }
 
     if (package->conflicts) {
-        g_slist_foreach (package->conflicts, (GFunc) g_free, NULL);
-        g_slist_free (package->conflicts);
+        g_slist_free_full(package->conflicts, g_free);
     }
 
     if (package->obsoletes) {
-        g_slist_foreach (package->obsoletes, (GFunc) g_free, NULL);
-        g_slist_free (package->obsoletes);
+        g_slist_free_full(package->obsoletes, g_free);
     }
 
     if (package->suggests) {
-        g_slist_foreach (package->suggests, (GFunc) g_free, NULL);
-        g_slist_free (package->suggests);
+        g_slist_free_full(package->suggests, g_free);
     }
 
     if (package->enhances) {
-        g_slist_foreach (package->enhances, (GFunc) g_free, NULL);
-        g_slist_free (package->enhances);
+        g_slist_free_full(package->enhances, g_free);
     }
 
     if (package->recommends) {
-        g_slist_foreach (package->recommends, (GFunc) g_free, NULL);
-        g_slist_free (package->recommends);
+        g_slist_free_full(package->recommends, g_free);
     }
 
     if (package->supplements) {
-        g_slist_foreach (package->supplements, (GFunc) g_free, NULL);
-        g_slist_free (package->supplements);
+        g_slist_free_full(package->supplements, g_free);
     }
 
     if (package->files) {
-        g_slist_foreach (package->files, (GFunc) g_free, NULL);
-        g_slist_free (package->files);
+        g_slist_free_full(package->files, g_free);
     }
 
     if (package->changelogs) {
-        g_slist_foreach (package->changelogs, (GFunc) g_free, NULL);
-        g_slist_free (package->changelogs);
+        g_slist_free_full(package->changelogs, g_free);
     }
 
     g_free(package->siggpg);

src/xml_dump.c

@@ -99,7 +99,7 @@ cr_xmlNewTextChild(xmlNodePtr parent,
 
     if (!orig_content) {
         content = BAD_CAST "";
-    } else if (xmlCheckUTF8(orig_content) && !cr_hascontrollchars(orig_content)) {
+    } else if (xmlCheckUTF8(orig_content)) {
         content = (xmlChar *) orig_content;
     } else {
         size_t len = strlen((const char *) orig_content);
@@ -198,6 +198,80 @@ cr_xml_dump_files(xmlNodePtr node, cr_Package *package, int primary)
     }
 }
 
+gboolean
+cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(GSList *dep)
+{
+    GSList *element;
+    for (element = dep; element; element=g_slist_next(element)) {
+        cr_Dependency *d = element->data;
+        if ((d->name    && cr_hascontrollchars((unsigned char *) d->name))    ||
+            (d->epoch   && cr_hascontrollchars((unsigned char *) d->epoch))   ||
+            (d->version && cr_hascontrollchars((unsigned char *) d->version)) ||
+            (d->release && cr_hascontrollchars((unsigned char *) d->release)))
+        {
+            return 1;
+        }
+    }
+    return 0;
+}
+
+gboolean
+cr_Package_contains_forbidden_control_chars(cr_Package *pkg)
+{
+    if ((pkg->name          && cr_hascontrollchars((unsigned char *) pkg->name))          ||
+        (pkg->arch          && cr_hascontrollchars((unsigned char *) pkg->arch))          ||
+        (pkg->version       && cr_hascontrollchars((unsigned char *) pkg->version))       ||
+        (pkg->epoch         && cr_hascontrollchars((unsigned char *) pkg->epoch))         ||
+        (pkg->release       && cr_hascontrollchars((unsigned char *) pkg->release))       ||
+        (pkg->summary       && cr_hascontrollchars((unsigned char *) pkg->summary))       ||
+        (pkg->description   && cr_hascontrollchars((unsigned char *) pkg->description))   ||
+        (pkg->url           && cr_hascontrollchars((unsigned char *) pkg->url))           ||
+        (pkg->rpm_license   && cr_hascontrollchars((unsigned char *) pkg->rpm_license))   ||
+        (pkg->rpm_vendor    && cr_hascontrollchars((unsigned char *) pkg->rpm_vendor))    ||
+        (pkg->rpm_group     && cr_hascontrollchars((unsigned char *) pkg->rpm_group))     ||
+        (pkg->rpm_buildhost && cr_hascontrollchars((unsigned char *) pkg->rpm_buildhost)) ||
+        (pkg->rpm_sourcerpm && cr_hascontrollchars((unsigned char *) pkg->rpm_sourcerpm)) ||
+        (pkg->rpm_packager  && cr_hascontrollchars((unsigned char *) pkg->rpm_packager))  ||
+        (pkg->location_href && cr_hascontrollchars((unsigned char *) pkg->location_href)) ||
+        (pkg->location_base && cr_hascontrollchars((unsigned char *) pkg->location_base)))
+    {
+        return 1;
+    }
+
+    if (cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(pkg->requires)    ||
+        cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(pkg->provides)    ||
+        cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(pkg->conflicts)   ||
+        cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(pkg->obsoletes)   ||
+        cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(pkg->suggests)    ||
+        cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(pkg->enhances)    ||
+        cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(pkg->recommends)  ||
+        cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(pkg->supplements))
+    {
+        return 1;
+    }
+
+    GSList *element;
+
+    for (element = pkg->files; element; element=g_slist_next(element)) {
+        cr_PackageFile *f = element->data;
+        if ((f->name && cr_hascontrollchars((unsigned char *) f->name)) ||
+            (f->path && cr_hascontrollchars((unsigned char *) f->path)))
+        {
+            return 1;
+        }
+    }
+
+    for (element = pkg->changelogs; element; element=g_slist_next(element)) {
+        cr_ChangelogEntry *ch = element->data;
+        if ((ch->author    && cr_hascontrollchars((unsigned char *) ch->author)) ||
+            (ch->changelog && cr_hascontrollchars((unsigned char *) ch->changelog)))
+        {
+            return 1;
+        }
+    }
+
+    return 0;
+}
 
 struct cr_XmlStruct
 cr_xml_dump(cr_Package *pkg, GError **err)
@@ -211,6 +285,12 @@ cr_xml_dump(cr_Package *pkg, GError **err)
     result.filelists = NULL;
     result.other     = NULL;
 
+    if (cr_Package_contains_forbidden_control_chars(pkg)) {
+        g_set_error(err, CREATEREPO_C_ERROR, CRE_XMLDATA,
+                    "Forbidden control chars found (ASCII values <32 except 9, 10 and 13).");
+        return result;
+    }
+
     if (!pkg)
         return result;
 

src/xml_dump.h

@@ -175,6 +175,22 @@ gboolean cr_hascontrollchars(const unsigned char *str);
  */
 gchar *cr_prepend_protocol(const gchar *url);
 
+/** Check if package contains any strings with chars
+ * with value <32 (except 9, 10 and 13), using cr_hascontrollchars
+ *
+ * @param pkg           the cr_Package in question
+ * @return              boolean value
+ */
+gboolean cr_Package_contains_forbidden_control_chars(cr_Package *pkg);
+
+/** Check if list of cr_Dependency stucts contains any strings with chars
+ * with value <32 (except 9, 10 and 13), using cr_hascontrollchars
+ *
+ * @param deps          the GSList of cr_Dependencies in question
+ * @return              boolean value
+ */
+gboolean cr_GSList_of_cr_Dependency_contains_forbidden_control_chars(GSList *deps);
+
 /** @} */
 
 #ifdef __cplusplus

src/xml_parser_updateinfo.c

@@ -354,7 +354,6 @@ cr_start_handler(void *pdata, const char *element, const char **attr)
         assert(pd->updateinfo);
         assert(pd->updaterecord);
         assert(pd->updatecollection);
-        assert(pd->updatecollectionmodule);
         assert(pd->updatecollectionpackage);
         val = cr_find_attr("type", attr);
         if (val)
@@ -365,7 +364,6 @@ cr_start_handler(void *pdata, const char *element, const char **attr)
         assert(pd->updateinfo);
         assert(pd->updaterecord);
         assert(pd->updatecollection);
-        assert(pd->updatecollectionmodule);
         assert(pd->updatecollectionpackage);
         package->reboot_suggested = TRUE;
         break;

tests/fixtures.h

@@ -106,4 +106,111 @@
 #define TEST_UPDATEINFO_02      TEST_UPDATEINFO_FILES_PATH"updateinfo_02.xml.xz"
 #define TEST_UPDATEINFO_03      TEST_UPDATEINFO_FILES_PATH"updateinfo_03.xml"
 
+#include "createrepo/package.h"
+
+cr_Package *
+get_package()
+{
+    cr_Package *p;
+    cr_Dependency *dep;
+    cr_PackageFile *file;
+
+    p = cr_package_new();
+    p->pkgId = "123456";
+    p->name = "foo";
+    p->arch = "x86_64";
+    p->version = "1.2.3";
+    p->epoch = "1";
+    p->release = "2";
+    p->summary = "foo package";
+    p->description = "super cool package";
+    p->url = "http://package.com";
+    p->time_file = 123456;
+    p->time_build = 234567;
+    p->rpm_license = "GPL";
+    p->rpm_vendor = NULL;
+    p->rpm_group = NULL;
+    p->rpm_buildhost = NULL;
+    p->rpm_sourcerpm = "foo.src.rpm";
+    p->rpm_header_start = 20;
+    p->rpm_header_end = 120;
+    p->rpm_packager = NULL;
+    p->size_package = 123;
+    p->size_installed = 20;
+    p->size_archive = 30;
+    p->location_href = "foo.rpm";
+    p->location_base = "/test/";
+    p->checksum_type = "sha256";
+
+    dep = cr_dependency_new();
+    dep->name = "foobar_provide";
+    dep->flags = NULL;
+    dep->pre = FALSE;
+    p->provides = (g_slist_prepend(p->provides, dep));
+
+    dep = cr_dependency_new();
+    dep->name = "foobar_dep";
+    dep->flags = NULL;
+    dep->pre = FALSE;
+    dep->epoch = "3";
+    p->requires = (g_slist_prepend(p->requires, dep));
+
+    dep = cr_dependency_new();
+    dep->name = "foobar_pre_dep";
+    dep->flags = "LE";
+    dep->epoch = "3";
+    dep->pre = TRUE;
+    p->requires = g_slist_prepend(p->requires, dep);
+
+    file = cr_package_file_new();
+    file->type = "";
+    file->path = "/bin/";
+    file->name = "foo";
+    p->files = g_slist_prepend(p->files, file);
+
+    file = cr_package_file_new();
+    file->type = "dir";
+    file->path = "/var/foo/";
+    file->name = NULL;
+    p->files = g_slist_prepend(p->files, file);
+
+    file = cr_package_file_new();
+    file->type = "dir";
+    file->path = "/var/foo/";
+    file->name = "baz";
+    p->files = g_slist_prepend(p->files, file);
+    return p;
+}
+
+cr_Package *
+get_empty_package()
+{
+    cr_Package *p;
+    cr_Dependency *dep;
+    cr_PackageFile *file;
+
+    p = cr_package_new();
+    p->name = "foo";
+
+    dep = cr_dependency_new();
+    dep->name   = NULL;
+    dep->flags  = NULL;
+    dep->pre    = FALSE;
+    p->requires = (g_slist_prepend(p->requires, dep));
+
+    dep = cr_dependency_new();
+    dep->name   = NULL;
+    dep->flags  = NULL;
+    dep->pre    = TRUE;
+    p->requires = g_slist_prepend(p->requires, dep);
+
+    file = cr_package_file_new();
+    file->type = NULL;
+    file->path = NULL;
+    file->name = NULL;
+    p->files   = g_slist_prepend(p->files, file);
+
+    return p;
+}
+
 #endif

tests/test_koji.c

@@ -23,26 +23,6 @@
 #include "createrepo/koji.h"
 #include "createrepo/load_metadata.h"
 
-cr_Package *
-get_package()
-{
-    cr_Package *p;
-    cr_Dependency *dep;
-    cr_PackageFile *file;
-
-    p = cr_package_new();
-    p->pkgId = "123456";
-    p->name = "foo";
-    p->arch = "x86_64";
-    p->version = "1.2.3";
-    p->epoch = "1";
-    p->release = "2";
-    p->summary = "foo package";
-    p->rpm_sourcerpm = "foo.src.rpm";
-
-    return p;
-}
-
 // Tests
 
 static void

tests/test_load_metadata.c

@@ -178,7 +178,6 @@ static void test_cr_metadata_locate_and_load_modulemd(void)
 {
     int ret;
     guint size;
-    cr_Package *pkg;
     cr_Metadata *metadata;
 
     metadata = cr_metadata_new(CR_HT_KEY_NAME, 0, NULL);

tests/test_locate_metadata.c

@@ -146,7 +146,7 @@ static void test_cr_copy_metadatum(void)
 static void test_cr_insert_additional_metadatum(void)
 {
     //add to not allocated GSList
-    GSList *d;
+    GSList *d = NULL;
     cr_Metadatum *m;
 
     d = cr_insert_additional_metadatum("./test_path.xml", "group", d);