diff --git a/docs/releases.rst b/docs/releases.rst index de70f27..f262284 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -5,6 +5,16 @@ ====================== +fmf-1.8.0 +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The symlink loop detection has been improved to correctly handle +multiple symlinks pointing to the same target directory. Previously, +only the first symlink was followed while subsequent ones were +incorrectly skipped as loops. The detection now tracks ancestor +directories per branch instead of globally across the whole tree. + + fmf-1.7.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/fmf/base.py b/fmf/base.py index 31959bb..0c1ae1a 100644 --- a/fmf/base.py +++ b/fmf/base.py @@ -104,12 +104,11 @@ def __init__(self, data, name=None, parent=None): # Special directives self._directives = dict() - # Store symlinks in while walking tree in grow() to detect - # symlink loops + # Track real paths of ancestor directories to detect symlink loops if parent is None: - self._symlinkdirs = [] + self._realpaths = set() else: - self._symlinkdirs = parent._symlinkdirs + self._realpaths = set(parent._realpaths) # Special handling for top parent if self.parent is None: @@ -710,6 +709,7 @@ def grow(self, path): if path in IGNORED_DIRECTORIES: # pragma: no cover log.debug("Ignoring '{0}' (special directory).".format(path)) return + self._realpaths.add(os.path.realpath(path)) log.info("Walking through directory {0}".format( os.path.abspath(path))) try: @@ -756,16 +756,10 @@ def grow(self, path): continue fulldir = os.path.join(dirpath, dirname) if os.path.islink(fulldir): - # According to the documentation, calling os.path.realpath - # with strict = True will raise OSError if a symlink loop - # is encountered. But it does not do that with a loop with - # more than one node fullpath = os.path.realpath(fulldir) - if fullpath in self._symlinkdirs: + if fullpath in self._realpaths: log.debug("Not entering symlink loop {}".format(fulldir)) continue - else: - self._symlinkdirs.append(fullpath) # Ignore metadata subtrees if os.path.isdir(os.path.join(path, dirname, SUFFIX)): diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index f913b6e..6236ebe 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -575,6 +575,59 @@ def test_validation_invalid_schema(self): with pytest.raises(fmf.utils.JsonSchemaError): self.wget.find('/recursion/deep').validate('invalid') + def test_symlink_shared_target(self): + """ + Multiple symlinks pointing to the same directory should all be followed + """ + + directory = tempfile.mkdtemp() + try: + Tree.init(directory) + common = os.path.join(directory, 'common') + os.mkdir(common) + with open(os.path.join(common, 'main.fmf'), 'w') as main: + main.write('execute:\n how: tmt\n') + for name in ('one', 'two'): + subdir = os.path.join(directory, name) + os.mkdir(subdir) + with open(os.path.join(subdir, 'main.fmf'), 'w') as main: + main.write(f'environment:\n VARIABLE: {name}\n') + os.symlink('../common', os.path.join(subdir, 'plan')) + + tree = Tree(directory) + one_plan = tree.find('/one/plan') + two_plan = tree.find('/two/plan') + assert one_plan is not None + assert two_plan is not None + assert one_plan.get('execute') == {'how': 'tmt'} + assert two_plan.get('execute') == {'how': 'tmt'} + assert one_plan.get('environment') == {'VARIABLE': 'one'} + assert two_plan.get('environment') == {'VARIABLE': 'two'} + finally: + rmtree(directory) + + def test_symlink_loop(self): + """ + Symlink loops should be detected and silently skipped + """ + + directory = tempfile.mkdtemp() + try: + Tree.init(directory) + subdir = os.path.join(directory, 'child') + os.mkdir(subdir) + with open(os.path.join(subdir, 'main.fmf'), 'w') as main: + main.write('key: value\n') + os.symlink('..', os.path.join(subdir, 'loop')) + + tree = Tree(directory) + child = tree.find('/child') + assert child is not None + assert child.get('key') == 'value' + assert tree.find('/child/loop') is None + finally: + rmtree(directory) + class TestRemote: """