From 3c758a4ea670fab1f4b55fa878ebf2b2ff4b678b Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hr=C3=A1zk=C3=BD?= <lhrazky@redhat.com>

Date: Tue, 28 Apr 2020 09:08:05 +0200

Subject: [PATCH] Handle empty comps group name (RhBug:1826198)

Don't crash on empty comps group/environment name. In outputs, use the

"<name-unset>" placeholder instead of the name.

https://bugzilla.redhat.com/show_bug.cgi?id=1826198

---

 dnf/cli/commands/group.py  |  4 ++--

 dnf/cli/output.py          | 16 ++++++++++------

 dnf/comps.py               | 11 ++++++++++-

 dnf/db/group.py            | 12 ++++++++----

 tests/repos/main_comps.xml |  7 +++++++

 tests/support.py           |  2 +-

 tests/test_comps.py        |  6 +++---

 tests/test_groups.py       |  9 +++++++++

 8 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/dnf/cli/commands/group.py b/dnf/cli/commands/group.py

index f535a50980..4ffd3b89c8 100644

--- a/dnf/cli/commands/group.py

+++ b/dnf/cli/commands/group.py

@@ -177,7 +177,7 @@ def _list(self, userlist):

         def _out_grp(sect, group):

             if not done:

                 print(sect)

-            msg = '   %s' % group.ui_name

+            msg = '   %s' % (group.ui_name if group.ui_name is not None else _("<name-unset>"))

             if print_ids:

                 msg += ' (%s)' % group.id

             if group.lang_only:

@@ -188,7 +188,7 @@ def _out_env(sect, envs):

             if envs:

                 print(sect)

             for e in envs:

-                msg = '   %s' % e.ui_name

+                msg = '   %s' % (e.ui_name if e.ui_name is not None else _("<name-unset>"))

                 if print_ids:

                     msg += ' (%s)' % e.id

                 print(msg)

diff --git a/dnf/cli/output.py b/dnf/cli/output.py

index 67eab80b19..2585a5c773 100644

--- a/dnf/cli/output.py

+++ b/dnf/cli/output.py

@@ -1221,47 +1221,51 @@ def _add_line(lines, data, a_wid, po, obsoletes=[]):

                 lines.append((name, "", "", "", "", "", ""))

             pkglist_lines.append((action, lines))

         if self.base._history:

+            def format_line(group):

+                name = group.getName()

+                return (name if name else _("<name-unset>"), "", "", "", "", "", "")

+

             install_env_group = self.base._history.env._installed

             if install_env_group:

                 action = _("Installing Environment Groups")

                 lines = []

                 for group in install_env_group.values():

-                    lines.append((group.getName(), "", "", "", "", "", ""))

+                    lines.append(format_line(group))

                 pkglist_lines.append((action, lines))

             upgrade_env_group = self.base._history.env._upgraded

             if upgrade_env_group:

                 action = _("Upgrading Environment Groups")

                 lines = []

                 for group in upgrade_env_group.values():

-                    lines.append((group.getName(), "", "", "", "", "", ""))

+                    lines.append(format_line(group))

                 pkglist_lines.append((action, lines))

             remove_env_group = self.base._history.env._removed

             if remove_env_group:

                 action = _("Removing Environment Groups")

                 lines = []

                 for group in remove_env_group.values():

-                    lines.append((group.getName(), "", "", "", "", "", ""))

+                    lines.append(format_line(group))

                 pkglist_lines.append((action, lines))

             install_group = self.base._history.group._installed

             if install_group:

                 action = _("Installing Groups")

                 lines = []

                 for group in install_group.values():

-                    lines.append((group.getName(), "", "", "", "", "", ""))

+                    lines.append(format_line(group))

                 pkglist_lines.append((action, lines))

             upgrade_group = self.base._history.group._upgraded

             if upgrade_group:

                 action = _("Upgrading Groups")

                 lines = []

                 for group in upgrade_group.values():

-                    lines.append((group.getName(), "", "", "", "", "", ""))

+                    lines.append(format_line(group))

                 pkglist_lines.append((action, lines))

             remove_group = self.base._history.group._removed

             if remove_group:

                 action = _("Removing Groups")

                 lines = []

                 for group in remove_group.values():

-                    lines.append((group.getName(), "", "", "", "", "", ""))

+                    lines.append(format_line(group))

                 pkglist_lines.append((action, lines))

         # show skipped conflicting packages

         if not self.conf.best and self.base._goal.actions & forward_actions:

diff --git a/dnf/comps.py b/dnf/comps.py

index 316d647087..4ca15b1e07 100644

--- a/dnf/comps.py

+++ b/dnf/comps.py

@@ -75,7 +75,16 @@ def _by_pattern(pattern, case_sensitive, sqn):

     else:

         match = re.compile(fnmatch.translate(pattern), flags=re.I).match

-    return {g for g in sqn if match(g.name) or match(g.id) or match(g.ui_name)}

+    ret = set()

+    for g in sqn:

+        if match(g.id):

+            ret.add(g)

+        elif g.name is not None and match(g.name):

