From 5e3799bb69e6d2687b83e20bd16357386e9347b6 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 19 Nov 2023 11:04:06 +0100 Subject: [PATCH] followSymlinks: Improve error message --- .gitignore | 3 ++ Makefile | 1 + src/libfetchers/input-accessor.cc | 2 +- src/libfetchers/memory-input-accessor.cc | 8 +++++ src/libfetchers/memory-input-accessor.hh | 1 + src/libfetchers/tests/input-accessor.cc | 29 ++++++++++++++++++ src/libfetchers/tests/local.mk | 39 ++++++++++++++++++++++++ src/libutil/memory-source-accessor.cc | 13 ++++++++ src/libutil/memory-source-accessor.hh | 1 + 9 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 src/libfetchers/tests/input-accessor.cc create mode 100644 src/libfetchers/tests/local.mk diff --git a/.gitignore b/.gitignore index 04d96ca2c09..64779849148 100644 --- a/.gitignore +++ b/.gitignore @@ -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 diff --git a/Makefile b/Makefile index 4f4ac0c6e29..e6f3d63e7d6 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/src/libfetchers/input-accessor.cc b/src/libfetchers/input-accessor.cc index e5e3c1d7242..bf24a5298a7 100644 --- a/src/libfetchers/input-accessor.cc +++ b/src/libfetchers/input-accessor.cc @@ -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))}; } diff --git a/src/libfetchers/memory-input-accessor.cc b/src/libfetchers/memory-input-accessor.cc index 057f3e37f73..2c35ff8b4dc 100644 --- a/src/libfetchers/memory-input-accessor.cc +++ b/src/libfetchers/memory-input-accessor.cc @@ -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 makeMemoryInputAccessor() diff --git a/src/libfetchers/memory-input-accessor.hh b/src/libfetchers/memory-input-accessor.hh index b75b02bfd61..5fb90466d0b 100644 --- a/src/libfetchers/memory-input-accessor.hh +++ b/src/libfetchers/memory-input-accessor.hh @@ -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 makeMemoryInputAccessor(); diff --git a/src/libfetchers/tests/input-accessor.cc b/src/libfetchers/tests/input-accessor.cc new file mode 100644 index 00000000000..30106c117dd --- /dev/null +++ b/src/libfetchers/tests/input-accessor.cc @@ -0,0 +1,29 @@ +#include "../input-accessor.hh" +#include "../memory-input-accessor.hh" +#include "gmock/gmock.h" +#include +#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")); + } +} + +} \ No newline at end of file diff --git a/src/libfetchers/tests/local.mk b/src/libfetchers/tests/local.mk new file mode 100644 index 00000000000..f372f8466e8 --- /dev/null +++ b/src/libfetchers/tests/local.mk @@ -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) diff --git a/src/libutil/memory-source-accessor.cc b/src/libutil/memory-source-accessor.cc index 78a4dd29815..ef892a3697f 100644 --- a/src/libutil/memory-source-accessor.cc +++ b/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(&f->raw)) + s->target = std::move(contents); + else + throw Error("file '%s' is not a symbolic link", path); + + return path; +} + using File = MemorySourceAccessor::File; diff --git a/src/libutil/memory-source-accessor.hh b/src/libutil/memory-source-accessor.hh index b908f3713c0..42fa9bea3c7 100644 --- a/src/libutil/memory-source-accessor.hh +++ b/src/libutil/memory-source-accessor.hh @@ -70,6 +70,7 @@ struct MemorySourceAccessor : virtual SourceAccessor File * open(const CanonPath & path, std::optional create); CanonPath addFile(CanonPath path, std::string && contents); + CanonPath addSymlink(CanonPath path, std::string && contents); }; /**