From 70319f18409ef2c079f26d7a152d95a5b34ea33a Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Mon, 15 Dec 2025 19:02:58 +0100 Subject: [PATCH] tarfile: unit test suite This will probably get the implementation of the fixtures revised when we land the new extraction code, but we are setting it up to be generic against that. The operator-> thing is kind of a crime. But it also makes the code vastly more readable so it's impossible to say if it's bad or not. Change-Id: Ia5aca69cefaa03cd533ad19d20d856ff7e76a546 --- lix/libutil/tarfile.cc | 2 +- lix/libutil/tarfile.hh | 2 + tests/unit/libutil/tarfile.cc | 253 ++++++++++++++++++++++++++++++++++ tests/unit/meson.build | 1 + 4 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 tests/unit/libutil/tarfile.cc diff --git a/lix/libutil/tarfile.cc b/lix/libutil/tarfile.cc index 8790980d0..0ea05d950 100644 --- a/lix/libutil/tarfile.cc +++ b/lix/libutil/tarfile.cc @@ -45,7 +45,7 @@ void TarArchive::check(int err, const std::string & reason) if (err == ARCHIVE_EOF) { throw EndOfFile("reached end of archive"); } else if (err != ARCHIVE_OK) { - throw Error(reason, archive_error_string(this->archive.get())); + throw ArchiveError(reason, archive_error_string(this->archive.get())); } } diff --git a/lix/libutil/tarfile.hh b/lix/libutil/tarfile.hh index e746f6e2e..292221ba4 100644 --- a/lix/libutil/tarfile.hh +++ b/lix/libutil/tarfile.hh @@ -7,6 +7,8 @@ namespace nix { +MakeError(ArchiveError, Error); + struct TarArchive { std::unique_ptr archive; diff --git a/tests/unit/libutil/tarfile.cc b/tests/unit/libutil/tarfile.cc new file mode 100644 index 000000000..eaefaeba6 --- /dev/null +++ b/tests/unit/libutil/tarfile.cc @@ -0,0 +1,253 @@ +#include +#include +#include + +#include "lix/libutil/tarfile.hh" +#include "lix/libstore/temporary-dir.hh" +#include "lix/libutil/async-io.hh" +#include "lix/libutil/file-system.hh" +#include "lix/libutil/serialise.hh" + +namespace nix { + +class TarWriter +{ + std::unique_ptr archive; + Sink * sink; + + static int callback_open(struct archive * archive, void * self) + { + return ARCHIVE_OK; + } + + static ssize_t + callback_write(struct archive * archive, void * self_, const void * buf, size_t len) + { + auto * self = static_cast(self_); + try { + (*self->sink)(std::string_view{static_cast(buf), len}); + return len; + } catch (std::exception & err) { // NOLINT(lix-foreign-exceptions) + // NOLINTNEXTLINE(lix-unsafe-c-calls): what() is a c string + archive_set_error(archive, EIO, "Sink threw exception: %s", err.what()); + return -1; + } + } + + static int callback_close(struct archive * archive, void * self) + { + return ARCHIVE_OK; + } + + void check(int err, const std::string & reason); + +public: + TarWriter(Sink & sink) : archive(archive_write_new()), sink(&sink) + { + check(archive_write_add_filter_gzip(archive.get()), "add filter gzip (%s)"); + check(archive_write_set_format_gnutar(archive.get()), "set format tar (%s)"); + archive_write_set_option(archive.get(), nullptr, "mac-ext", nullptr); + + check( + archive_write_open( + archive.get(), + static_cast(this), + callback_open, + callback_write, + callback_close + ), + "opening archive (%s)" + ); + } + + void close() && + { + check(archive_write_close(archive.get()), "closing archive (%s)"); + } + + using EntryPtr = std::unique_ptr; + + EntryPtr newEntry() + { + return EntryPtr{archive_entry_new()}; + } + + EntryPtr header(const char * path, mode_t mode, unsigned int filetype) + { + auto entry = newEntry(); + // NOLINTNEXTLINE(lix-unsafe-c-calls): test code lol + archive_entry_set_pathname(entry.get(), path); + archive_entry_set_mode(entry.get(), mode); + archive_entry_set_filetype(entry.get(), filetype); + return entry; + } + + void writeHeader(EntryPtr && entry) + { + check(archive_write_header(archive.get(), entry.get()), "write header (%s)"); + } + + void file(const char * path, std::string content, mode_t mode = 0700) + { + auto entry = header(path, mode, AE_IFREG); + archive_entry_set_size(entry.get(), content.length()); + writeHeader(std::move(entry)); + ssize_t written = archive_write_data(archive.get(), content.data(), content.length()); + if (written < 0) { + check(written, "write data (%s)"); + } + } + + void dir(const char * path, mode_t mode = 0700) + { + auto entry = header(path, mode, AE_IFDIR); + writeHeader(std::move(entry)); + } + + void symlink(const char * path, const char * target, mode_t mode = 0700) + { + auto entry = header(path, mode, AE_IFLNK); + // NOLINTNEXTLINE(lix-unsafe-c-calls): test code lol + archive_entry_set_symlink(entry.get(), target); + writeHeader(std::move(entry)); + } + + void hardlink(const char * path, const char * target, mode_t mode = 0700) + { + auto entry = header(path, mode, AE_IFLNK); + // NOLINTNEXTLINE(lix-unsafe-c-calls): test code lol + archive_entry_set_hardlink(entry.get(), target); + writeHeader(std::move(entry)); + } +}; + +void TarWriter::check(int err, const std::string & reason) +{ + if (err == ARCHIVE_EOF) { + throw EndOfFile("reached end of archive"); + } else if (err != ARCHIVE_OK) { + throw Error(reason, archive_error_string(this->archive.get())); + } +} + +/** Abstracted file status operations (for e.g. being able to replace it with a NAR or something + * else without blowing up all the tests) */ +struct FileChecker +{ + Path baseDir; + + bool fileExists(std::string subpath) + { + return nix::pathExists(this->baseDir + "/" + subpath); + } +}; + +struct TarFixture : testing::Test +{ + Path tmpDir; + AutoDelete deleter; + StringSink sink; + std::unique_ptr writer; + + TarFixture() + : tmpDir{createTempDir()} + , deleter{tmpDir, true} + , sink{} + , writer{std::make_unique(sink)} + { + } + + void finish() + { + if (writer) { + std::move(*writer).close(); + writer = nullptr; + } + } + + FileChecker extract(); +}; + +FileChecker TarFixture::extract() +{ + finish(); + + AsyncIoRoot aio; + auto stream = AsyncStringInputStream{sink.s}; + + aio.blockOn(unpackTarfile(stream, tmpDir)); + + return FileChecker{tmpDir}; +} + +TEST_F(TarFixture, readTrivial) +{ + writer->dir("foo"); + writer->file("foo/bar", "blah"); + auto result = extract(); + + ASSERT_TRUE(result.fileExists("foo/bar")); +} + +TEST_F(TarFixture, dotdotShouldFail) +{ + writer->dir("../foo"); + writer->file("../foo/bar", "blah"); + writer->dir("bar"); + writer->file("bar/nya", "kitty"); + + ASSERT_THROW(extract(), ArchiveError); +} + +TEST_F(TarFixture, okayHardlinkWorks) +{ + writer->dir("somedir"); + writer->file("somedir/somefile", "mrrp"); + writer->hardlink("somedir/link", "somedir/somefile"); + auto result = extract(); + + ASSERT_TRUE(result.fileExists("somedir/somefile")); + ASSERT_TRUE(result.fileExists("somedir/link")); +} + +TEST_F(TarFixture, badHardlinkOrderFails) +{ + writer->dir("somedir"); + writer->hardlink("somedir/link", "somedir/somefile"); + writer->file("somedir/somefile", "mrrp"); + + ASSERT_THROW(extract(), ArchiveError); +} + +TEST_F(TarFixture, okaySymlinkWorksIncludingInFunnyOrder) +{ + writer->dir("somedir"); + writer->symlink("somedir/link", "somedir/somefile"); + writer->file("somedir/somefile", "mrrp"); + auto result = extract(); + + ASSERT_TRUE(result.fileExists("somedir/somefile")); + ASSERT_TRUE(result.fileExists("somedir/link")); +} + +TEST_F(TarFixture, badFileOnTopOfFile) +{ + writer->dir("somedir"); + writer->file("somedir/file", "ohno"); + writer->file("somedir/file/mrrp", "mrrp"); + + ASSERT_THROW(extract(), ArchiveError); +} + +TEST_F(TarFixture, badHardlinkTraversalOverFile) +{ + writer->dir("somedir"); + writer->file("somedir/file", "ohno"); + writer->hardlink("somedir/link", "somedir/file/mrrp"); + + ASSERT_THROW(extract(), ArchiveError); +} + +} diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 2b5d9f469..017e2fe19 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -71,6 +71,7 @@ libutil_tests_sources = files( 'libutil/references.cc', 'libutil/serialise.cc', 'libutil/suggestions.cc', + 'libutil/tarfile.cc', 'libutil/terminal.cc', 'libutil/tests.cc', 'libutil/thread-pool.cc',