Improve symlink handling

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)

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),

src/libfetchers/input-accessor.cc

@@ -87,6 +87,14 @@ void InputAccessor::dumpPath(
     dump(path);
 }
 
+std::optional<InputAccessor::Stat> 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};
+}
+
 }

src/libfetchers/input-accessor.hh

@@ -31,6 +31,8 @@ struct InputAccessor
 
     virtual Stat lstat(const CanonPath & path) = 0;
 
+    std::optional<Stat> maybeLstat(const CanonPath & path);
+
     typedef std::optional<Type> DirEntry;
 
     typedef std::map<std::string, DirEntry> DirEntries;
@@ -101,6 +103,9 @@ struct SourcePath
     InputAccessor::Stat lstat() const
     {  return accessor.lstat(path); }
 
+    std::optional<InputAccessor::Stat> 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);

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 !(

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<CanonPath> 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;

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);