From df2aa2969031affe816cb7ac5b36edc39fa7322b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 17 May 2022 13:56:26 +0200 Subject: [PATCH] Improve symlink handling --- src/libexpr/parser.y | 2 +- src/libexpr/primops.cc | 3 +- src/libfetchers/input-accessor.cc | 46 +++++++++++++++++++++++++------ src/libfetchers/input-accessor.hh | 7 +++++ src/libutil/canon-path.cc | 5 ---- src/libutil/canon-path.hh | 6 ++-- src/nix-env/nix-env.cc | 4 +-- 7 files changed, 51 insertions(+), 22 deletions(-) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 8dbf7fd39..5f5222494 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -680,7 +680,7 @@ SourcePath resolveExprPath(const SourcePath & path) { /* If `path' is a symlink, follow it. This is so that relative path references work. */ - SourcePath path2 { path.accessor, path.path.resolveSymlinks() }; + auto path2 = path.resolveSymlinks(); /* If `path' refers to a directory, append `/default.nix'. */ if (path2.lstat().type == InputAccessor::tDirectory) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3d0601e25..83e1ab6e1 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1376,7 +1376,8 @@ static void prim_storePath(EvalState & state, const PosIdx pos, Value * * args, /* Resolve symlinks in ‘path’, unless ‘path’ itself is a symlink directly in the store. The latter condition is necessary so e.g. nix-push does the right thing. */ - if (!state.store->isStorePath(path.abs())) path = path.resolveSymlinks(); + if (!state.store->isStorePath(path.abs())) + path = CanonPath(canonPath(path.abs(), true)); if (!state.store->isInStore(path.abs())) throw EvalError({ .msg = hintfmt("path '%1%' is not in the Nix store", path), diff --git a/src/libfetchers/input-accessor.cc b/src/libfetchers/input-accessor.cc index a64678855..e448b42b5 100644 --- a/src/libfetchers/input-accessor.cc +++ b/src/libfetchers/input-accessor.cc @@ -87,6 +87,14 @@ void InputAccessor::dumpPath( dump(path); } +std::optional InputAccessor::maybeLstat(const CanonPath & path) +{ + // FIXME: merge these into one operation. + if (!pathExists(path)) + return {}; + return lstat(path); +} + std::string InputAccessor::showPath(const CanonPath & path) { return "/virtual/" + std::to_string(number) + path.abs(); @@ -157,14 +165,7 @@ struct FSInputAccessorImpl : FSInputAccessor CanonPath makeAbsPath(const CanonPath & path) { - // FIXME: resolve symlinks in 'path' and check that any - // intermediate path is allowed. - auto p = root + path; - try { - return p.resolveSymlinks(); - } catch (Error &) { - return p; - } + return root + path; } void checkAllowed(const CanonPath & absPath) override @@ -239,7 +240,10 @@ struct MemoryInputAccessorImpl : MemoryInputAccessor Stat lstat(const CanonPath & path) override { - throw UnimplementedError("MemoryInputAccessor::lstat"); + auto i = files.find(path); + if (i != files.end()) + return Stat { .type = tRegular, .isExecutable = false }; + throw Error("file '%s' does not exist", path); } DirEntries readDirectory(const CanonPath & path) override @@ -275,4 +279,28 @@ SourcePath SourcePath::parent() const return {accessor, std::move(*p)}; } +SourcePath SourcePath::resolveSymlinks() const +{ + CanonPath res("/"); + + for (auto & component : path) { + res.push(component); + while (true) { + if (auto st = accessor.maybeLstat(res)) { + if (st->type != InputAccessor::tSymlink) break; + auto target = accessor.readLink(res); + if (hasPrefix(target, "/")) + res = CanonPath(target); + else { + res.pop(); + res.extend(CanonPath(target)); + } + } else + break; + } + } + + return {accessor, res}; +} + } diff --git a/src/libfetchers/input-accessor.hh b/src/libfetchers/input-accessor.hh index 6f4c3ef6b..df5fd8e7a 100644 --- a/src/libfetchers/input-accessor.hh +++ b/src/libfetchers/input-accessor.hh @@ -31,6 +31,8 @@ struct InputAccessor virtual Stat lstat(const CanonPath & path) = 0; + std::optional maybeLstat(const CanonPath & path); + typedef std::optional DirEntry; typedef std::map DirEntries; @@ -101,6 +103,9 @@ struct SourcePath InputAccessor::Stat lstat() const { return accessor.lstat(path); } + std::optional maybeLstat() const + { return accessor.maybeLstat(path); } + InputAccessor::DirEntries readDirectory() const { return accessor.readDirectory(path); } @@ -132,6 +137,8 @@ struct SourcePath { return std::tie(accessor, path) < std::tie(x.accessor, x.path); } + + SourcePath resolveSymlinks() const; }; std::ostream & operator << (std::ostream & str, const SourcePath & path); diff --git a/src/libutil/canon-path.cc b/src/libutil/canon-path.cc index e854ad950..7aeabe8d9 100644 --- a/src/libutil/canon-path.cc +++ b/src/libutil/canon-path.cc @@ -26,11 +26,6 @@ void CanonPath::pop() path.resize(std::max((size_t) 1, slash)); } -CanonPath CanonPath::resolveSymlinks() const -{ - return CanonPath(unchecked_t(), canonPath(abs(), true)); -} - bool CanonPath::isWithin(const CanonPath & parent) const { return !( diff --git a/src/libutil/canon-path.hh b/src/libutil/canon-path.hh index 62333b949..b1be0341e 100644 --- a/src/libutil/canon-path.hh +++ b/src/libutil/canon-path.hh @@ -91,8 +91,8 @@ public: } }; - Iterator begin() { return Iterator(rel()); } - Iterator end() { return Iterator(rel().substr(path.size() - 1)); } + Iterator begin() const { return Iterator(rel()); } + Iterator end() const { return Iterator(rel().substr(path.size() - 1)); } std::optional parent() const; @@ -136,8 +136,6 @@ public: return i == path.end() && j != x.path.end(); } - CanonPath resolveSymlinks() const; - /* Return true if `this` is equal to `parent` or a child of `parent`. */ bool isWithin(const CanonPath & parent) const; diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 982dc785a..d7f7eb7ac 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -119,7 +119,7 @@ static void getAllExprs(EvalState & state, InputAccessor::Stat st; try { - st = path2.accessor.lstat(path2.path.resolveSymlinks()); + st = path2.resolveSymlinks().lstat(); } catch (Error &) { continue; // ignore dangling symlinks in ~/.nix-defexpr } @@ -158,7 +158,7 @@ static void getAllExprs(EvalState & state, static void loadSourceExpr(EvalState & state, const SourcePath & path, Value & v) { - auto st = path.accessor.lstat(path.path.resolveSymlinks()); + auto st = path.resolveSymlinks().lstat(); if (isNixExpr(path, st)) state.evalFile(path, v);