From b14f88e0d46c61280a69da9559cf54cbce058eb5 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Thu, 7 Mar 2024 04:06:03 +0100 Subject: [PATCH] Merge pull request #9985 from alois31/symlink-resolution Restore `builtins.pathExists` behavior on broken symlinks (cherry picked from commit d53c8901ef7f2033855dd99063522e3d56a19dab) === note that this variant differs markedly from the source commit because we haven't endured quite as much lazy trees. Change-Id: I0facf282f21fe0db4134be5c65a8368c1b3a06fc --- src/libexpr/primops.cc | 11 ++++--- src/libutil/source-path.cc | 22 +++++++------ src/libutil/source-path.hh | 31 ++++++++++++++++--- .../functional/lang/eval-okay-pathexists.nix | 3 ++ .../functional/lang/symlink-resolution/broken | 1 + 5 files changed, 50 insertions(+), 18 deletions(-) create mode 120000 tests/functional/lang/symlink-resolution/broken diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 1a961582f..c4fdc6098 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1540,11 +1540,12 @@ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args, || arg.str().ends_with("/.")); try { - auto checked = state.checkSourcePath(path); - auto exists = checked.pathExists(); - if (exists && mustBeDir) { - exists = checked.lstat().type == InputAccessor::tDirectory; - } + auto checked = state + .checkSourcePath(path) + .resolveSymlinks(mustBeDir ? SymlinkResolution::Full : SymlinkResolution::Ancestors); + + auto st = checked.maybeLstat(); + auto exists = st && (!mustBeDir || st->type == InputAccessor::tDirectory); v.mkBool(exists); } catch (SysError & e) { /* Don't give away info from errors while canonicalising diff --git a/src/libutil/source-path.cc b/src/libutil/source-path.cc index b6563ee90..3ccbca06b 100644 --- a/src/libutil/source-path.cc +++ b/src/libutil/source-path.cc @@ -56,7 +56,7 @@ InputAccessor::DirEntries SourcePath::readDirectory() const return res; } -SourcePath SourcePath::resolveSymlinks() const +SourcePath SourcePath::resolveSymlinks(SymlinkResolution mode) const { SourcePath res(CanonPath::root); @@ -66,6 +66,8 @@ SourcePath SourcePath::resolveSymlinks() const for (auto & c : path) todo.push_back(std::string(c)); + bool resolve_last = mode == SymlinkResolution::Full; + while (!todo.empty()) { auto c = *todo.begin(); todo.pop_front(); @@ -75,14 +77,16 @@ SourcePath SourcePath::resolveSymlinks() const res.path.pop(); else { res.path.push(c); - if (auto st = res.maybeLstat(); st && st->type == InputAccessor::tSymlink) { - if (!linksAllowed--) - throw Error("infinite symlink recursion in path '%s'", path); - auto target = res.readLink(); - res.path.pop(); - if (hasPrefix(target, "/")) - res.path = CanonPath::root; - todo.splice(todo.begin(), tokenizeString>(target, "/")); + if (resolve_last || !todo.empty()) { + if (auto st = res.maybeLstat(); st && st->type == InputAccessor::tSymlink) { + if (!linksAllowed--) + throw Error("infinite symlink recursion in path '%s'", path); + auto target = res.readLink(); + res.path.pop(); + if (hasPrefix(target, "/")) + res.path = CanonPath::root; + todo.splice(todo.begin(), tokenizeString>(target, "/")); + } } } } diff --git a/src/libutil/source-path.hh b/src/libutil/source-path.hh index 54698de77..03cc998e3 100644 --- a/src/libutil/source-path.hh +++ b/src/libutil/source-path.hh @@ -12,6 +12,26 @@ namespace nix { +/** + * Note there is a decent chance this type soon goes away because the problem is solved another way. + * See the discussion in https://github.com/NixOS/nix/pull/9985. + */ +enum class SymlinkResolution { + /** + * Resolve symlinks in the ancestors only. + * + * Only the last component of the result is possibly a symlink. + */ + Ancestors, + + /** + * Resolve symlinks fully, realpath(3)-style. + * + * No component of the result will be a symlink. + */ + Full, +}; + /** * An abstraction for accessing source files during * evaluation. Currently, it's just a wrapper around `CanonPath` that @@ -121,11 +141,14 @@ struct SourcePath } /** - * Resolve any symlinks in this `SourcePath` (including its - * parents). The result is a `SourcePath` in which no element is a - * symlink. + * Resolve any symlinks in this `SourcePath` according to the + * given resolution mode. + * + * @param mode might only be a temporary solution for this. + * See the discussion in https://github.com/NixOS/nix/pull/9985. */ - SourcePath resolveSymlinks() const; + SourcePath resolveSymlinks( + SymlinkResolution mode = SymlinkResolution::Full) const; }; std::ostream & operator << (std::ostream & str, const SourcePath & path); diff --git a/tests/functional/lang/eval-okay-pathexists.nix b/tests/functional/lang/eval-okay-pathexists.nix index c5e7a62de..99253d764 100644 --- a/tests/functional/lang/eval-okay-pathexists.nix +++ b/tests/functional/lang/eval-okay-pathexists.nix @@ -27,3 +27,6 @@ builtins.pathExists (./lib.nix) && !builtins.pathExists (builtins.toPath (builtins.toString ./bla.nix)) && builtins.pathExists ./lib.nix && !builtins.pathExists ./bla.nix +&& builtins.pathExists ./symlink-resolution/foo/overlays/overlay.nix +&& builtins.pathExists ./symlink-resolution/broken +&& builtins.pathExists (builtins.toString ./symlink-resolution/foo/overlays + "/.") diff --git a/tests/functional/lang/symlink-resolution/broken b/tests/functional/lang/symlink-resolution/broken new file mode 120000 index 000000000..e07da690b --- /dev/null +++ b/tests/functional/lang/symlink-resolution/broken @@ -0,0 +1 @@ +nonexistent \ No newline at end of file