Skip to content

Commit

Permalink
followSymlinks: Improve error message
Browse files Browse the repository at this point in the history
  • Loading branch information
roberth committed Nov 19, 2023
1 parent 2fc6d09 commit 5e3799b
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ perl/Makefile.config
/src/libexpr/nix.tbl
/src/libexpr/tests/libnixexpr-tests

# /src/libfetchers
/src/libfetchers/tests/libnixfetchers-tests

# /src/libstore/
*.gen.*
/src/libstore/tests/libnixstore-tests
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ UNIT_TEST_ENV = _NIX_TEST_UNIT_DATA=unit-test-data
makefiles += \
src/libutil/tests/local.mk \
src/libstore/tests/local.mk \
src/libfetchers/tests/local.mk \
src/libexpr/tests/local.mk
endif

Expand Down
2 changes: 1 addition & 1 deletion src/libfetchers/input-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ SourcePath SourcePath::followSymlinks() const {
while (true) {
// Basic cycle/depth limit to avoid infinite loops.
if (++followCount >= maxFollow)
throw Error("too many symbolic links encountered while traversing the path '%s'", path);
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))};
}
Expand Down
8 changes: 8 additions & 0 deletions src/libfetchers/memory-input-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,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()
Expand Down
1 change: 1 addition & 0 deletions src/libfetchers/memory-input-accessor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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();
Expand Down
29 changes: 29 additions & 0 deletions src/libfetchers/tests/input-accessor.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#include "../input-accessor.hh"
#include "../memory-input-accessor.hh"
#include "gmock/gmock.h"
#include <gtest/gtest.h>
#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->root() + "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("'/origin'"));
EXPECT_THAT(msg, testing::HasSubstr("assuming it leads to a cycle after following 1000 indirections"));
}
}

}
39 changes: 39 additions & 0 deletions src/libfetchers/tests/local.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
check: libfetchers-tests-exe_RUN

programs += libfetchers-tests-exe

libfetchers-tests-exe_NAME = libnixfetchers-tests

libfetchers-tests-exe_DIR := $(d)

ifeq ($(INSTALL_UNIT_TESTS), yes)
libfetchers-tests-exe_INSTALL_DIR := $(checkbindir)
else
libfetchers-tests-exe_INSTALL_DIR :=
endif


libfetchers-tests-exe_LIBS = libfetchers-tests libstore


libfetchers-tests-exe_LDFLAGS := $(GTEST_LIBS)

libraries += libfetchers-tests

libfetchers-tests_NAME = libnixfetchers-tests

libfetchers-tests_DIR := $(d)

ifeq ($(INSTALL_UNIT_TESTS), yes)
libfetchers-tests_INSTALL_DIR := $(checklibdir)
else
libfetchers-tests_INSTALL_DIR :=
endif

libfetchers-tests_SOURCES := $(wildcard $(d)/*.cc)

libfetchers-tests_CXXFLAGS += -I src/libfetchers -I src/libutil -I src/libstore

libfetchers-tests_LIBS = libutil-tests libstore-tests libutil libstore libfetchers

libfetchers-tests_LDFLAGS := -lrapidcheck $(GTEST_LIBS)
13 changes: 13 additions & 0 deletions src/libutil/memory-source-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions src/libutil/memory-source-accessor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

/**
Expand Down

0 comments on commit 5e3799b

Please sign in to comment.