From 9ff5f6492f46b7f3342d47f138b590f09e939865 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Sat, 7 Dec 2019 22:35:14 +0700 Subject: [PATCH 01/10] libarchive proof of concept --- Makefile.config.in | 1 + configure.ac | 2 + release-common.nix | 1 + src/libstore/download.cc | 2 +- src/libutil/local.mk | 2 +- src/libutil/tarfile.cc | 135 +++++++++++++++++++---- src/libutil/tarfile.hh | 3 +- src/nix-prefetch-url/nix-prefetch-url.cc | 5 +- 8 files changed, 124 insertions(+), 27 deletions(-) diff --git a/Makefile.config.in b/Makefile.config.in index 7e3b35b98..fe609ce06 100644 --- a/Makefile.config.in +++ b/Makefile.config.in @@ -18,6 +18,7 @@ SODIUM_LIBS = @SODIUM_LIBS@ LIBLZMA_LIBS = @LIBLZMA_LIBS@ SQLITE3_LIBS = @SQLITE3_LIBS@ LIBBROTLI_LIBS = @LIBBROTLI_LIBS@ +LIBARCHIVE_LIBS = @LIBARCHIVE_LIBS@ EDITLINE_LIBS = @EDITLINE_LIBS@ bash = @bash@ bindir = @bindir@ diff --git a/configure.ac b/configure.ac index 9dd0acd86..29835195f 100644 --- a/configure.ac +++ b/configure.ac @@ -178,6 +178,8 @@ AC_CHECK_LIB([bz2], [BZ2_bzWriteOpen], [true], [AC_MSG_ERROR([Nix requires libbz2, which is part of bzip2. See https://web.archive.org/web/20180624184756/http://www.bzip.org/.])]) AC_CHECK_HEADERS([bzlib.h], [true], [AC_MSG_ERROR([Nix requires libbz2, which is part of bzip2. See https://web.archive.org/web/20180624184756/http://www.bzip.org/.])]) +# Checks for libarchive +PKG_CHECK_MODULES([LIBARCHIVE], [libarchive >= 3.4.0], [CXXFLAGS="$LIBARCHIVE_CFLAGS $CXXFLAGS"]) # Look for SQLite, a required dependency. PKG_CHECK_MODULES([SQLITE3], [sqlite3 >= 3.6.19], [CXXFLAGS="$SQLITE3_CFLAGS $CXXFLAGS"]) diff --git a/release-common.nix b/release-common.nix index dd5f939d9..f8c93f76e 100644 --- a/release-common.nix +++ b/release-common.nix @@ -49,6 +49,7 @@ rec { [ curl bzip2 xz brotli editline openssl pkgconfig sqlite boehmgc + libarchive boost nlohmann_json rustc cargo diff --git a/src/libstore/download.cc b/src/libstore/download.cc index 61e88c5c1..c7c1b93ad 100644 --- a/src/libstore/download.cc +++ b/src/libstore/download.cc @@ -907,7 +907,7 @@ CachedDownloadResult Downloader::downloadCached( printInfo("unpacking '%s'...", url); Path tmpDir = createTempDir(); AutoDelete autoDelete(tmpDir, true); - unpackTarfile(store->toRealPath(storePath), tmpDir, baseNameOf(url)); + unpackTarfile(store->toRealPath(storePath), tmpDir); auto members = readDirectory(tmpDir); if (members.size() != 1) throw nix::Error("tarball '%s' contains an unexpected number of top-level files", url); diff --git a/src/libutil/local.mk b/src/libutil/local.mk index 35c1f6c13..16c1fa03f 100644 --- a/src/libutil/local.mk +++ b/src/libutil/local.mk @@ -6,6 +6,6 @@ libutil_DIR := $(d) libutil_SOURCES := $(wildcard $(d)/*.cc) -libutil_LDFLAGS = $(LIBLZMA_LIBS) -lbz2 -pthread $(OPENSSL_LIBS) $(LIBBROTLI_LIBS) $(BOOST_LDFLAGS) -lboost_context +libutil_LDFLAGS = $(LIBLZMA_LIBS) -lbz2 -pthread $(OPENSSL_LIBS) $(LIBBROTLI_LIBS) $(LIBARCHIVE_LIBS) $(BOOST_LDFLAGS) -lboost_context libutil_LIBS = libnixrust diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 2cc7793fd..ab30002dd 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -1,5 +1,8 @@ #include "rust-ffi.hh" #include "compression.hh" +#include +#include +#include "finally.hh" extern "C" { rust::Result> * @@ -8,29 +11,123 @@ extern "C" { namespace nix { +std::shared_ptr archive_read_ptr() { + return std::shared_ptr(archive_read_new(), + [](auto p) { + archive_read_close(p); + archive_read_free(p); + }); +} +void archive_read_open_source(std::shared_ptr a, Source& s, unsigned int bufsize = 1024) { + std::shared_ptr buffer((unsigned char*)malloc(bufsize), [](auto p) { free(p); }); + typedef struct { + decltype(buffer) buf; + Source& src; + unsigned int bs; + } St; + St* state = new St({buffer, s, bufsize}); + if (archive_read_open(a.get(), state, + NULL /* open */, + ([] (struct archive*, void* sptr, const void** buf) -> long int { + St& s = *(static_cast(sptr)); + *buf = s.buf.get(); + try { + return s.src.read(s.buf.get(), s.bs); + } catch (EndOfFile &) { + return 0; + } + /* TODO: I don't know what happens if anything else is thrown here */ + }), [] (struct archive*, void* sptr) { + delete static_cast(sptr); + return ARCHIVE_OK; + })) { + throw Error("archive is corrupt (%s)", archive_error_string(a.get())); + } +} +std::shared_ptr archive_write_ptr() { + return std::shared_ptr(archive_write_disk_new(), + [](auto p) { + archive_write_close(p); + archive_write_free(p); + }); +} +static void copy_data(std::shared_ptr ar, std::shared_ptr aw) +{ + int r; + const void *buff; + size_t size; + la_int64_t offset; + + for (;;) { + r = archive_read_data_block(ar.get(), &buff, &size, &offset); + if (r == ARCHIVE_EOF) return; + if (r < ARCHIVE_OK) { + throw Error("archive is corrupt (%s)", archive_error_string(ar.get())); + } + r = archive_write_data_block(aw.get(), buff, size, offset); + if (r < ARCHIVE_OK) { + throw Error("could not write archive output (%s)", archive_error_string(aw.get())); + } + } +} + +static void extract_archive(std::shared_ptr a, const Path & destDir) { + char * cwd = getcwd(0, 0); + if (!cwd) throw SysError("getting current directory"); + Finally freeCwd([&]() { free(cwd); }); + int r = chdir(destDir.c_str()); + if (r != 0) throw SysError("setting directory to tar output path"); + struct archive_entry *entry; + r = archive_read_next_header(a.get(), &entry); + if (r != ARCHIVE_OK) { + throw Error("archive is corrupt (%s)", archive_error_string(a.get())); + } + int flags = 0; + auto ext = archive_write_ptr(); + flags |= ARCHIVE_EXTRACT_PERM; + flags |= ARCHIVE_EXTRACT_FFLAGS; + archive_write_disk_set_options(ext.get(), flags); + archive_write_disk_set_standard_lookup(ext.get()); + for(;;) { + r = archive_read_next_header(a.get(), &entry); + if (r == ARCHIVE_EOF) break; + if (r == ARCHIVE_WARN) { + std::cerr << "warning: " << archive_error_string(a.get()); + } else if (r < ARCHIVE_WARN) { + throw Error("archive is corrupt (%s)", archive_error_string(a.get())); + } + r = archive_write_header(ext.get(), entry); + if (r != ARCHIVE_OK) { + throw Error("could not write archive output (%s)", archive_error_string(ext.get())); + } + if (archive_entry_size(entry) > 0) { + copy_data(a, ext); + } + archive_write_finish_entry(ext.get()); + } + r = chdir(cwd); + if (r != 0) throw SysError("resetting directory after archive extraction"); +} void unpackTarfile(Source & source, const Path & destDir) { - rust::Source source2(source); - rust::CBox(unpack_tarfile(source2, destDir))->unwrap(); + auto a = archive_read_ptr(); + archive_read_support_filter_all(a.get()); + archive_read_support_format_all(a.get()); + archive_read_open_source(a, source); + createDirs(destDir); + extract_archive(a, destDir); } - -void unpackTarfile(const Path & tarFile, const Path & destDir, - std::optional baseName) +void unpackTarfile(const Path & tarFile, const Path & destDir) { - if (!baseName) baseName = baseNameOf(tarFile); - - auto source = sinkToSource([&](Sink & sink) { - // FIXME: look at first few bytes to determine compression type. - auto decompressor = - // FIXME: add .gz support - hasSuffix(*baseName, ".bz2") ? makeDecompressionSink("bzip2", sink) : - hasSuffix(*baseName, ".xz") ? makeDecompressionSink("xz", sink) : - makeDecompressionSink("none", sink); - readFile(tarFile, *decompressor); - decompressor->finish(); - }); - - unpackTarfile(*source, destDir); + auto a = archive_read_ptr(); + archive_read_support_filter_all(a.get()); + archive_read_support_format_all(a.get()); + int r = archive_read_open_filename(a.get(), tarFile.c_str(), 16384); + if (r != ARCHIVE_OK) { + throw Error("archive is corrupt (%s)", archive_error_string(a.get())); + } + createDirs(destDir); + extract_archive(a, destDir); } } diff --git a/src/libutil/tarfile.hh b/src/libutil/tarfile.hh index ce0911e2a..89a024f1d 100644 --- a/src/libutil/tarfile.hh +++ b/src/libutil/tarfile.hh @@ -4,7 +4,6 @@ namespace nix { void unpackTarfile(Source & source, const Path & destDir); -void unpackTarfile(const Path & tarFile, const Path & destDir, - std::optional baseName = {}); +void unpackTarfile(const Path & tarFile, const Path & destDir); } diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 78c883833..48714446b 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -190,10 +190,7 @@ static int _main(int argc, char * * argv) printInfo("unpacking..."); Path unpacked = (Path) tmpDir + "/unpacked"; createDirs(unpacked); - if (hasSuffix(baseNameOf(uri), ".zip")) - runProgram("unzip", true, {"-qq", tmpFile, "-d", unpacked}); - else - unpackTarfile(tmpFile, unpacked, baseNameOf(uri)); + unpackTarfile(tmpFile, unpacked); /* If the archive unpacks to a single file/directory, then use that as the top-level. */ From 232b39076688c0720f0b5d567cd2a2ac1061c676 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Sat, 7 Dec 2019 23:00:37 +0700 Subject: [PATCH 02/10] fixup! libarchive proof of concept --- src/libutil/tarfile.cc | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index ab30002dd..6c3fb4634 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -61,11 +61,11 @@ static void copy_data(std::shared_ptr ar, std::shared_ptr ar, std::shared_ptr a, const Path & destDir) { char * cwd = getcwd(0, 0); if (!cwd) throw SysError("getting current directory"); - Finally freeCwd([&]() { free(cwd); }); + Finally freeCwd([&]() { chdir(cwd); free(cwd); }); int r = chdir(destDir.c_str()); if (r != 0) throw SysError("setting directory to tar output path"); struct archive_entry *entry; - r = archive_read_next_header(a.get(), &entry); - if (r != ARCHIVE_OK) { - throw Error("archive is corrupt (%s)", archive_error_string(a.get())); - } int flags = 0; auto ext = archive_write_ptr(); flags |= ARCHIVE_EXTRACT_PERM; @@ -100,13 +96,10 @@ static void extract_archive(std::shared_ptr a, const Path & dest if (r != ARCHIVE_OK) { throw Error("could not write archive output (%s)", archive_error_string(ext.get())); } - if (archive_entry_size(entry) > 0) { - copy_data(a, ext); - } + copy_data(a, ext); archive_write_finish_entry(ext.get()); } - r = chdir(cwd); - if (r != 0) throw SysError("resetting directory after archive extraction"); + //if (r != 0) throw SysError("resetting directory after archive extraction"); } void unpackTarfile(Source & source, const Path & destDir) { From f54c168031a2a9b63c0cbf61fb06fc8d4e40841b Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Sat, 7 Dec 2019 23:10:27 +0700 Subject: [PATCH 03/10] add wrapper function around libarchive to c++ errors --- src/libutil/tarfile.cc | 48 ++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 6c3fb4634..8e2f7c8cb 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -18,6 +18,14 @@ std::shared_ptr archive_read_ptr() { archive_read_free(p); }); } +static void ac(std::shared_ptr a, int r, const char* str = "archive is corrupt (%s)") { + if (r == ARCHIVE_EOF) { + throw EndOfFile("tar ended"); + } + if (r != ARCHIVE_OK) { + throw Error(str, archive_error_string(a.get())); + } +} void archive_read_open_source(std::shared_ptr a, Source& s, unsigned int bufsize = 1024) { std::shared_ptr buffer((unsigned char*)malloc(bufsize), [](auto p) { free(p); }); typedef struct { @@ -26,7 +34,7 @@ void archive_read_open_source(std::shared_ptr a, Source& s, unsi unsigned int bs; } St; St* state = new St({buffer, s, bufsize}); - if (archive_read_open(a.get(), state, + ac(a, archive_read_open(a.get(), state, NULL /* open */, ([] (struct archive*, void* sptr, const void** buf) -> long int { St& s = *(static_cast(sptr)); @@ -40,9 +48,7 @@ void archive_read_open_source(std::shared_ptr a, Source& s, unsi }), [] (struct archive*, void* sptr) { delete static_cast(sptr); return ARCHIVE_OK; - })) { - throw Error("archive is corrupt (%s)", archive_error_string(a.get())); - } + })); } std::shared_ptr archive_write_ptr() { return std::shared_ptr(archive_write_disk_new(), @@ -53,28 +59,27 @@ std::shared_ptr archive_write_ptr() { } static void copy_data(std::shared_ptr ar, std::shared_ptr aw) { - int r; const void *buff; size_t size; la_int64_t offset; for (;;) { - r = archive_read_data_block(ar.get(), &buff, &size, &offset); - if (r == ARCHIVE_EOF) return; - if (r != ARCHIVE_OK) { - throw Error("archive is corrupt (%s)", archive_error_string(ar.get())); - } - r = archive_write_data_block(aw.get(), buff, size, offset); - if (r != ARCHIVE_OK) { - throw Error("could not write archive output (%s)", archive_error_string(aw.get())); + try { + ac(ar, archive_read_data_block(ar.get(), &buff, &size, &offset)); + } catch (EndOfFile &) { + return; } + ac(aw, archive_write_data_block(aw.get(), buff, size, offset), "could not write archive output (%s)"); } } - static void extract_archive(std::shared_ptr a, const Path & destDir) { char * cwd = getcwd(0, 0); if (!cwd) throw SysError("getting current directory"); - Finally freeCwd([&]() { chdir(cwd); free(cwd); }); + Finally backCwd([&]() { + int r = chdir(cwd); + free(cwd); + if (r != 0) throw SysError("resetting directory after archive extraction"); + }); int r = chdir(destDir.c_str()); if (r != 0) throw SysError("setting directory to tar output path"); struct archive_entry *entry; @@ -90,16 +95,12 @@ static void extract_archive(std::shared_ptr a, const Path & dest if (r == ARCHIVE_WARN) { std::cerr << "warning: " << archive_error_string(a.get()); } else if (r < ARCHIVE_WARN) { - throw Error("archive is corrupt (%s)", archive_error_string(a.get())); - } - r = archive_write_header(ext.get(), entry); - if (r != ARCHIVE_OK) { - throw Error("could not write archive output (%s)", archive_error_string(ext.get())); + ac(a, r); } + ac(ext, archive_write_header(ext.get(), entry), "could not write archive output (%s)"); copy_data(a, ext); archive_write_finish_entry(ext.get()); } - //if (r != 0) throw SysError("resetting directory after archive extraction"); } void unpackTarfile(Source & source, const Path & destDir) { @@ -115,10 +116,7 @@ void unpackTarfile(const Path & tarFile, const Path & destDir) auto a = archive_read_ptr(); archive_read_support_filter_all(a.get()); archive_read_support_format_all(a.get()); - int r = archive_read_open_filename(a.get(), tarFile.c_str(), 16384); - if (r != ARCHIVE_OK) { - throw Error("archive is corrupt (%s)", archive_error_string(a.get())); - } + ac(a, archive_read_open_filename(a.get(), tarFile.c_str(), 16384)); createDirs(destDir); extract_archive(a, destDir); } From 1355554d1207267ccfb272532917e8085eeb9fb9 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Sat, 7 Dec 2019 23:23:11 +0700 Subject: [PATCH 04/10] code 'cleanup' --- src/libutil/tarfile.cc | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 8e2f7c8cb..b44ebea64 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -20,7 +20,7 @@ std::shared_ptr archive_read_ptr() { } static void ac(std::shared_ptr a, int r, const char* str = "archive is corrupt (%s)") { if (r == ARCHIVE_EOF) { - throw EndOfFile("tar ended"); + throw EndOfFile("reached end of archive"); } if (r != ARCHIVE_OK) { throw Error(str, archive_error_string(a.get())); @@ -72,16 +72,25 @@ static void copy_data(std::shared_ptr ar, std::shared_ptr a, const Path & destDir) { - char * cwd = getcwd(0, 0); - if (!cwd) throw SysError("getting current directory"); - Finally backCwd([&]() { - int r = chdir(cwd); - free(cwd); - if (r != 0) throw SysError("resetting directory after archive extraction"); - }); - int r = chdir(destDir.c_str()); - if (r != 0) throw SysError("setting directory to tar output path"); + // need to chdir back *after* archive closing + PushD newDir(destDir); struct archive_entry *entry; int flags = 0; auto ext = archive_write_ptr(); @@ -90,7 +99,7 @@ static void extract_archive(std::shared_ptr a, const Path & dest archive_write_disk_set_options(ext.get(), flags); archive_write_disk_set_standard_lookup(ext.get()); for(;;) { - r = archive_read_next_header(a.get(), &entry); + int r = archive_read_next_header(a.get(), &entry); if (r == ARCHIVE_EOF) break; if (r == ARCHIVE_WARN) { std::cerr << "warning: " << archive_error_string(a.get()); @@ -101,22 +110,27 @@ static void extract_archive(std::shared_ptr a, const Path & dest copy_data(a, ext); archive_write_finish_entry(ext.get()); } + // done in dtor, but this error can be 'failed to set permissions' + // so it's important + ac(ext, archive_write_close(ext.get()), "finishing archive extraction"); } void unpackTarfile(Source & source, const Path & destDir) { - auto a = archive_read_ptr(); + auto a = nix::archive_read_ptr(); archive_read_support_filter_all(a.get()); archive_read_support_format_all(a.get()); - archive_read_open_source(a, source); + nix::archive_read_open_source(a, source); + createDirs(destDir); extract_archive(a, destDir); } void unpackTarfile(const Path & tarFile, const Path & destDir) { - auto a = archive_read_ptr(); + auto a = nix::archive_read_ptr(); archive_read_support_filter_all(a.get()); archive_read_support_format_all(a.get()); ac(a, archive_read_open_filename(a.get(), tarFile.c_str(), 16384)); + createDirs(destDir); extract_archive(a, destDir); } From fe7ec70e6bf8458a95b216b5bb72e078a0025486 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Sat, 7 Dec 2019 23:28:31 +0700 Subject: [PATCH 05/10] remove rust unpack_tarfile ffi --- src/libutil/tarfile.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index b44ebea64..4da596281 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -4,11 +4,6 @@ #include #include "finally.hh" -extern "C" { - rust::Result> * - unpack_tarfile(rust::Source source, rust::StringSlice dest_dir); -} - namespace nix { std::shared_ptr archive_read_ptr() { From 28ee687adff95f35a52cbadf8052a21130b3cdfc Mon Sep 17 00:00:00 2001 From: Puck Meerburg Date: Sat, 7 Dec 2019 18:08:33 +0000 Subject: [PATCH 06/10] Clean up libarchive support --- src/libutil/tarfile.cc | 173 ++++++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 90 deletions(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 4da596281..0dc62b21a 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -6,75 +6,78 @@ namespace nix { -std::shared_ptr archive_read_ptr() { - return std::shared_ptr(archive_read_new(), - [](auto p) { - archive_read_close(p); - archive_read_free(p); - }); -} -static void ac(std::shared_ptr a, int r, const char* str = "archive is corrupt (%s)") { - if (r == ARCHIVE_EOF) { - throw EndOfFile("reached end of archive"); - } - if (r != ARCHIVE_OK) { - throw Error(str, archive_error_string(a.get())); - } -} -void archive_read_open_source(std::shared_ptr a, Source& s, unsigned int bufsize = 1024) { - std::shared_ptr buffer((unsigned char*)malloc(bufsize), [](auto p) { free(p); }); - typedef struct { - decltype(buffer) buf; - Source& src; - unsigned int bs; - } St; - St* state = new St({buffer, s, bufsize}); - ac(a, archive_read_open(a.get(), state, - NULL /* open */, - ([] (struct archive*, void* sptr, const void** buf) -> long int { - St& s = *(static_cast(sptr)); - *buf = s.buf.get(); - try { - return s.src.read(s.buf.get(), s.bs); - } catch (EndOfFile &) { - return 0; - } - /* TODO: I don't know what happens if anything else is thrown here */ - }), [] (struct archive*, void* sptr) { - delete static_cast(sptr); - return ARCHIVE_OK; - })); -} -std::shared_ptr archive_write_ptr() { - return std::shared_ptr(archive_write_disk_new(), - [](auto p) { - archive_write_close(p); - archive_write_free(p); - }); -} -static void copy_data(std::shared_ptr ar, std::shared_ptr aw) -{ - const void *buff; - size_t size; - la_int64_t offset; +struct TarArchive { + struct archive *archive; + Source *source; + unsigned char buffer[4096]; + + void check(int err, const char *reason = "Failed to extract archive (%s)") { + if (err == ARCHIVE_EOF) + throw EndOfFile("reached end of archive"); + else if (err != ARCHIVE_OK) + throw Error(reason, archive_error_string(this->archive)); + } + + TarArchive(Source& source) { + this->archive = archive_read_new(); + this->source = &source; + + archive_read_support_filter_all(archive); + archive_read_support_format_all(archive); + check(archive_read_open(archive, (void *)this, TarArchive::callback_open, TarArchive::callback_read, TarArchive::callback_close), "Failed to open archive (%s)"); + } + + TarArchive(const Path &path) { + this->archive = archive_read_new(); + + archive_read_support_filter_all(archive); + archive_read_support_format_all(archive); + check(archive_read_open_filename(archive, path.c_str(), 16384), "Failed to open archive (%s)"); + } + + void close() { + check(archive_read_close(archive), "Failed to close archive (%s)"); + } + + ~TarArchive() { + if (this->archive) archive_read_free(this->archive); + } + +private: + static int callback_open(struct archive *, void *self) { + return ARCHIVE_OK; + } + + static ssize_t callback_read(struct archive *archive, void *_self, const void **buffer) { + TarArchive *self = (TarArchive *)_self; + *buffer = self->buffer; + + try { + return self->source->read(self->buffer, 4096); + } catch (EndOfFile &) { + return 0; + } catch (std::exception &err) { + archive_set_error(archive, EIO, "Source threw exception: %s", err.what()); + + return -1; + } + } + + static int callback_close(struct archive *, void *self) { + return ARCHIVE_OK; + } +}; - for (;;) { - try { - ac(ar, archive_read_data_block(ar.get(), &buff, &size, &offset)); - } catch (EndOfFile &) { - return; - } - ac(aw, archive_write_data_block(aw.get(), buff, size, offset), "could not write archive output (%s)"); - } -} struct PushD { char * oldDir; - PushD(std::string newDir) { + + PushD(const std::string &newDir) { oldDir = getcwd(0, 0); if (!oldDir) throw SysError("getcwd"); int r = chdir(newDir.c_str()); if (r != 0) throw SysError("changing directory to tar output path"); } + ~PushD() { int r = chdir(oldDir); free(oldDir); @@ -83,51 +86,41 @@ struct PushD { /* can't throw out of a destructor */ } }; -static void extract_archive(std::shared_ptr a, const Path & destDir) { + +static void extract_archive(TarArchive &archive, const Path & destDir) { // need to chdir back *after* archive closing PushD newDir(destDir); struct archive_entry *entry; - int flags = 0; - auto ext = archive_write_ptr(); - flags |= ARCHIVE_EXTRACT_PERM; - flags |= ARCHIVE_EXTRACT_FFLAGS; - archive_write_disk_set_options(ext.get(), flags); - archive_write_disk_set_standard_lookup(ext.get()); + int flags = ARCHIVE_EXTRACT_FFLAGS | ARCHIVE_EXTRACT_PERM; + for(;;) { - int r = archive_read_next_header(a.get(), &entry); + int r = archive_read_next_header(archive.archive, &entry); if (r == ARCHIVE_EOF) break; - if (r == ARCHIVE_WARN) { - std::cerr << "warning: " << archive_error_string(a.get()); - } else if (r < ARCHIVE_WARN) { - ac(a, r); - } - ac(ext, archive_write_header(ext.get(), entry), "could not write archive output (%s)"); - copy_data(a, ext); - archive_write_finish_entry(ext.get()); + else if (r == ARCHIVE_WARN) + std::cerr << "warning: " << archive_error_string(archive.archive) << std::endl; + else + archive.check(r); + + archive.check(archive_read_extract(archive.archive, entry, flags)); } - // done in dtor, but this error can be 'failed to set permissions' - // so it's important - ac(ext, archive_write_close(ext.get()), "finishing archive extraction"); + + archive.close(); } + void unpackTarfile(Source & source, const Path & destDir) { - auto a = nix::archive_read_ptr(); - archive_read_support_filter_all(a.get()); - archive_read_support_format_all(a.get()); - nix::archive_read_open_source(a, source); + auto archive = TarArchive(source); createDirs(destDir); - extract_archive(a, destDir); + extract_archive(archive, destDir); } + void unpackTarfile(const Path & tarFile, const Path & destDir) { - auto a = nix::archive_read_ptr(); - archive_read_support_filter_all(a.get()); - archive_read_support_format_all(a.get()); - ac(a, archive_read_open_filename(a.get(), tarFile.c_str(), 16384)); + auto archive = TarArchive(tarFile); createDirs(destDir); - extract_archive(a, destDir); + extract_archive(archive, destDir); } } From eba82b7c88b36445feaccfd396756f9701766bc6 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Mon, 9 Dec 2019 17:21:46 +0700 Subject: [PATCH 07/10] further clean up libarchive code --- src/libutil/tarfile.cc | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 0dc62b21a..68e918891 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -1,15 +1,14 @@ -#include "rust-ffi.hh" -#include "compression.hh" #include #include -#include "finally.hh" + +#include "serialise.hh" namespace nix { struct TarArchive { struct archive *archive; Source *source; - unsigned char buffer[4096]; + std::vector buffer; void check(int err, const char *reason = "Failed to extract archive (%s)") { if (err == ARCHIVE_EOF) @@ -18,7 +17,7 @@ struct TarArchive { throw Error(reason, archive_error_string(this->archive)); } - TarArchive(Source& source) { + TarArchive(Source& source) : buffer(4096) { this->archive = archive_read_new(); this->source = &source; @@ -35,6 +34,9 @@ struct TarArchive { check(archive_read_open_filename(archive, path.c_str(), 16384), "Failed to open archive (%s)"); } + // disable copy constructor + TarArchive(const TarArchive&) = delete; + void close() { check(archive_read_close(archive), "Failed to close archive (%s)"); } @@ -47,13 +49,13 @@ private: static int callback_open(struct archive *, void *self) { return ARCHIVE_OK; } - + static ssize_t callback_read(struct archive *archive, void *_self, const void **buffer) { - TarArchive *self = (TarArchive *)_self; - *buffer = self->buffer; + TarArchive *self = (TarArchive *)_self; + *buffer = self->buffer.data(); try { - return self->source->read(self->buffer, 4096); + return self->source->read(self->buffer.data(), 4096); } catch (EndOfFile &) { return 0; } catch (std::exception &err) { @@ -82,7 +84,7 @@ struct PushD { int r = chdir(oldDir); free(oldDir); if (r != 0) - std::cerr << "warning: popd failed to chdir"; + std::cerr << "warning: failed to change directory back after tar extraction"; /* can't throw out of a destructor */ } }; @@ -91,7 +93,11 @@ static void extract_archive(TarArchive &archive, const Path & destDir) { // need to chdir back *after* archive closing PushD newDir(destDir); struct archive_entry *entry; - int flags = ARCHIVE_EXTRACT_FFLAGS | ARCHIVE_EXTRACT_PERM; + int flags = ARCHIVE_EXTRACT_FFLAGS + | ARCHIVE_EXTRACT_PERM + | ARCHIVE_EXTRACT_SECURE_SYMLINKS + | ARCHIVE_EXTRACT_SECURE_NODOTDOT + | ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS; for(;;) { int r = archive_read_next_header(archive.archive, &entry); From b232eea40ae0dd08293b5ef6542366a0018a1587 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Mon, 9 Dec 2019 17:28:15 +0700 Subject: [PATCH 08/10] nix-rust: remove unused tar file code --- nix-rust/Cargo.lock | 74 ++--------------------------------------- nix-rust/Cargo.toml | 1 - nix-rust/src/lib.rs | 8 ----- nix-rust/src/tarfile.rs | 46 ------------------------- 4 files changed, 3 insertions(+), 126 deletions(-) delete mode 100644 nix-rust/src/tarfile.rs diff --git a/nix-rust/Cargo.lock b/nix-rust/Cargo.lock index 0112ed471..53e842826 100644 --- a/nix-rust/Cargo.lock +++ b/nix-rust/Cargo.lock @@ -1,84 +1,16 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -[[package]] -name = "cfg-if" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" - -[[package]] -name = "filetime" -version = "0.2.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", - "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", - "redox_syscall 0.1.56 (registry+https://github.com/rust-lang/crates.io-index)", - "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", -] - [[package]] name = "libc" -version = "0.2.65" +version = "0.2.66" source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "nix-rust" version = "0.1.0" dependencies = [ - "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", - "tar 0.4.26 (registry+https://github.com/rust-lang/crates.io-index)", -] - -[[package]] -name = "redox_syscall" -version = "0.1.56" -source = "registry+https://github.com/rust-lang/crates.io-index" - -[[package]] -name = "tar" -version = "0.4.26" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "filetime 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", - "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", - "redox_syscall 0.1.56 (registry+https://github.com/rust-lang/crates.io-index)", - "xattr 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", -] - -[[package]] -name = "winapi" -version = "0.3.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", -] - -[[package]] -name = "winapi-i686-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" - -[[package]] -name = "winapi-x86_64-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" - -[[package]] -name = "xattr" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)", ] [metadata] -"checksum cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" -"checksum filetime 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "1ff6d4dab0aa0c8e6346d46052e93b13a16cf847b54ed357087c35011048cc7d" -"checksum libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)" = "1a31a0627fdf1f6a39ec0dd577e101440b7db22672c0901fe00a9a6fbb5c24e8" -"checksum redox_syscall 0.1.56 (registry+https://github.com/rust-lang/crates.io-index)" = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84" -"checksum tar 0.4.26 (registry+https://github.com/rust-lang/crates.io-index)" = "b3196bfbffbba3e57481b6ea32249fbaf590396a52505a2615adbb79d9d826d3" -"checksum winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)" = "8093091eeb260906a183e6ae1abdba2ef5ef2257a21801128899c3fc699229c6" -"checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" -"checksum winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" -"checksum xattr 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "244c3741f4240ef46274860397c7c74e50eb23624996930e484c16679633a54c" +"checksum libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)" = "d515b1f41455adea1313a4a2ac8a8a477634fbae63cc6100e3aebb207ce61558" diff --git a/nix-rust/Cargo.toml b/nix-rust/Cargo.toml index c4f4ceb09..40b6b7d50 100644 --- a/nix-rust/Cargo.toml +++ b/nix-rust/Cargo.toml @@ -9,5 +9,4 @@ name = "nixrust" crate-type = ["cdylib"] [dependencies] -tar = "0.4" libc = "0.2" diff --git a/nix-rust/src/lib.rs b/nix-rust/src/lib.rs index 1b88ac8af..a25a791e1 100644 --- a/nix-rust/src/lib.rs +++ b/nix-rust/src/lib.rs @@ -1,6 +1,5 @@ mod error; mod foreign; -mod tarfile; pub use error::Error; @@ -23,10 +22,3 @@ impl CBox { } } -#[no_mangle] -pub extern "C" fn unpack_tarfile( - source: foreign::Source, - dest_dir: &str, -) -> CBox> { - CBox::new(tarfile::unpack_tarfile(source, dest_dir).map_err(|err| err.into())) -} diff --git a/nix-rust/src/tarfile.rs b/nix-rust/src/tarfile.rs deleted file mode 100644 index 379d9098f..000000000 --- a/nix-rust/src/tarfile.rs +++ /dev/null @@ -1,46 +0,0 @@ -use crate::{foreign::Source, Error}; -use std::fs; -use std::io; -use std::os::unix::fs::OpenOptionsExt; -use std::path::Path; -use tar::Archive; - -pub fn unpack_tarfile(source: Source, dest_dir: &str) -> Result<(), Error> { - let dest_dir = Path::new(dest_dir); - - let mut tar = Archive::new(source); - - for file in tar.entries()? { - let mut file = file?; - - let dest_file = dest_dir.join(file.path()?); - - fs::create_dir_all(dest_file.parent().unwrap())?; - - match file.header().entry_type() { - tar::EntryType::Directory => { - fs::create_dir(dest_file)?; - } - tar::EntryType::Regular => { - let mode = if file.header().mode()? & (libc::S_IXUSR as u32) == 0 { - 0o666 - } else { - 0o777 - }; - let mut f = fs::OpenOptions::new() - .create(true) - .write(true) - .mode(mode) - .open(dest_file)?; - io::copy(&mut file, &mut f)?; - } - tar::EntryType::Symlink => { - std::os::unix::fs::symlink(file.header().link_name()?.unwrap(), dest_file)?; - } - tar::EntryType::XGlobalHeader | tar::EntryType::XHeader => {} - t => return Err(Error::Misc(format!("unsupported tar entry type '{:?}'", t))), - } - } - - Ok(()) -} From 3663a8a7e92bd66f21fa653ba4d825a357ca6168 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Mon, 9 Dec 2019 17:31:05 +0700 Subject: [PATCH 09/10] release.nix: add libarchive to rpm and deb dependencies --- release.nix | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/release.nix b/release.nix index 9cf4c74f2..5d742958c 100644 --- a/release.nix +++ b/release.nix @@ -394,10 +394,10 @@ let src = jobs.tarball; diskImage = (diskImageFun vmTools.diskImageFuns) { extraPackages = - [ "sqlite" "sqlite-devel" "bzip2-devel" "libcurl-devel" "openssl-devel" "xz-devel" "libseccomp-devel" "libsodium-devel" "boost-devel" "bison" "flex" ] + [ "sqlite" "sqlite-devel" "bzip2-devel" "libcurl-devel" "openssl-devel" "xz-devel" "libarchive-devel" "libseccomp-devel" "libsodium-devel" "boost-devel" "bison" "flex" ] ++ extraPackages; }; # At most 2047MB can be simulated in qemu-system-i386 - memSize = 2047; + memSize = 2047; meta.schedulingPriority = 50; postRPMInstall = "cd /tmp/rpmout/BUILD/nix-* && make installcheck"; #enableParallelBuilding = true; @@ -417,7 +417,7 @@ let src = jobs.tarball; diskImage = (diskImageFun vmTools.diskImageFuns) { extraPackages = - [ "libsqlite3-dev" "libbz2-dev" "libcurl-dev" "libcurl3-nss" "libssl-dev" "liblzma-dev" "libseccomp-dev" "libsodium-dev" "libboost-all-dev" ] + [ "libsqlite3-dev" "libbz2-dev" "libcurl-dev" "libcurl3-nss" "libarchive-dev" "libssl-dev" "liblzma-dev" "libseccomp-dev" "libsodium-dev" "libboost-all-dev" ] ++ extraPackages; }; memSize = 2047; meta.schedulingPriority = 50; From f765e441237cb6679c33cb44372a5b30168c42c8 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Mon, 9 Dec 2019 18:39:37 +0700 Subject: [PATCH 10/10] downgrade required libarchive version (ubuntu 16.04) --- configure.ac | 2 +- release.nix | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 29835195f..28a7b68fb 100644 --- a/configure.ac +++ b/configure.ac @@ -179,7 +179,7 @@ AC_CHECK_LIB([bz2], [BZ2_bzWriteOpen], [true], AC_CHECK_HEADERS([bzlib.h], [true], [AC_MSG_ERROR([Nix requires libbz2, which is part of bzip2. See https://web.archive.org/web/20180624184756/http://www.bzip.org/.])]) # Checks for libarchive -PKG_CHECK_MODULES([LIBARCHIVE], [libarchive >= 3.4.0], [CXXFLAGS="$LIBARCHIVE_CFLAGS $CXXFLAGS"]) +PKG_CHECK_MODULES([LIBARCHIVE], [libarchive >= 3.1.2], [CXXFLAGS="$LIBARCHIVE_CFLAGS $CXXFLAGS"]) # Look for SQLite, a required dependency. PKG_CHECK_MODULES([SQLITE3], [sqlite3 >= 3.6.19], [CXXFLAGS="$SQLITE3_CFLAGS $CXXFLAGS"]) diff --git a/release.nix b/release.nix index 5d742958c..65c94b17c 100644 --- a/release.nix +++ b/release.nix @@ -424,7 +424,7 @@ let postInstall = "make installcheck"; configureFlags = "--sysconfdir=/etc"; debRequires = - [ "curl" "libsqlite3-0" "libbz2-1.0" "bzip2" "xz-utils" "libssl1.0.0" "liblzma5" "libseccomp2" ] + [ "curl" "libsqlite3-0" "libbz2-1.0" "bzip2" "xz-utils" "libarchive" "libssl1.0.0" "liblzma5" "libseccomp2" ] ++ extraDebPackages; debMaintainer = "Eelco Dolstra "; doInstallCheck = true;