diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 504063da0..903fe3e96 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1269,7 +1269,7 @@ void DerivationBuilderImpl::writeBuilderFile(const std::string & name, std::stri openat(tmpDirFd.get(), name.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; if (!fd) throw SysError("creating file %s", PathFmt(path)); - writeFile(fd, path, contents); + writeFile(fd.get(), contents); chownToBuilder(fd.get(), path); } diff --git a/src/libutil-tests/file-system-at.cc b/src/libutil-tests/file-system-at.cc index e60392693..fc387fe80 100644 --- a/src/libutil-tests/file-system-at.cc +++ b/src/libutil-tests/file-system-at.cc @@ -84,14 +84,18 @@ TEST(openFileEnsureBeneathNoSymlinks, works) dirSink.createDirectory(CanonPath("d")); dirSink.createSymlink(CanonPath("c"), "./d"); }); +#ifdef _WIN32 + EXPECT_THROW(sink.createDirectory(CanonPath("a/b/c/e")), SymlinkNotAllowed); +#else // FIXME: This still follows symlinks on Unix (incorrectly succeeds) sink.createDirectory(CanonPath("a/b/c/e")); +#endif // Test that symlinks in intermediate path are detected during nested operations - ASSERT_THROW( + EXPECT_THROW( sink.createDirectory( CanonPath("a/b/c/f"), [](FileSystemObjectSink & dirSink, const CanonPath & relPath) {}), SymlinkNotAllowed); - ASSERT_THROW( + EXPECT_THROW( sink.createRegularFile( CanonPath("a/b/c/regular"), [](CreateRegularFileSink & crf) { crf("some contents"); }), SymlinkNotAllowed); @@ -100,7 +104,7 @@ TEST(openFileEnsureBeneathNoSymlinks, works) AutoCloseFD dirFd = openDirectory(tmpDir); // Helper to open files with platform-specific arguments - auto openRead = [&](std::string_view path) -> Descriptor { + auto openRead = [&](std::string_view path) -> AutoCloseFD { return openFileEnsureBeneathNoSymlinks( dirFd.get(), CanonPath(path), @@ -114,7 +118,7 @@ TEST(openFileEnsureBeneathNoSymlinks, works) ); }; - auto openReadDir = [&](std::string_view path) -> Descriptor { + auto openReadDir = [&](std::string_view path) -> AutoCloseFD { return openFileEnsureBeneathNoSymlinks( dirFd.get(), CanonPath(path), @@ -128,7 +132,7 @@ TEST(openFileEnsureBeneathNoSymlinks, works) ); }; - auto openCreateExclusive = [&](std::string_view path) -> Descriptor { + auto openCreateExclusive = [&](std::string_view path) -> AutoCloseFD { return openFileEnsureBeneathNoSymlinks( dirFd.get(), CanonPath(path), @@ -154,19 +158,19 @@ TEST(openFileEnsureBeneathNoSymlinks, works) #if !defined(_WIN32) && !defined(__CYGWIN__) // This returns ELOOP on cygwin when O_NOFOLLOW is used - EXPECT_EQ(openCreateExclusive("a/broken_symlink"), INVALID_DESCRIPTOR); + EXPECT_FALSE(openCreateExclusive("a/broken_symlink")); /* Sanity check, no symlink shenanigans and behaves the same as regular openat with O_EXCL | O_CREAT. */ EXPECT_EQ(errno, EEXIST); #endif EXPECT_THROW(openCreateExclusive("a/absolute_symlink/broken_symlink"), SymlinkNotAllowed); // Test invalid paths - EXPECT_EQ(openRead("c/d/regular/a"), INVALID_DESCRIPTOR); - EXPECT_EQ(openReadDir("c/d/regular"), INVALID_DESCRIPTOR); + EXPECT_FALSE(openRead("c/d/regular/a")); + EXPECT_FALSE(openReadDir("c/d/regular")); // Test valid paths work - EXPECT_TRUE(AutoCloseFD{openRead("c/d/regular")}); - EXPECT_TRUE(AutoCloseFD{openCreateExclusive("a/regular")}); + EXPECT_TRUE(openRead("c/d/regular")); + EXPECT_TRUE(openCreateExclusive("a/regular")); } } // namespace nix diff --git a/src/libutil-tests/file-system.cc b/src/libutil-tests/file-system.cc index 6bfcb7d0c..eaf62f190 100644 --- a/src/libutil-tests/file-system.cc +++ b/src/libutil-tests/file-system.cc @@ -16,10 +16,12 @@ using namespace std::string_view_literals; #ifdef _WIN32 # define FS_SEP L"\\" -# define FS_ROOT L"C:" FS_SEP // Need a mounted one, C drive is likely +# define FS_ROOT_NO_TRAILING_SLASH L"C:" // Need a mounted one, C drive is likely +# define FS_ROOT FS_ROOT_NO_TRAILING_SLASH FS_SEP #else # define FS_SEP "/" -# define FS_ROOT FS_SEP +# define FS_ROOT_NO_TRAILING_SLASH FS_SEP +# define FS_ROOT FS_ROOT_NO_TRAILING_SLASH #endif #ifndef PATH_MAX @@ -44,7 +46,7 @@ TEST(absPath, doesntChangeRoot) { auto p = absPath(std::filesystem::path{FS_ROOT}); - ASSERT_EQ(p, FS_ROOT); + ASSERT_EQ(p, FS_ROOT_NO_TRAILING_SLASH); } TEST(absPath, turnsEmptyPathIntoCWD) @@ -196,42 +198,42 @@ TEST(baseNameOf, absoluteNothingSlashNothing) TEST(isInDir, trivialCase) { - EXPECT_TRUE(isInDir("/foo/bar", "/foo")); + EXPECT_TRUE(isInDir(FS_ROOT "foo" FS_SEP "bar", FS_ROOT "foo")); } TEST(isInDir, notInDir) { - EXPECT_FALSE(isInDir("/zes/foo/bar", "/foo")); + EXPECT_FALSE(isInDir(FS_ROOT "zes" FS_SEP "foo" FS_SEP "bar", FS_ROOT "foo")); } TEST(isInDir, emptyDir) { - EXPECT_FALSE(isInDir("/zes/foo/bar", "")); + EXPECT_FALSE(isInDir(FS_ROOT "zes" FS_SEP "foo" FS_SEP "bar", "")); } TEST(isInDir, hiddenSubdirectory) { - EXPECT_TRUE(isInDir("/foo/.ssh", "/foo")); + EXPECT_TRUE(isInDir(FS_ROOT "foo" FS_SEP ".ssh", FS_ROOT "foo")); } TEST(isInDir, ellipsisEntry) { - EXPECT_TRUE(isInDir("/foo/...", "/foo")); + EXPECT_TRUE(isInDir(FS_ROOT "foo" FS_SEP "...", FS_ROOT "foo")); } TEST(isInDir, sameDir) { - EXPECT_FALSE(isInDir("/foo", "/foo")); + EXPECT_FALSE(isInDir(FS_ROOT "foo", FS_ROOT "foo")); } TEST(isInDir, sameDirDot) { - EXPECT_FALSE(isInDir("/foo/.", "/foo")); + EXPECT_FALSE(isInDir(FS_ROOT "foo" FS_SEP ".", FS_ROOT "foo")); } TEST(isInDir, dotDotPrefix) { - EXPECT_FALSE(isInDir("/foo/../bar", "/foo")); + EXPECT_FALSE(isInDir(FS_ROOT "foo" FS_SEP ".." FS_SEP "bar", FS_ROOT "foo")); } /* ---------------------------------------------------------------------------- @@ -240,8 +242,8 @@ TEST(isInDir, dotDotPrefix) TEST(isDirOrInDir, trueForSameDirectory) { - ASSERT_EQ(isDirOrInDir("/nix", "/nix"), true); - ASSERT_EQ(isDirOrInDir("/", "/"), true); + ASSERT_EQ(isDirOrInDir(FS_ROOT "nix", FS_ROOT "nix"), true); + ASSERT_EQ(isDirOrInDir(FS_ROOT, FS_ROOT), true); } TEST(isDirOrInDir, trueForEmptyPaths) @@ -251,17 +253,17 @@ TEST(isDirOrInDir, trueForEmptyPaths) TEST(isDirOrInDir, falseForDisjunctPaths) { - ASSERT_EQ(isDirOrInDir("/foo", "/bar"), false); + ASSERT_EQ(isDirOrInDir(FS_ROOT "foo", FS_ROOT "bar"), false); } TEST(isDirOrInDir, relativePaths) { - ASSERT_EQ(isDirOrInDir("/foo/..", "/foo"), false); + ASSERT_EQ(isDirOrInDir(FS_ROOT "foo" FS_SEP "..", FS_ROOT "foo"), false); } TEST(isDirOrInDir, relativePathsTwice) { - ASSERT_EQ(isDirOrInDir("/foo/..", "/foo/."), false); + ASSERT_EQ(isDirOrInDir(FS_ROOT "foo" FS_SEP "..", FS_ROOT "foo" FS_SEP "."), false); } /* ---------------------------------------------------------------------------- @@ -294,13 +296,16 @@ TEST(makeParentCanonical, noParent) TEST(makeParentCanonical, root) { - ASSERT_EQ(makeParentCanonical("/"), "/"); + ASSERT_EQ(makeParentCanonical(FS_ROOT), FS_ROOT_NO_TRAILING_SLASH); } /* ---------------------------------------------------------------------------- * chmodIfNeeded * --------------------------------------------------------------------------*/ +#ifndef _WIN32 +// Windows doesn't support Unix-style permission bits - lstat always +// returns the same mode regardless of what chmod sets. TEST(chmodIfNeeded, works) { auto [autoClose_, tmpFile] = nix::createTempFile(); @@ -316,6 +321,7 @@ TEST(chmodIfNeeded, works) } } } +#endif TEST(chmodIfNeeded, nonexistent) { diff --git a/src/libutil-tests/source-accessor.cc b/src/libutil-tests/source-accessor.cc index 9c7e05b82..8f19642a4 100644 --- a/src/libutil-tests/source-accessor.cc +++ b/src/libutil-tests/source-accessor.cc @@ -137,4 +137,34 @@ TEST_F(FSSourceAccessorTest, works) } } +/* ---------------------------------------------------------------------------- + * RestoreSink non-directory at root (no dirFd) + * --------------------------------------------------------------------------*/ + +TEST_F(FSSourceAccessorTest, RestoreSinkRegularFileAtRoot) +{ + auto filePath = tmpDir / "rootfile"; + { + RestoreSink sink(false); + sink.dstPath = filePath; + // No dirFd set - this tests the !dirFd path + sink.createRegularFile(CanonPath::root, [](CreateRegularFileSink & crf) { crf("root content"); }); + } + + EXPECT_THAT(makeFSSourceAccessor(filePath), HasContents(CanonPath::root, "root content")); +} + +TEST_F(FSSourceAccessorTest, RestoreSinkSymlinkAtRoot) +{ + auto linkPath = tmpDir / "rootlink"; + { + RestoreSink sink(false); + sink.dstPath = linkPath; + // No dirFd set - this tests the !dirFd path + sink.createSymlink(CanonPath::root, "symlink_target"); + } + + EXPECT_THAT(makeFSSourceAccessor(linkPath), HasSymlink(CanonPath::root, "symlink_target")); +} + } // namespace nix diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index d4277b778..f16e832bb 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -184,24 +184,6 @@ void AutoCloseFD::close() } } -void AutoCloseFD::fsync() const -{ - if (fd != INVALID_DESCRIPTOR) { - int result; - result = -#ifdef _WIN32 - ::FlushFileBuffers(fd) -#elif defined(__APPLE__) - ::fcntl(fd, F_FULLFSYNC) -#else - ::fsync(fd) -#endif - ; - if (result == -1) - throw NativeSysError("fsync file descriptor %1%", fd); - } -} - void AutoCloseFD::startFsync() const { #ifdef __linux__ diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index f4ed33ffe..2940ac44e 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -340,23 +340,23 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, FsSync sync) if (!fd) throw SysError("opening file '%1%'", path); - writeFile(fd, path, s, mode, sync); + writeFile(fd.get(), s, sync, &path); /* Close explicitly to propagate the exceptions. */ fd.close(); } -void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode, FsSync sync) +void writeFile(Descriptor fd, std::string_view s, FsSync sync, const Path * origPath) { - assert(fd); + assert(fd != INVALID_DESCRIPTOR); try { - writeFull(fd.get(), s); + writeFull(fd, s); if (sync == FsSync::Yes) - fd.fsync(); + syncDescriptor(fd); } catch (Error & e) { - e.addTrace({}, "writing file '%1%'", origPath); + e.addTrace({}, "writing file '%1%'", origPath ? *origPath : descriptorToPath(fd).string()); throw; } } diff --git a/src/libutil/fs-sink.cc b/src/libutil/fs-sink.cc index a7e55aea8..1bfeb1b35 100644 --- a/src/libutil/fs-sink.cc +++ b/src/libutil/fs-sink.cc @@ -181,7 +181,7 @@ void RestoreSink::createRegularFile(const CanonPath & path, std::function