+            ret.add(g)

+        elif g.ui_name is not None and match(g.ui_name):

+            ret.add(g)

+

+    return ret

 def _fn_display_order(group):

diff --git a/dnf/db/group.py b/dnf/db/group.py

index e3a087760b..5d7e18d1a8 100644

--- a/dnf/db/group.py

+++ b/dnf/db/group.py

@@ -78,8 +78,10 @@ def _get_obj_id(self, obj):

     def new(self, obj_id, name, translated_name, pkg_types):

         swdb_group = self.history.swdb.createCompsGroupItem()

         swdb_group.setGroupId(obj_id)

-        swdb_group.setName(name)

-        swdb_group.setTranslatedName(translated_name)

+        if name is not None:

+            swdb_group.setName(name)

+        if translated_name is not None:

+            swdb_group.setTranslatedName(translated_name)

         swdb_group.setPackageTypes(pkg_types)

         return swdb_group

@@ -136,8 +138,10 @@ def _get_obj_id(self, obj):

     def new(self, obj_id, name, translated_name, pkg_types):

         swdb_env = self.history.swdb.createCompsEnvironmentItem()

         swdb_env.setEnvironmentId(obj_id)

-        swdb_env.setName(name)

-        swdb_env.setTranslatedName(translated_name)

+        if name is not None:

+            swdb_env.setName(name)

+        if translated_name is not None:

+            swdb_env.setTranslatedName(translated_name)

         swdb_env.setPackageTypes(pkg_types)

         return swdb_env

diff --git a/tests/repos/main_comps.xml b/tests/repos/main_comps.xml

index 9e694d13a5..584bb25b3a 100644

--- a/tests/repos/main_comps.xml

+++ b/tests/repos/main_comps.xml

@@ -49,6 +49,13 @@

       <packagereq type="optional">brokendeps</packagereq>

     </packagelist>

   </group>

+  <group>

+    <id>missing-name-group</id>

+    <name></name>

+    <packagelist>

+      <packagereq type="mandatory">meaning-of-life</packagereq>

+    </packagelist>

+  </group>

   <category>

    <id>base-system</id>

    <display_order>99</display_order>

diff --git a/tests/support.py b/tests/support.py

index e549ba5b95..a7d6a8542c 100644

--- a/tests/support.py

+++ b/tests/support.py

@@ -94,7 +94,7 @@ def mock_open(mock=None, data=None):

 MAIN_NSOLVABLES = 9

 UPDATES_NSOLVABLES = 4

 AVAILABLE_NSOLVABLES = MAIN_NSOLVABLES + UPDATES_NSOLVABLES

-TOTAL_GROUPS = 4

+TOTAL_GROUPS = 5

 TOTAL_NSOLVABLES = SYSTEM_NSOLVABLES + AVAILABLE_NSOLVABLES

diff --git a/tests/test_comps.py b/tests/test_comps.py

index 30d468e3af..763218587f 100644

--- a/tests/test_comps.py

+++ b/tests/test_comps.py

@@ -107,7 +107,7 @@ def test_group_packages(self):

     def test_iteration(self):

         comps = self.comps

         self.assertEqual([g.name for g in comps.groups_iter()],

-                         ['Base', 'Solid Ground', "Pepper's", "Broken Group"])

+                         ['Base', 'Solid Ground', "Pepper's", "Broken Group", None])

         self.assertEqual([c.name for c in comps.categories_iter()],

                          ['Base System'])

         g = dnf.util.first(comps.groups_iter())

@@ -115,7 +115,7 @@ def test_iteration(self):

     def test_group_display_order(self):

         self.assertEqual([g.name for g in self.comps.groups],

-                         ["Pepper's", 'Base', 'Solid Ground', 'Broken Group'])

+                         ["Pepper's", 'Base', 'Solid Ground', 'Broken Group', None])

     def test_packages(self):

         comps = self.comps

@@ -127,7 +127,7 @@ def test_packages(self):

     def test_size(self):

         comps = self.comps

-        self.assertLength(comps, 6)

+        self.assertLength(comps, 7)

         self.assertLength(comps.groups, tests.support.TOTAL_GROUPS)

         self.assertLength(comps.categories, 1)

         self.assertLength(comps.environments, 1)

diff --git a/tests/test_groups.py b/tests/test_groups.py

index fe388f96c0..8972da687e 100644

--- a/tests/test_groups.py

+++ b/tests/test_groups.py

@@ -295,6 +295,15 @@ def test_group_install_broken_optional_nonstrict(self):

         self.assertLength(inst, 1)

         self.assertEmpty(removed)

+    def test_group_install_missing_name(self):

+        comps_group = self.base.comps.group_by_pattern('missing-name-group')

+

+        cnt = self.base.group_install(comps_group.id, ('mandatory', 'default', 'optional'),

+                                      strict=False)

+        self._swdb_commit()

+        self.base.resolve()

+        self.assertEqual(cnt, 1)

+

 class EnvironmentInstallTest(tests.support.ResultTestCase):

     """Set up a test where sugar is considered not installed."""