#1272 check architecture names for mistakes
Merged 4 years ago by mikem. Opened 5 years ago by tkopecek.
tkopecek/koji issue1237  into  master

file modified
+6 -6
@@ -195,7 +195,7 @@ 

          if val is not None:

              vals[key] = val

      if 'arches' in vals:

-         vals['arches'] = parse_arches(vals['arches'])

+         vals['arches'] = koji.parse_arches(vals['arches'])

  

      session.multicall = True

      for host in args:
@@ -350,7 +350,7 @@ 

              continue

          to_add.append(package)

      if options.extra_arches:

-         opts['extra_arches'] = parse_arches(options.extra_arches)

+         opts['extra_arches'] = koji.parse_arches(options.extra_arches)

  

      # add the packages

      print("Adding %i packages to tag %s" % (len(to_add), dsttag['name']))
@@ -472,7 +472,7 @@ 

      source = args[1]

      opts = {}

      if build_opts.arch_override:

-         opts['arch_override'] = parse_arches(build_opts.arch_override)

+         opts['arch_override'] = koji.parse_arches(build_opts.arch_override)

      for key in ('skip_tag', 'scratch', 'repo_id', 'fail_fast'):

          val = getattr(build_opts, key)

          if val is not None:
@@ -4823,7 +4823,7 @@ 

      if options.parent:

          opts['parent'] = options.parent

      if options.arches:

-         opts['arches'] = parse_arches(options.arches)

+         opts['arches'] = koji.parse_arches(options.arches)

      if options.maven_support:

          opts['maven_support'] = True

      if options.include_all:
@@ -4865,7 +4865,7 @@ 

      tag = args[0]

      opts = {}

      if options.arches:

-         opts['arches'] = parse_arches(options.arches)

+         opts['arches'] = koji.parse_arches(options.arches)

      if options.no_perm:

          opts['perm_id'] = None

      elif options.perm:
@@ -6237,7 +6237,7 @@ 

          parser.error(_("Please specify an archlist, a tag, and at least one package"))

          assert False  # pragma: no cover

      activate_session(session, goptions)

-     arches = parse_arches(args[0])

+     arches = koji.parse_arches(args[0])

      tag = args[1]

      for package in args[2:]:

          #really should implement multicall...

file modified
+2 -20
@@ -20,6 +20,8 @@ 

  

  import koji

  from koji.util import to_list

+ # import parse_arches to current namespace for backward compatibility

+ from koji import parse_arches

  

  # for compatibility with plugins based on older version of lib

  # Use optparse imports directly in new code.
@@ -132,26 +134,6 @@ 

          print_task_recurse(child,depth+1)

  

  

- def parse_arches(arches, to_list=False):

-     """Normalize user input for a list of arches.

- 

-     This method parses a single comma- or space-separated string of arches and

-     returns a space-separated string.

- 

-     :param str arches: comma- or space-separated string of arches, eg.

-                        "x86_64,ppc64le", or "x86_64 ppc64le"

-     :param bool to_list: return a list of each arch, instead of a single

-                          string. This is False by default.

-     :returns: a space-separated string like "x86_64 ppc64le", or a list like

-               ['x86_64', 'ppc64le'].

-     """

-     arches = arches.replace(',', ' ').split()

-     if to_list:

-         return arches

-     else:

-         return ' '.join(arches)

- 

- 

  class TaskWatcher(object):

  

      def __init__(self,task_id,session,level=0,quiet=False):

file modified
+15 -2
@@ -875,10 +875,11 @@ 

  

  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

+     data['extra_arches'] = koji.parse_arches(data['extra_arches'], strict=True, allow_none=True)

+     _pkglist_remove(tag_id, pkg_id)

      insert = InsertProcessor('tag_packages', data=data)

      insert.make_create()  #XXX user_id?

      insert.execute()
@@ -915,6 +916,8 @@ 

              assert_policy('package_list', policy_data)

      if not pkg:

          pkg = lookup_package(pkginfo, create=True)

+     # validate arches before running callbacks

+     extra_arches = koji.parse_arches(extra_arches, strict=True, allow_none=True)

      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:
@@ -2997,6 +3000,8 @@ 

          raise koji.GenericError("Tag name %s is too long. Max length is %s characters",

                                  name, max_name_length)

  

+     arches = koji.parse_arches(arches, strict=True, allow_none=True)

+ 

      if not context.opts.get('EnableMaven') and (maven_support or maven_include_all):

          raise koji.GenericError("Maven support not enabled")

  
