Symlink review

.gitignore

@@ -47,6 +47,9 @@ perl/Makefile.config
 /src/libexpr/nix.tbl
 /tests/unit/libexpr/libnixexpr-tests
 
+# /src/libfetchers
+/tests/unit/libfetchers/libnixfetchers-tests
+
 # /src/libstore/
 *.gen.*
 /tests/unit/libstore/libnixstore-tests

Makefile

@@ -34,6 +34,7 @@ makefiles += \
   tests/unit/libutil-support/local.mk \
   tests/unit/libstore/local.mk \
   tests/unit/libstore-support/local.mk \
+  tests/unit/libfetchers/local.mk \
   tests/unit/libexpr/local.mk \
   tests/unit/libexpr-support/local.mk
 endif

src/libfetchers/memory-input-accessor.cc

@@ -13,6 +13,14 @@ struct MemoryInputAccessorImpl : MemoryInputAccessor, MemorySourceAccessor
             MemorySourceAccessor::addFile(path, std::move(contents))
         };
     }
+
+    SourcePath addSymlink(CanonPath path, std::string && contents) override
+    {
+        return {
+            ref(shared_from_this()),
+            MemorySourceAccessor::addSymlink(path, std::move(contents))
+        };
+    }
 };
 
 ref<MemoryInputAccessor> makeMemoryInputAccessor()

src/libfetchers/memory-input-accessor.hh

@@ -9,6 +9,7 @@ namespace nix {
 struct MemoryInputAccessor : InputAccessor
 {
     virtual SourcePath addFile(CanonPath path, std::string && contents) = 0;
+    virtual SourcePath addSymlink(CanonPath path, std::string && contents) = 0;
 };
 
 ref<MemoryInputAccessor> makeMemoryInputAccessor();

src/libutil/memory-source-accessor.cc

@@ -121,6 +121,19 @@ CanonPath MemorySourceAccessor::addFile(CanonPath path, std::string && contents)
     return path;
 }
 
+CanonPath MemorySourceAccessor::addSymlink(CanonPath path, std::string &&contents)
+{
+    auto * f = open(path, File { File::Symlink {} });
+    if (!f)
+        throw Error("file '%s' cannot be made because some parent file is not a directory", path);
+    if (auto * s = std::get_if<File::Symlink>(&f->raw))
+        s->target = std::move(contents);
+    else
+        throw Error("file '%s' is not a symbolic link", path);
+
+    return path;
+}
+
 
 using File = MemorySourceAccessor::File;
 

src/libutil/memory-source-accessor.hh

@@ -70,6 +70,7 @@ struct MemorySourceAccessor : virtual SourceAccessor
     File * open(const CanonPath & path, std::optional<File> create);
 
     CanonPath addFile(CanonPath path, std::string && contents);
+    CanonPath addSymlink(CanonPath path, std::string && contents);
 };
 
 /**

src/libutil/source-path.cc

@@ -62,11 +62,27 @@ bool SourcePath::operator<(const SourcePath & x) const
     return std::tie(*accessor, path) < std::tie(*x.accessor, x.path);
 }
 
+SourcePath SourcePath::followSymlinks() const {
+    SourcePath path = *this;
+    unsigned int followCount = 0, maxFollow = 1000;
+
+    /* If `path' is a symlink, follow it.  This is so that relative
+       path references work. */
+    while (true) {
+        // Basic cycle/depth limit to avoid infinite loops.
+        if (++followCount >= maxFollow)
+            throw Error("too many levels of symbolic links while traversing the path '%s'; assuming it leads to a cycle after following %d indirections", this->to_string(), maxFollow);
+        if (path.lstat().type != InputAccessor::tSymlink) break;
+        path = {path.accessor, CanonPath(path.readLink(), path.path.parent().value_or(CanonPath::root))};
+    }
+    return path;
+}
+
 SourcePath SourcePath::resolveSymlinks() const
 {
     auto res = SourcePath(accessor);
 
-    int linksAllowed = 1024;
+    int linksAllowed = 1000;
 
     std::list<std::string> todo;
     for (auto & c : path)

src/libutil/source-path.hh

@@ -103,10 +103,19 @@ struct SourcePath
 
     /**
      * Resolve any symlinks in this `SourcePath` (including its
-     * parents). The result is a `SourcePath` in which no element is a
-     * symlink.
+     * parents).
+     *
+     * @return A `SourcePath` in which no element is a symlink.
      */
     SourcePath resolveSymlinks() const;
+
+    /**
+     * If this `SourcePath` is a symlink, resolve it, but do not resolve
+     * symlinks in its parent paths.
+     *
+     * @return A `SourcePath` in which the final element is not a symlink.
+     */
+    SourcePath followSymlinks() const;
 };
 
 std::ostream & operator << (std::ostream & str, const SourcePath & path);

tests/unit/libfetchers/input-accessor.cc

@@ -0,0 +1,30 @@
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <input-accessor.hh>
+#include <memory-input-accessor.hh>
+#include "terminal.hh"
+
+namespace nix {
+
+TEST(SourcePath, followSymlinks_cycle) {
+    auto fs = makeMemoryInputAccessor();
+    fs->addSymlink({"origin", CanonPath::root}, "a");
+    fs->addSymlink({"a", CanonPath::root}, "b");
+    fs->addSymlink({"b", CanonPath::root}, "a");
+
+    ASSERT_TRUE(fs->pathExists({"a", CanonPath::root}));
+    SourcePath origin { fs, CanonPath { "/origin" } };
+    try {
+        origin.followSymlinks();
+        ASSERT_TRUE(false);
+    } catch (const Error &e) {
+        auto msg = filterANSIEscapes(e.what(), true);
+        // EXPECT_THAT(msg, ("too many levels of symbolic links"));
+        EXPECT_THAT(msg, testing::HasSubstr("too many levels of symbolic links"));
+        EXPECT_THAT(msg, testing::HasSubstr("«unknown»/origin'"));
+        EXPECT_THAT(msg, testing::HasSubstr("assuming it leads to a cycle after following 1000 indirections"));
+    }
+}
+
+}

tests/unit/libfetchers/local.mk

@@ -0,0 +1,32 @@
+check: libfetchers-tests_RUN
+
+programs += libfetchers-tests
+
+libfetchers-tests_NAME = libnixfetchers-tests
+
+libfetchers-tests_ENV := _NIX_TEST_UNIT_DATA=$(d)/data
+
+libfetchers-tests_DIR := $(d)
+
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libfetchers-tests_INSTALL_DIR := $(checkbindir)
+else
+  libfetchers-tests_INSTALL_DIR :=
+endif
+
+libfetchers-tests_SOURCES := $(wildcard $(d)/*.cc)
+
+libfetchers-tests_EXTRA_INCLUDES = \
+    -I tests/unit/libstore-support \
+    -I tests/unit/libutil-support \
+    -I src/libfetchers \
+    -I src/libstore \
+    -I src/libutil
+
+libfetchers-tests_CXXFLAGS += $(libfetchers-tests_EXTRA_INCLUDES)
+
+libfetchers-tests_LIBS = \
+    libstore-test-support libutil-test-support \
+    libfetchers libstore libutil
+
+libfetchers-tests_LDFLAGS := -lrapidcheck $(GTEST_LIBS)