check architecture names for mistakes

Fixes: https://pagure.io/koji/issue/1237

hub/kojihub.py

@@ -883,10 +883,11 @@ def _pkglist_remove(tag_id, pkg_id):
 
 def _pkglist_add(tag_id, pkg_id, owner, block, extra_arches):
     #revoke old entry (if present)
-    _pkglist_remove(tag_id, pkg_id)
     data = dslice(locals(), ('tag_id', 'owner', 'extra_arches'))
     data['package_id'] = pkg_id
     data['blocked'] = block
+    validate_arches_string(data['extra_arches'])
+    _pkglist_remove(tag_id, pkg_id)
     insert = InsertProcessor('tag_packages', data=data)
     insert.make_create()  #XXX user_id?
     insert.execute()
@@ -923,6 +924,8 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force,
             assert_policy('package_list', policy_data)
     if not pkg:
         pkg = lookup_package(pkginfo, create=True)
+    # validate arches before running callbacks
+    validate_arches_string(extra_arches)
     koji.plugin.run_callbacks('prePackageListChange', action=action, tag=tag, package=pkg, owner=owner,
                               block=block, extra_arches=extra_arches, force=force, update=update)
     # first check to see if package is:
@@ -3021,6 +3024,16 @@ def lookup_build_target(info, strict=False, create=False):
     return lookup_name('build_target', info, strict, create)
 
 
+def validate_arches_string(arches, strict=True):
+    """run checks against architectures space-separated strings"""
+    if re.match(r'^[a-zA-Z0-9_\- ]+$', arches):
+        return True
+    elif strict:
+        raise koji.GenericError("Architecture can be only [a-zA-Z0-9_-]")
+    else:
+        return False
+
+
 def create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_support=False, maven_include_all=False, extra=None):
     """Create a new tag"""
     context.session.assertPerm('admin')
@@ -3035,6 +3048,9 @@ def _create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_s
         raise koji.GenericError("Tag name %s is too long. Max length is %s characters",
                                 name, max_name_length)
 
+    if arches is not None:
+        validate_arches_string(arches)
+
     if not context.opts.get('EnableMaven') and (maven_support or maven_include_all):
         raise koji.GenericError("Maven support not enabled")
 
@@ -3215,6 +3231,11 @@ SET name = %(name)s
 WHERE id = %(tagID)i"""
         _dml(update, values)
 
+    # sanitize architecture names (space-separated string)
+    arches = kwargs.get('arches')
+    if arches and tag['arches'] != arches:
+        validate_arches_string(arches)
+
     #check for changes
     data = tag.copy()
     changed = False

tests/test_hub/test_create_tag.py

@@ -1,8 +1,6 @@
+# coding: utf-8
 from __future__ import absolute_import
-import copy
 import mock
-import shutil
-import tempfile
 try:
     import unittest2 as unittest
 except ImportError:
@@ -67,3 +65,20 @@ class TestCreateTag(unittest.TestCase):
         self.assertEqual(insert.data, values)
         self.assertEqual(insert.rawdata, {})
         insert = self.inserts[0]
+
+    def test_invalid_archs(self):
+        self.get_tag.return_value = None
+        self.get_tag_id.return_value = 99
+        self.context.event_id = 42
+        self.context.session.user_id = 23
+
+        with self.assertRaises(koji.GenericError):
+            kojihub.create_tag('newtag', arches=u'ěšč')
+
+        with self.assertRaises(koji.GenericError):
+            kojihub.create_tag('newtag', arches=u'arch1;arch2')
+
+        with self.assertRaises(koji.GenericError):
+            kojihub.create_tag('newtag', arches=u'arch1,arch2')
+
+        self.assertEqual(len(self.inserts), 0)

tests/test_hub/test_edit_tag.py

@@ -1,3 +1,4 @@
+# coding: utf-8
 from __future__ import absolute_import
 import mock
 try:
@@ -197,3 +198,40 @@ WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333})
         self.get_perm_id.assert_called_once()
         self._singleValue.assert_called_once()
         self.assertEqual(cm.exception.args[0], 'Name newtag already taken by tag 2')
+
+    def test_invalid_archs(self):
+        self.get_tag.return_value = {
+            'create_event': 42,
+            'creator_id': 23,
+            'arches': 'arch1 arch2',
+            'locked': True,
+            'maven_include_all': True,
+            'maven_support': True,
+            'perm_id': None,
+            'tag_id': 333,
+            'name': 'newtag',
+            'id': 345,
+        }
+
+        # valid
+        kwargs = {
+            'name': 'newtag',
+            'arches': 'valid_arch',
+        }
+        kojihub._edit_tag('tag', **kwargs)
+
+        # invalid 1
+        kwargs['arches'] = u'ěšč'
+        with self.assertRaises(koji.GenericError):
+            kojihub._edit_tag('tag', **kwargs)
+
+        # invalid 2
+        kwargs['arches'] = u'arch1;arch2'
+        with self.assertRaises(koji.GenericError):
+            kojihub._edit_tag('tag', **kwargs)
+
+        # invalid 2
+        kwargs['arches'] = u'arch1,arch2'
+        with self.assertRaises(koji.GenericError):
+            kojihub._edit_tag('tag', **kwargs)
+

tests/test_hub/test_validate_arches_string.py

@@ -0,0 +1,25 @@
+# coding: utf-8
+from __future__ import absolute_import
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+import koji
+import kojihub
+
+class TestValidateArchesString(unittest.TestCase):
+    def test_valid_arches(self):
+        kojihub.validate_arches_string('i386')
+        kojihub.validate_arches_string('i386 x86_64')
+        kojihub.validate_arches_string('i386 x86_64   ')
+
+    def test_invalid_arches(self):
+        with self.assertRaises(koji.GenericError):
+            kojihub.validate_arches_string(u'ěšč')
+
+        with self.assertRaises(koji.GenericError):
+            kojihub.validate_arches_string(u'i386;x86_64')
+
+        with self.assertRaises(koji.GenericError):
+            kojihub.validate_arches_string(u'i386,x86_64')
+