diff --git a/manifest_xml.py b/manifest_xml.py index d05f4d0..cd5954d 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -670,6 +670,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md for node in manifest.childNodes: if node.nodeName == 'include': name = self._reqatt(node, 'name') + msg = self._CheckLocalPath(name) + if msg: + raise ManifestInvalidPathError( + ' invalid "name": %s: %s' % (name, msg)) include_groups = '' if parent_groups: include_groups = parent_groups @@ -979,6 +983,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md reads a element from the manifest file """ name = self._reqatt(node, 'name') + msg = self._CheckLocalPath(name, dir_ok=True) + if msg: + raise ManifestInvalidPathError( + ' invalid "name": %s: %s' % (name, msg)) if parent: name = self._JoinName(parent.name, name) @@ -999,9 +1007,11 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md path = node.getAttribute('path') if not path: path = name - if path.startswith('/'): - raise ManifestParseError("project %s path cannot be absolute in %s" % - (name, self.manifestFile)) + else: + msg = self._CheckLocalPath(path, dir_ok=True) + if msg: + raise ManifestInvalidPathError( + ' invalid "path": %s: %s' % (path, msg)) rebase = XmlBool(node, 'rebase', True) sync_c = XmlBool(node, 'sync-c', False) @@ -1124,7 +1134,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md def _CheckLocalPath(path, dir_ok=False, cwd_dot_ok=False): """Verify |path| is reasonable for use in filesystem paths. - Used with & elements. + Used with & & elements. This only validates the |path| in isolation: it does not check against the current filesystem state. Thus it is suitable as a first-past in a parser. diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py index 4b54514..f69e9cf 100644 --- a/tests/test_manifest_xml.py +++ b/tests/test_manifest_xml.py @@ -24,6 +24,40 @@ import error import manifest_xml +# Invalid paths that we don't want in the filesystem. +INVALID_FS_PATHS = ( + '', + '.', + '..', + '../', + './', + 'foo/', + './foo', + '../foo', + 'foo/./bar', + 'foo/../../bar', + '/foo', + './../foo', + '.git/foo', + # Check case folding. + '.GIT/foo', + 'blah/.git/foo', + '.repo/foo', + '.repoconfig', + # Block ~ due to 8.3 filenames on Windows filesystems. + '~', + 'foo~', + 'blah/foo~', + # Block Unicode characters that get normalized out by filesystems. + u'foo\u200Cbar', +) + +# Make sure platforms that use path separators (e.g. Windows) are also +# rejected properly. +if os.path.sep != '/': + INVALID_FS_PATHS += tuple(x.replace('/', os.path.sep) for x in INVALID_FS_PATHS) + + class ManifestParseTestCase(unittest.TestCase): """TestCase for parsing manifests.""" @@ -86,38 +120,7 @@ class ManifestValidateFilePaths(unittest.TestCase): def test_bad_paths(self): """Make sure bad paths (src & dest) are rejected.""" - PATHS = ( - '', - '.', - '..', - '../', - './', - 'foo/', - './foo', - '../foo', - 'foo/./bar', - 'foo/../../bar', - '/foo', - './../foo', - '.git/foo', - # Check case folding. - '.GIT/foo', - 'blah/.git/foo', - '.repo/foo', - '.repoconfig', - # Block ~ due to 8.3 filenames on Windows filesystems. - '~', - 'foo~', - 'blah/foo~', - # Block Unicode characters that get normalized out by filesystems. - u'foo\u200Cbar', - ) - # Make sure platforms that use path separators (e.g. Windows) are also - # rejected properly. - if os.path.sep != '/': - PATHS += tuple(x.replace('/', os.path.sep) for x in PATHS) - - for path in PATHS: + for path in INVALID_FS_PATHS: self.assertRaises( error.ManifestInvalidPathError, self.check_both, path, 'a') self.assertRaises( @@ -248,51 +251,11 @@ class XmlManifestTests(ManifestParseTestCase): '' + '') - def test_project_group(self): - """Check project group settings.""" - manifest = self.getXmlManifest(""" - - - - - - -""") - self.assertEqual(len(manifest.projects), 2) - # Ordering isn't guaranteed. - result = { - manifest.projects[0].name: manifest.projects[0].groups, - manifest.projects[1].name: manifest.projects[1].groups, - } - project = manifest.projects[0] - self.assertCountEqual( - result['test-name'], - ['name:test-name', 'all', 'path:test-path']) - self.assertCountEqual( - result['extras'], - ['g1', 'g2', 'g1', 'name:extras', 'all', 'path:path']) - def test_project_set_revision_id(self): - """Check setting of project's revisionId.""" - manifest = self.getXmlManifest(""" - - - - - -""") - self.assertEqual(len(manifest.projects), 1) - project = manifest.projects[0] - project.SetRevisionId('ABCDEF') - self.assertEqual( - manifest.ToXml().toxml(), - '' + - '' + - '' + - '' + - '') +class IncludeElementTests(ManifestParseTestCase): + """Tests for .""" - def test_include_levels(self): + def test_group_levels(self): root_m = os.path.join(self.manifest_dir, 'root.xml') with open(root_m, 'w') as fp: fp.write(""" @@ -335,8 +298,133 @@ class XmlManifestTests(ManifestParseTestCase): # Check level2 proj group not removed. self.assertIn('l2g1', proj.groups) + def test_bad_name_checks(self): + """Check handling of bad name attribute.""" + def parse(name): + manifest = self.getXmlManifest(f""" + + + + + +""") + # Force the manifest to be parsed. + manifest.ToXml() -class SuperProjectTests(ManifestParseTestCase): + # Handle empty name explicitly because a different codepath rejects it. + with self.assertRaises(error.ManifestParseError): + parse('') + + for path in INVALID_FS_PATHS: + if not path: + continue + + with self.assertRaises(error.ManifestInvalidPathError): + parse(path) + + +class ProjectElementTests(ManifestParseTestCase): + """Tests for .""" + + def test_group(self): + """Check project group settings.""" + manifest = self.getXmlManifest(""" + + + + + + +""") + self.assertEqual(len(manifest.projects), 2) + # Ordering isn't guaranteed. + result = { + manifest.projects[0].name: manifest.projects[0].groups, + manifest.projects[1].name: manifest.projects[1].groups, + } + project = manifest.projects[0] + self.assertCountEqual( + result['test-name'], + ['name:test-name', 'all', 'path:test-path']) + self.assertCountEqual( + result['extras'], + ['g1', 'g2', 'g1', 'name:extras', 'all', 'path:path']) + + def test_set_revision_id(self): + """Check setting of project's revisionId.""" + manifest = self.getXmlManifest(""" + + + + + +""") + self.assertEqual(len(manifest.projects), 1) + project = manifest.projects[0] + project.SetRevisionId('ABCDEF') + self.assertEqual( + manifest.ToXml().toxml(), + '' + + '' + + '' + + '' + + '') + + def test_trailing_slash(self): + """Check handling of trailing slashes in attributes.""" + def parse(name, path): + return self.getXmlManifest(f""" + + + + + +""") + + manifest = parse('a/path/', 'foo') + self.assertEqual(manifest.projects[0].gitdir, + os.path.join(self.tempdir, '.repo/projects/foo.git')) + self.assertEqual(manifest.projects[0].objdir, + os.path.join(self.tempdir, '.repo/project-objects/a/path.git')) + + manifest = parse('a/path', 'foo/') + self.assertEqual(manifest.projects[0].gitdir, + os.path.join(self.tempdir, '.repo/projects/foo.git')) + self.assertEqual(manifest.projects[0].objdir, + os.path.join(self.tempdir, '.repo/project-objects/a/path.git')) + + def test_bad_path_name_checks(self): + """Check handling of bad path & name attributes.""" + def parse(name, path): + manifest = self.getXmlManifest(f""" + + + + + +""") + # Force the manifest to be parsed. + manifest.ToXml() + + # Verify the parser is valid by default to avoid buggy tests below. + parse('ok', 'ok') + + # Handle empty name explicitly because a different codepath rejects it. + # Empty path is OK because it defaults to the name field. + with self.assertRaises(error.ManifestParseError): + parse('', 'ok') + + for path in INVALID_FS_PATHS: + if not path or path.endswith('/'): + continue + + with self.assertRaises(error.ManifestInvalidPathError): + parse(path, 'ok') + with self.assertRaises(error.ManifestInvalidPathError): + parse('ok', path) + + +class SuperProjectElementTests(ManifestParseTestCase): """Tests for .""" def test_superproject(self):