@@ -3177,6 +3182,11 @@ 

  WHERE id = %(tagID)i"""

          _dml(update, values)

  

+     # sanitize architecture names (space-separated string)

+     arches = kwargs.get('arches')

+     if arches and tag['arches'] != arches:

+         kwargs['arches'] = koji.parse_arches(arches, strict=True, allow_none=True)

+ 

      #check for changes

      data = tag.copy()

      changed = False
@@ -10876,6 +10886,9 @@ 

      def addHost(self, hostname, arches, krb_principal=None):

          """Add a host to the database"""

          context.session.assertPerm('admin')

+         # validate arches

+         arches = " ".join(arches)

+         arches = koji.parse_arches(arches, strict=True)

          if get_host(hostname):

              raise koji.GenericError('host already exists: %s' % hostname)

          q = """SELECT id FROM channels WHERE name = 'default'"""
@@ -10893,7 +10906,7 @@ 

          _dml(insert, dslice(locals(), ('hostID', 'userID', 'hostname')))

  

          insert = InsertProcessor('host_config')

-         insert.set(host_id=hostID, arches=" ".join(arches))

+         insert.set(host_id=hostID, arches=arches)

          insert.make_create()

          insert.execute()

  

file modified
+31
@@ -1104,6 +1104,37 @@ 

      else:

          return arch

  

+ def parse_arches(arches, to_list=False, strict=False, allow_none=False):

+     """Normalize user input for a list of arches.

+ 

+     This method parses a single comma- or space-separated string of arches and

+     returns a space-separated string.

+ 

+     Raise an error if arches string contain non-allowed characters. In strict

+     version allow only space-separated strings (db input).

+ 

+     :param str arches: comma- or space-separated string of arches, eg.

+                        "x86_64,ppc64le", or "x86_64 ppc64le"

+     :param bool to_list: return a list of each arch, instead of a single

+                          string. This is False by default.

+     :param bool allow_none: convert None to ""

+     :returns: a space-separated string like "x86_64 ppc64le", or a list like

+               ['x86_64', 'ppc64le'].

+     """

+     if allow_none and arches is None:

+         arches = ''

+     if not strict:

+         arches = arches.replace(',', ' ')

+     if not re.match(r'^[a-zA-Z0-9_\- ]*$', arches):

+         raise GenericError("Architecture can be only [a-zA-Z0-9_-]")

+ 

+     arches = arches.split()

+     if to_list:

+         return arches

+     else:

+         return ' '.join(arches)

+ 

+ 

  class POMHandler(xml.sax.handler.ContentHandler):

      def __init__(self, values, fields):

          xml.sax.handler.ContentHandler.__init__(self)

@@ -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:
@@ -55,7 +53,7 @@ 

          insert = self.inserts[0]

          self.assertEqual(insert.table, 'tag_config')

          values = {

-             'arches': None,

+             'arches': '',

              'create_event': 42,

              'creator_id': 23,

              'locked': False,
@@ -67,3 +65,20 @@ 

          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)

@@ -1,3 +1,4 @@ 

+ # coding: utf-8

  from __future__ import absolute_import

  import mock

  try:
@@ -197,3 +198,40 @@ 

          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)

+ 

@@ -0,0 +1,31 @@ 

+ # coding: utf-8

+ from __future__ import absolute_import

+ try:

+     import unittest2 as unittest

+ except ImportError:

+     import unittest

+ import koji

+ 

+ class TestParseArchesString(unittest.TestCase):

+     def test_parse_valid_arches(self):

+         r = koji.parse_arches('i386', to_list=True)

+         self.assertEqual(['i386'], r)

+ 

+         r = koji.parse_arches('i386 x86_64', to_list=True)

+         self.assertEqual(['i386', 'x86_64'], r)

+ 

+         r = koji.parse_arches('i386 x86_64   ', to_list=True)

+         self.assertEqual(['i386', 'x86_64'], r)

+ 

+         r = koji.parse_arches('i386,x86_64', to_list=True)

+         self.assertEqual(['i386', 'x86_64'], r)

+ 

+     def test_parse_invalid_arches(self):

+         with self.assertRaises(koji.GenericError):

+             koji.parse_arches(u'ěšč')

+ 

+         with self.assertRaises(koji.GenericError):

+             koji.parse_arches(u'i386;x86_64')

+ 

+         with self.assertRaises(koji.GenericError):

+             koji.parse_arches(u'i386,x86_64', strict=True)

rebased onto 79f2f492d7818c0260a6249dc8664493ac66bc1d

5 years ago

As there is similar function for CLI, I've merged them together and moved to base lib.

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

5 years ago

rebased onto b08d50f

5 years ago

Found bug in parsing None (fixed in rebase).

pretty please pagure-ci rebuild

5 years ago

1 new commit added

  • validate archs in addHost
4 years ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done

4 years ago

Commit 30a1b65 fixes this pull-request

Pull-Request has been merged by mikem

4 years ago