diff --git a/src/rust/engine/fs/src/glob_matching.rs b/src/rust/engine/fs/src/glob_matching.rs index 71b932738b50..403017505bc0 100644 --- a/src/rust/engine/fs/src/glob_matching.rs +++ b/src/rust/engine/fs/src/glob_matching.rs @@ -27,7 +27,7 @@ pub trait GlobMatching: VFS { /// /// TODO: Should handle symlink loops (which would exhibit as an infinite loop in expand). /// - fn canonicalize(&self, symbolic_path: PathBuf, link: &Link) -> BoxFuture, E> { + fn canonicalize(&self, symbolic_path: PathBuf, link: Link) -> BoxFuture, E> { GlobMatchingImplementation::canonicalize(self, symbolic_path, link) } @@ -121,7 +121,7 @@ trait GlobMatchingImplementation: VFS { future::ok(None).to_boxed() } else { match stat { - Stat::Link(l) => context.canonicalize(stat_symbolic_path, l), + Stat::Link(l) => context.canonicalize(stat_symbolic_path, l.clone()), Stat::Dir(d) => future::ok(Some(PathStat::dir( stat_symbolic_path.to_owned(), d.clone(), @@ -399,8 +399,7 @@ trait GlobMatchingImplementation: VFS { } } - fn canonicalize(&self, symbolic_path: PathBuf, link: &Link) -> BoxFuture, E> { - let link = link.clone(); + fn canonicalize(&self, symbolic_path: PathBuf, link: Link) -> BoxFuture, E> { // Read the link, which may result in PathGlob(s) that match 0 or 1 Path. let context = self.clone(); self diff --git a/src/rust/engine/fs/src/lib.rs b/src/rust/engine/fs/src/lib.rs index f19de176c07b..bc9dbc50227a 100644 --- a/src/rust/engine/fs/src/lib.rs +++ b/src/rust/engine/fs/src/lib.rs @@ -763,7 +763,7 @@ impl PathStatGetter for Arc { .and_then(move |maybe_stat| { match maybe_stat { // Note: This will drop PathStats for symlinks which don't point anywhere. - Some(Stat::Link(link)) => fs.canonicalize(link.0.clone(), &link), + Some(Stat::Link(link)) => fs.canonicalize(link.0.clone(), link), Some(Stat::Dir(dir)) => { future::ok(Some(PathStat::dir(dir.0.clone(), dir))).to_boxed() } diff --git a/tests/python/pants_test/engine/test_fs.py b/tests/python/pants_test/engine/test_fs.py index 68b9b037c1c0..a452f060ba29 100644 --- a/tests/python/pants_test/engine/test_fs.py +++ b/tests/python/pants_test/engine/test_fs.py @@ -8,17 +8,17 @@ import os import tarfile import unittest -from builtins import object, open, str +from builtins import open, str from contextlib import contextmanager from future.utils import PY2, text_type -from pants.base.project_tree import Dir, Link from pants.engine.fs import (EMPTY_DIRECTORY_DIGEST, Digest, DirectoryToMaterialize, FilesContent, MergedDirectories, PathGlobs, PathGlobsAndRoot, Snapshot, UrlToFetch, create_fs_rules) from pants.engine.scheduler import ExecutionError from pants.util.contextutil import http_server, temporary_dir +from pants.util.dirutil import relative_symlink from pants.util.meta import AbstractClass from pants_test.engine.scheduler_test_base import SchedulerTestBase from pants_test.test_base import TestBase @@ -30,14 +30,6 @@ from http.server import BaseHTTPRequestHandler -class DirectoryListing(object): - """TODO: See #4027.""" - - -class ReadLink(object): - """TODO: See #4027.""" - - class FSTest(TestBase, SchedulerTestBase, AbstractClass): _original_src = os.path.join(os.path.dirname(__file__), 'examples/fs_test/fs_test.tar') @@ -57,16 +49,19 @@ def specs(filespecs): else: return PathGlobs(include=filespecs) - def assert_walk_dirs(self, filespecs_or_globs, paths, ignore_patterns=None): - self.assert_walk_snapshot('dirs', filespecs_or_globs, paths, ignore_patterns=ignore_patterns) + def assert_walk_dirs(self, filespecs_or_globs, paths, **kwargs): + self.assert_walk_snapshot('dirs', filespecs_or_globs, paths, **kwargs) - def assert_walk_files(self, filespecs_or_globs, paths, ignore_patterns=None): - self.assert_walk_snapshot('files', filespecs_or_globs, paths, ignore_patterns=ignore_patterns) + def assert_walk_files(self, filespecs_or_globs, paths, **kwargs): + self.assert_walk_snapshot('files', filespecs_or_globs, paths, **kwargs) - def assert_walk_snapshot(self, field, filespecs_or_globs, paths, ignore_patterns=None): + def assert_walk_snapshot(self, field, filespecs_or_globs, paths, ignore_patterns=None, prepare=None): with self.mk_project_tree(ignore_patterns=ignore_patterns) as project_tree: scheduler = self.mk_scheduler(rules=create_fs_rules(), project_tree=project_tree) + if prepare: + prepare(project_tree) result = self.execute(scheduler, Snapshot, self.specs(filespecs_or_globs))[0] + print('>>> got Snapshot {} containing {} and {}'.format(result, result.dirs, result.files)) self.assertEqual(sorted([p.path for p in getattr(result, field)]), sorted(paths)) def assert_content(self, filespecs_or_globs, expected_content): @@ -85,17 +80,6 @@ def assert_digest(self, filespecs_or_globs, expected_files): self.assertEqual(set(expected_files), {f.path for f in result.files}) self.assertTrue(result.directory_digest.fingerprint is not None) - def assert_fsnodes(self, filespecs_or_globs, subject_product_pairs): - with self.mk_project_tree() as project_tree: - scheduler = self.mk_scheduler(rules=create_fs_rules(), project_tree=project_tree) - request = self.execute_request(scheduler, Snapshot, self.specs(filespecs_or_globs)) - - # Validate that FilesystemNodes for exactly the given subjects are reachable under this - # request. - fs_nodes = [n for n, _ in scheduler.product_graph.walk(roots=request.roots) - if type(n) is "TODO: need a new way to filter for FS intrinsics"] - self.assertEqual({(n.subject, n.product) for n in fs_nodes}, set(subject_product_pairs)) - def test_walk_literal(self): self.assert_walk_files(['4.txt'], ['4.txt']) self.assert_walk_files(['a/b/1.txt', 'a/b/2'], ['a/b/1.txt', 'a/b/2']) @@ -141,6 +125,18 @@ def test_walk_single_star(self): def test_walk_parent_link(self): self.assert_walk_files(['c.ln/../3.txt'], ['c.ln/../3.txt']) + def test_walk_escaping_symlink(self): + link = 'subdir/escaping' + dest = '../../this-is-probably-nonexistent' + def prepare(project_tree): + link_path = os.path.join(project_tree.build_root, link) + dest_path = os.path.join(project_tree.build_root, dest) + relative_symlink(dest_path, link_path) + + exc_reg = '.*While expanding link.*{}.*may not traverse outside of the buildroot.*{}.*'.format(link, dest) + with self.assertRaisesRegexp(Exception, exc_reg): + self.assert_walk_files([link], [], prepare=prepare) + def test_walk_recursive_all(self): self.assert_walk_files(['**'], ['4.txt', 'a/3.txt', @@ -231,52 +227,6 @@ def test_files_content_symlink(self): def test_files_digest_literal(self): self.assert_digest(['a/3.txt', '4.txt'], ['a/3.txt', '4.txt']) - @unittest.skip('Skipped to expedite landing #3821; see: #4027.') - def test_nodes_file(self): - self.assert_fsnodes(['4.txt'], [ - (Dir(''), DirectoryListing), - ]) - - @unittest.skip('Skipped to expedite landing #3821; see: #4027.') - def test_nodes_symlink_file(self): - self.assert_fsnodes(['c.ln/2'], [ - (Dir(''), DirectoryListing), - (Link('c.ln'), ReadLink), - (Dir('a'), DirectoryListing), - (Dir('a/b'), DirectoryListing), - ]) - self.assert_fsnodes(['d.ln/b/1.txt'], [ - (Dir(''), DirectoryListing), - (Link('d.ln'), ReadLink), - (Dir('a'), DirectoryListing), - (Dir('a/b'), DirectoryListing), - ]) - - @unittest.skip('Skipped to expedite landing #3821; see: #4027.') - def test_nodes_symlink_globbed_dir(self): - self.assert_fsnodes(['*/2'], [ - # Scandir for the root. - (Dir(''), DirectoryListing), - # Read links to determine whether they're actually directories. - (Link('c.ln'), ReadLink), - (Link('d.ln'), ReadLink), - # Scan second level destinations: `a/b` is matched via `c.ln`. - (Dir('a'), DirectoryListing), - (Dir('a/b'), DirectoryListing), - ]) - - @unittest.skip('Skipped to expedite landing #3821; see: #4027.') - def test_nodes_symlink_globbed_file(self): - self.assert_fsnodes(['d.ln/b/*.txt'], [ - # NB: Needs to scandir every Dir on the way down to track whether - # it is traversing a symlink. - (Dir(''), DirectoryListing), - # Traverse one symlink. - (Link('d.ln'), ReadLink), - (Dir('a'), DirectoryListing), - (Dir('a/b'), DirectoryListing), - ]) - def test_snapshot_from_outside_buildroot(self): with temporary_dir() as temp_dir: with open(os.path.join(temp_dir, "roland"), "w") as f: