android/git-repo
1
Fork 0
mirror of https://gerrit.googlesource.com/git-repo synced 2024-12-21 07:16:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

manifest_xml: unify bool & int parsing

Browse Source 
We've been overly lenient with boolean parsing by ignoring invalid
values as "false" even if the user didn't intend that.  Turn all
unknown values into warnings to avoid breaking existing manifests,
and unify the parsing logic in a helper to simplify.

We've been stricter about numbers, but still copying & pasting
inconsistent code.  Add a helper for this too.  For out of range
sync-j numbers (i.e. less than 1), throw a warning for now, but
mark it for future hard failures.

Change-Id: I924162b8036e6a5f1e31b6ebb24b6a26ed63712d
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/256457
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
This commit is contained in:
Mike Frysinger 2020-02-22 05:30:12 -05:00
parent 23d7dafd10
commit bb8ee7f54a
2 changed files with 127 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

125
manifest_xml.py
View File

@ -57,6 +57,60 @@ urllib.parse.uses_netloc.extend([
'rpc'])
def XmlBool(node, attr, default=None):
"""Determine boolean value of |node|'s |attr|.
Invalid values will issue a non-fatal warning.
Args:
node: XML node whose attributes we access.
attr: The attribute to access.
default: If the attribute is not set (value is empty), then use this.
Returns:
True if the attribute is a valid string representing true.
False if the attribute is a valid string representing false.
|default| otherwise.
"""
value = node.getAttribute(attr)
s = value.lower()
if s == '':
return default
elif s in {'yes', 'true', '1'}:
return True
elif s in {'no', 'false', '0'}:
return False
else:
print('warning: manifest: %s="%s": ignoring invalid XML boolean' %
(attr, value), file=sys.stderr)
return default
def XmlInt(node, attr, default=None):
"""Determine integer value of |node|'s |attr|.
Args:
node: XML node whose attributes we access.
attr: The attribute to access.
default: If the attribute is not set (value is empty), then use this.
Returns:
The number if the attribute is a valid number.
Raises:
ManifestParseError: The number is invalid.
"""
value = node.getAttribute(attr)
if not value:
return default
try:
return int(value)
except ValueError:
raise ManifestParseError('manifest: invalid %s="%s" integer' %
(attr, value))
class _Default(object):
"""Project defaults within the manifest."""
@ -757,29 +811,14 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
d.destBranchExpr = node.getAttribute('dest-branch') or None
d.upstreamExpr = node.getAttribute('upstream') or None
sync_j = node.getAttribute('sync-j')
if sync_j == '' or sync_j is None:
d.sync_j = 1
else:
d.sync_j = int(sync_j)
d.sync_j = XmlInt(node, 'sync-j', 1)
if d.sync_j <= 0:
raise ManifestParseError('%s: sync-j must be greater than 0, not "%s"' %
(self.manifestFile, d.sync_j))
sync_c = node.getAttribute('sync-c')
if not sync_c:
d.sync_c = False
else:
d.sync_c = sync_c.lower() in ("yes", "true", "1")
sync_s = node.getAttribute('sync-s')
if not sync_s:
d.sync_s = False
else:
d.sync_s = sync_s.lower() in ("yes", "true", "1")
sync_tags = node.getAttribute('sync-tags')
if not sync_tags:
d.sync_tags = True
else:
d.sync_tags = sync_tags.lower() in ("yes", "true", "1")
d.sync_c = XmlBool(node, 'sync-c', False)
d.sync_s = XmlBool(node, 'sync-s', False)
d.sync_tags = XmlBool(node, 'sync-tags', True)
return d
def _ParseNotice(self, node):
@ -856,39 +895,15 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
raise ManifestParseError("project %s path cannot be absolute in %s" %
(name, self.manifestFile))
rebase = node.getAttribute('rebase')
if not rebase:
rebase = True
else:
rebase = rebase.lower() in ("yes", "true", "1")
rebase = XmlBool(node, 'rebase', True)
sync_c = XmlBool(node, 'sync-c', False)
sync_s = XmlBool(node, 'sync-s', self._default.sync_s)
sync_tags = XmlBool(node, 'sync-tags', self._default.sync_tags)
sync_c = node.getAttribute('sync-c')
if not sync_c:
sync_c = False
else:
sync_c = sync_c.lower() in ("yes", "true", "1")
sync_s = node.getAttribute('sync-s')
if not sync_s:
sync_s = self._default.sync_s
else:
sync_s = sync_s.lower() in ("yes", "true", "1")
sync_tags = node.getAttribute('sync-tags')
if not sync_tags:
sync_tags = self._default.sync_tags
else:
sync_tags = sync_tags.lower() in ("yes", "true", "1")
clone_depth = node.getAttribute('clone-depth')
if clone_depth:
try:
clone_depth = int(clone_depth)
if clone_depth <= 0:
raise ValueError()
except ValueError:
raise ManifestParseError('invalid clone-depth %s in %s' %
(clone_depth, self.manifestFile))
clone_depth = XmlInt(node, 'clone-depth')
if clone_depth is not None and clone_depth <= 0:
raise ManifestParseError('%s: clone-depth must be greater than 0, not "%s"' %
(self.manifestFile, clone_depth))
dest_branch = node.getAttribute('dest-branch') or self._default.destBranchExpr
@ -911,7 +926,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
groups.extend(set(default_groups).difference(groups))
if self.IsMirror and node.hasAttribute('force-path'):
if node.getAttribute('force-path').lower() in ("yes", "true", "1"):
if XmlBool(node, 'force-path', False):
gitdir = os.path.join(self.topdir, '%s.git' % path)
project = Project(manifest=self,

57
tests/test_manifest_xml.py
View File

@ -20,6 +20,7 @@ from __future__ import print_function
import os
import unittest
import xml.dom.minidom
import error
import manifest_xml
@ -89,3 +90,59 @@ class ManifestValidateFilePaths(unittest.TestCase):
error.ManifestInvalidPathError, self.check_both, path, 'a')
self.assertRaises(
error.ManifestInvalidPathError, self.check_both, 'a', path)
class ValueTests(unittest.TestCase):
"""Check utility parsing code."""
def _get_node(self, text):
return xml.dom.minidom.parseString(text).firstChild
def test_bool_default(self):
"""Check XmlBool default handling."""
node = self._get_node('<node/>')
self.assertIsNone(manifest_xml.XmlBool(node, 'a'))
self.assertIsNone(manifest_xml.XmlBool(node, 'a', None))
self.assertEqual(123, manifest_xml.XmlBool(node, 'a', 123))
node = self._get_node('<node a=""/>')
self.assertIsNone(manifest_xml.XmlBool(node, 'a'))
def test_bool_invalid(self):
"""Check XmlBool invalid handling."""
node = self._get_node('<node a="moo"/>')
self.assertEqual(123, manifest_xml.XmlBool(node, 'a', 123))
def test_bool_true(self):
"""Check XmlBool true values."""
for value in ('yes', 'true', '1'):
node = self._get_node('<node a="%s"/>' % (value,))
self.assertTrue(manifest_xml.XmlBool(node, 'a'))
def test_bool_false(self):
"""Check XmlBool false values."""
for value in ('no', 'false', '0'):
node = self._get_node('<node a="%s"/>' % (value,))
self.assertFalse(manifest_xml.XmlBool(node, 'a'))
def test_int_default(self):
"""Check XmlInt default handling."""
node = self._get_node('<node/>')
self.assertIsNone(manifest_xml.XmlInt(node, 'a'))
self.assertIsNone(manifest_xml.XmlInt(node, 'a', None))
self.assertEqual(123, manifest_xml.XmlInt(node, 'a', 123))
node = self._get_node('<node a=""/>')
self.assertIsNone(manifest_xml.XmlInt(node, 'a'))
def test_int_good(self):
"""Check XmlInt numeric handling."""
for value in (-1, 0, 1, 50000):
node = self._get_node('<node a="%s"/>' % (value,))
self.assertEqual(value, manifest_xml.XmlInt(node, 'a'))
def test_int_invalid(self):
"""Check XmlInt invalid handling."""
with self.assertRaises(error.ManifestParseError):
node = self._get_node('<node a="xx"/>')
manifest_xml.XmlInt(node, 'a')