PR#180: reject builds with hyphens in the version or release - koji

koji

#180 reject builds with hyphens in the version or release

Merged 7 years ago by mikem. Opened 7 years ago by mikem.

https://github.com/mikem23/koji-playground.git reject-hyphens into master

add unit tests for check_NVR[A] functions

Mike McLean • 7 years ago

e1de395

fix cut and paste errors

Mike McLean • 7 years ago

3022670

assert basic nvr sanity for cg imports

Mike McLean • 7 years ago

b481229

hub/kojihub.py

file modified

		`@@ -4975,6 +4975,8 @@`
		`datetime.datetime.fromtimestamp(float(metadata['build']['end_time'])).isoformat(' ')`
		`self.buildinfo = buildinfo`

		`+ koji.check_NVR(buildinfo, strict=True)`
		`+`
		`# get typeinfo`
		`b_extra = self.metadata['build'].get('extra', {})`
		`typeinfo = b_extra.get('typeinfo', {})`
		`@@ -5178,6 +5180,8 @@`
		`raise koji.GenericError("Missing buildroot metadata for id %(buildroot_id)r" % fileinfo)`
		`if fileinfo['type'] not in ['rpm', 'log']:`
		`self.prep_archive(fileinfo)`
		`+ if fileinfo['type'] == 'rpm':`
		`+ koji.check_NVRA(fileinfo['filename'], strict=True)`
		`outputs.append(fileinfo)`
		`self.prepped_outputs = outputs`

koji/__init__.py

file modified

+60

		`@@ -916,6 +916,66 @@`
		`ret['location'] = location`
		`return ret`

		`+`
		`+ def check_NVR(nvr, strict=False):`
		`+ """Perform basic validity checks on an NVR`
		`+`
		`+ nvr may be a string or a dictionary with keys name, version, and release`
		`+`
		`+ This function only performs minimal, basic checking. It does not enforce`
		`+ the sort of constraints that a project might have in their packaging`
		`+ guidelines.`
		`+ """`
		`+`
		`+ try:`
		`+ return _check_NVR(nvr)`
		`+ except GenericError:`
		`+ if strict:`
		`+ raise`
		`+ else:`
		`+ return False`
		`+`
		`+ def _check_NVR(nvr):`
		`+ if isinstance(nvr, basestring):`
		`+ nvr = parse_NVR(nvr)`
		`+ if '-' in nvr['version']:`
		`+ raise GenericError('The "-" character not allowed in version field')`
		`+ if '-' in nvr['release']:`
		`+ raise GenericError('The "-" character not allowed in release field')`
		`+ # anything else?`
		`+ return True`
		`+`
		`+`
		`+ def check_NVRA(nvra, strict=False):`
		`+ """Perform basic validity checks on an NVRA`
		`+`
		`+ nvra may be a string or a dictionary with keys name, version, and release`
		`+`
		`+ This function only performs minimal, basic checking. It does not enforce`
		`+ the sort of constraints that a project might have in their packaging`
		`+ guidelines.`
		`+ """`
		`+ try:`
		`+ return _check_NVRA(nvra)`
		`+ except GenericError:`
		`+ if strict:`
		`+ raise`
		`+ else:`
		`+ return False`
		`+`
		`+`
		`+ def _check_NVRA(nvra):`
		`+ if isinstance(nvra, basestring):`
		`+ nvra = parse_NVRA(nvra)`
		`+ if '-' in nvra['version']:`
		`+ raise GenericError('The "-" character not allowed in version field')`
		`+ if '-' in nvra['release']:`
		`+ raise GenericError('The "-" character not allowed in release field')`
		`+ if '.' in nvra['arch']:`
		`+ raise GenericError('The "." character not allowed in arch field')`
		`+ return True`
		`+`
		`+`
		`def is_debuginfo(name):`
		`"""Determines if an rpm is a debuginfo rpm, based on name"""`
		`if name.endswith('-debuginfo') or name.find('-debuginfo-') != -1:`

tests/test_parsers.py

file modified

+44

		`@@ -63,5 +63,49 @@`
		`self.assertEqual(ret['arch'], "src")`
		`self.assertEqual(ret['src'], True)`

		`+ def test_check_NVR(self):`
		`+ """Test the check_NVR function"""`
		`+ good = [`
		`+ "name-version-release",`
		`+ "fnord-5.23-17",`
		`+ {'name': 'foo', 'version': '2.2.2', 'release': '1.1'},`
		`+ ]`
		`+ bad = [`
		`+ "this is not an NVR",`
		`+ {'name': 'foo', 'version': '2.2.2-a', 'release': '1.1'},`
		`+ {'name': 'foo', 'version': '2.2.2', 'release': '1.1-b'},`
		`+ ]`
		`+ for value in good:`
		`+ self.assertEqual(koji.check_NVR(value), True)`
		`+ for value in bad:`
		`+ self.assertEqual(koji.check_NVR(value), False)`
		`+ self.assertRaises(koji.GenericError,`
		`+ koji.check_NVR, value, strict=True)`
		`+`
		`+ def test_check_NVRA(self):`
		`+ """Test the check_NVRA function"""`
		`+ good = [`
		`+ "name-version-release.arch",`
		`+ "fnord-5.23-17.x86_64",`
		`+ {'name': 'foo', 'version': '2.2.2', 'release': '1.1',`
		`+ 'arch': 'i686'},`
		`+ ]`
		`+ bad = [`
		`+ "this is not an NVRA",`
		`+ "fnord-5.23-17",`
		`+ {'name': 'foo', 'version': '2.2.2-a', 'release': '1.1',`
		`+ 'arch': 'ppc64'},`
		`+ {'name': 'foo', 'version': '2.2.2', 'release': '1.1-b',`
		`+ 'arch': 'x86_64'},`
		`+ {'name': 'foo', 'version': '2.2.2', 'release': '1.1',`
		`+ 'arch': 'x.86.64'},`
		`+ ]`
		`+ for value in good:`
		`+ self.assertEqual(koji.check_NVRA(value), True)`
		`+ for value in bad:`
		`+ self.assertEqual(koji.check_NVRA(value), False)`
		`+ self.assertRaises(koji.GenericError,`
		`+ koji.check_NVRA, value, strict=True)`
		`+`
		`if __name__ == '__main__':`
		`unittest.main()`

no initial comment

mikem commented 7 years ago

rpm itself is pretty wide open about the characters in the version or release. However, in order to sanely parse nvr and nvra strings, koji must assume that they do not contain hyphens. If a build like this is imported, it causes numerous problems in the UI, so better to block them.

In the future, we can expand these sanity checks, but starting with the most basic for now.