From 38b29fb72ca4a07afbec1fd5067f59ca7d7f0fab Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 12 Dec 2019 15:15:18 +0100 Subject: [PATCH 01/16] libstore/ssh: Improve error message on failing `execvp` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the `throw` is reached, this means that execvp into `ssh` wasn’t successful. We can hint at a usual problem, which is a missing `ssh` executable. Test with: ``` env PATH= ./result/bin/nix-copy-closure --builders '' unusedhost ``` and the bash version with ``` env PATH= ./result/bin/nix-copy-closure --builders '' localhost ``` --- src/libstore/ssh.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index ac3ccd63d..ddc99a6cd 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -47,10 +47,13 @@ std::unique_ptr SSHMaster::startCommand(const std::string throw SysError("duping over stderr"); Strings args; + const char * execInto; if (fakeSSH) { + execInto = "bash"; args = { "bash", "-c" }; } else { + execInto = "ssh"; args = { "ssh", host.c_str(), "-x", "-a" }; addCommonSSHOpts(args); if (socketPath != "") @@ -62,7 +65,8 @@ std::unique_ptr SSHMaster::startCommand(const std::string args.push_back(command); execvp(args.begin()->c_str(), stringsToCharPtrs(args).data()); - throw SysError("executing '%s' on '%s'", command, host); + // could not exec ssh/bash + throw SysError("Failed to exec into %s. Is it in PATH?", execInto); }); @@ -108,7 +112,7 @@ Path SSHMaster::startMaster() addCommonSSHOpts(args); execvp(args.begin()->c_str(), stringsToCharPtrs(args).data()); - throw SysError("starting SSH master"); + throw SysError("Failed to exec into ssh. Is it in PATH?"); }); out.writeSide = -1; From c6295a3afd87a605f30f1de8b09d7885eb08fedb Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Fri, 13 Dec 2019 03:29:33 -0500 Subject: [PATCH 02/16] Initial gzip support Closes #3256 --- configure.ac | 5 +++ release-common.nix | 2 +- src/libutil/compression.cc | 64 ++++++++++++++++++++++++++++++++++++++ src/libutil/tarfile.cc | 1 + tests/tarball.sh | 17 ++++++++++ 5 files changed, 88 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 9dd0acd86..99248b90c 100644 --- a/configure.ac +++ b/configure.ac @@ -209,6 +209,11 @@ PKG_CHECK_MODULES([LIBLZMA], [liblzma], [CXXFLAGS="$LIBLZMA_CFLAGS $CXXFLAGS"]) AC_CHECK_LIB([lzma], [lzma_stream_encoder_mt], [AC_DEFINE([HAVE_LZMA_MT], [1], [xz multithreaded compression support])]) +# Look for zlib, a required dependency. +PKG_CHECK_MODULES([ZLIB], [zlib], [CXXFLAGS="$ZLIB_CFLAGS $CXXFLAGS"]) +AC_CHECK_HEADER([zlib.h],[:],[AC_MSG_ERROR([could not find the zlib.h header])]) +LDFLAGS="-lz $LDFLAGS" + # Look for libbrotli{enc,dec}. PKG_CHECK_MODULES([LIBBROTLI], [libbrotlienc libbrotlidec], [CXXFLAGS="$LIBBROTLI_CFLAGS $CXXFLAGS"]) diff --git a/release-common.nix b/release-common.nix index dd5f939d9..3765e61b3 100644 --- a/release-common.nix +++ b/release-common.nix @@ -47,7 +47,7 @@ rec { buildDeps = [ curl - bzip2 xz brotli editline + bzip2 xz brotli zlib editline openssl pkgconfig sqlite boehmgc boost nlohmann_json diff --git a/src/libutil/compression.cc b/src/libutil/compression.cc index 0dd84e320..17b506d5d 100644 --- a/src/libutil/compression.cc +++ b/src/libutil/compression.cc @@ -11,6 +11,8 @@ #include #include +#include + #include namespace nix { @@ -42,6 +44,66 @@ struct NoneSink : CompressionSink void write(const unsigned char * data, size_t len) override { nextSink(data, len); } }; +struct GzipDecompressionSink : CompressionSink +{ + Sink & nextSink; + z_stream strm; + bool finished = false; + uint8_t outbuf[BUFSIZ]; + + GzipDecompressionSink(Sink & nextSink) : nextSink(nextSink) + { + strm.zalloc = Z_NULL; + strm.zfree = Z_NULL; + strm.opaque = Z_NULL; + strm.avail_in = 0; + strm.next_in = Z_NULL; + strm.next_out = outbuf; + strm.avail_out = sizeof(outbuf); + + // Enable gzip and zlib decoding (+32) with 15 windowBits + int ret = inflateInit2(&strm,15+32); + if (ret != Z_OK) + throw CompressionError("unable to initialise gzip encoder"); + } + + ~GzipDecompressionSink() + { + inflateEnd(&strm); + } + + void finish() override + { + CompressionSink::flush(); + write(nullptr, 0); + } + + void write(const unsigned char * data, size_t len) override + { + assert(len <= std::numeric_limits::max()); + + strm.next_in = (Bytef *) data; + strm.avail_in = len; + + while (!finished && (!data || strm.avail_in)) { + checkInterrupt(); + + int ret = inflate(&strm,Z_SYNC_FLUSH); + if (ret != Z_OK && ret != Z_STREAM_END) + throw CompressionError("error while decompressing gzip file: %d: %d: %d",ret, len, strm.avail_in); + + + finished = ret == Z_STREAM_END; + + if (strm.avail_out < sizeof(outbuf) || strm.avail_in == 0) { + nextSink(outbuf, sizeof(outbuf) - strm.avail_out); + strm.next_out = (Bytef *) outbuf; + strm.avail_out = sizeof(outbuf); + } + } + } +}; + struct XzDecompressionSink : CompressionSink { Sink & nextSink; @@ -215,6 +277,8 @@ ref makeDecompressionSink(const std::string & method, Sink & ne return make_ref(nextSink); else if (method == "bzip2") return make_ref(nextSink); + else if (method == "gzip") + return make_ref(nextSink); else if (method == "br") return make_ref(nextSink); else diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 262bc655f..c00673182 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -24,6 +24,7 @@ void unpackTarfile(const Path & tarFile, const Path & destDir, auto decompressor = // FIXME: add .gz support hasSuffix(*baseName, ".bz2") ? makeDecompressionSink("bzip2", sink) : + hasSuffix(*baseName, ".gz") ? makeDecompressionSink("gzip", sink) : hasSuffix(*baseName, ".xz") ? makeDecompressionSink("xz", sink) : makeDecompressionSink("none", sink); readFile(tarFile, *decompressor); diff --git a/tests/tarball.sh b/tests/tarball.sh index ba534c626..ec810b4d7 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -26,3 +26,20 @@ nix-instantiate --eval -E 'with ; 1 + 2' -I fnord=file://no-such-ta (! nix-instantiate --eval -E ' 1' -I fnord=file://no-such-tarball.tar.xz) nix-instantiate --eval -E '' -I fnord=file://no-such-tarball.tar.xz -I fnord=. + +tarball=$TEST_ROOT/tarball.tar.gz +(cd $TEST_ROOT && tar c tarball) | gzip > $tarball + +nix-env -f file://$tarball -qa --out-path | grep -q dependencies + +nix-build -o $TEST_ROOT/result file://$tarball + +nix-build -o $TEST_ROOT/result '' -I foo=file://$tarball + +nix-build -o $TEST_ROOT/result -E "import (fetchTarball file://$tarball)" + +nix-instantiate --eval -E '1 + 2' -I fnord=file://no-such-tarball.tar.gz +nix-instantiate --eval -E 'with ; 1 + 2' -I fnord=file://no-such-tarball.tar.gz +(! nix-instantiate --eval -E ' 1' -I fnord=file://no-such-tarball.tar.gz) + +nix-instantiate --eval -E '' -I fnord=file://no-such-tarball.tar.gz -I fnord=. From d1b238ec3cd74d652af46f577f992c9a44ac8e32 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 12:53:20 +0100 Subject: [PATCH 03/16] Simplify --- src/libstore/ssh.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index ddc99a6cd..2ee7115c5 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -47,13 +47,10 @@ std::unique_ptr SSHMaster::startCommand(const std::string throw SysError("duping over stderr"); Strings args; - const char * execInto; if (fakeSSH) { - execInto = "bash"; args = { "bash", "-c" }; } else { - execInto = "ssh"; args = { "ssh", host.c_str(), "-x", "-a" }; addCommonSSHOpts(args); if (socketPath != "") @@ -66,7 +63,7 @@ std::unique_ptr SSHMaster::startCommand(const std::string execvp(args.begin()->c_str(), stringsToCharPtrs(args).data()); // could not exec ssh/bash - throw SysError("Failed to exec into %s. Is it in PATH?", execInto); + throw SysError("unable to execute '%s'", args.front()); }); @@ -112,7 +109,7 @@ Path SSHMaster::startMaster() addCommonSSHOpts(args); execvp(args.begin()->c_str(), stringsToCharPtrs(args).data()); - throw SysError("Failed to exec into ssh. Is it in PATH?"); + throw SysError("unable to execute '%s'", args.front()); }); out.writeSide = -1; From 3e787423c22c34d65fd6fe0cdf2e6ec333d81c0b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 12:55:52 +0100 Subject: [PATCH 04/16] Remove FIXME --- src/libutil/tarfile.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index c00673182..79fb9f721 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -22,7 +22,6 @@ void unpackTarfile(const Path & tarFile, const Path & destDir, 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, ".gz") ? makeDecompressionSink("gzip", sink) : hasSuffix(*baseName, ".xz") ? makeDecompressionSink("xz", sink) : From e8aa2290ed4517d42408909f18f1b9434261c36a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 14:42:55 +0100 Subject: [PATCH 05/16] Only install *.sb files on macOS --- src/libstore/local.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libstore/local.mk b/src/libstore/local.mk index d3254554d..ac68c2342 100644 --- a/src/libstore/local.mk +++ b/src/libstore/local.mk @@ -13,7 +13,9 @@ ifneq ($(OS), FreeBSD) libstore_LDFLAGS += -ldl endif +ifeq ($(OS), Darwin) libstore_FILES = sandbox-defaults.sb sandbox-minimal.sb sandbox-network.sb +endif $(foreach file,$(libstore_FILES),$(eval $(call install-data-in,$(d)/$(file),$(datadir)/nix/sandbox))) From d89d9958a771fb6299eae084c1b52d0ecf95901b Mon Sep 17 00:00:00 2001 From: Dima Date: Fri, 13 Dec 2019 14:44:14 +0100 Subject: [PATCH 06/16] bugfix: Adding depth limit to resolveExprPath There is no termination condition for evaluation of cyclical expression paths which can lead to infinite loops. This addresses one spot in the parser in a similar fashion as utils.cc/canonPath does. This issue can be reproduced by something like: ``` ln -s a b ln -s b a nix-instantiate -E 'import ./a' ``` --- src/libexpr/parser.y | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 10a057062..df55986f6 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -576,10 +576,15 @@ Path resolveExprPath(Path path) { assert(path[0] == '/'); + unsigned int followCount = 0, maxFollow = 1024; + /* If `path' is a symlink, follow it. This is so that relative path references work. */ struct stat st; while (true) { + // Basic cycle/depth limit to avoid infinite loops. + if (++followCount >= maxFollow) + throw Error(format("can't resolve expression. infinite symlink recursion in path '%1%'") % path); if (lstat(path.c_str(), &st)) throw SysError(format("getting status of '%1%'") % path); if (!S_ISLNK(st.st_mode)) break; From 4581159e3f5a1cc38aa8e90dfd45f0a63354be44 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 17:26:58 +0100 Subject: [PATCH 07/16] Simplify tarball test --- tests/tarball.sh | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/tests/tarball.sh b/tests/tarball.sh index ec810b4d7..7738b948d 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -10,36 +10,28 @@ mkdir -p $tarroot cp dependencies.nix $tarroot/default.nix cp config.nix dependencies.builder*.sh $tarroot/ -tarball=$TEST_ROOT/tarball.tar.xz -(cd $TEST_ROOT && tar c tarball) | xz > $tarball +test_tarball() { + local ext="$1" + local compressor="$2" -nix-env -f file://$tarball -qa --out-path | grep -q dependencies + tarball=$TEST_ROOT/tarball.tar$ext + (cd $TEST_ROOT && tar c tarball) | $compressor > $tarball -nix-build -o $TEST_ROOT/result file://$tarball + nix-env -f file://$tarball -qa --out-path | grep -q dependencies -nix-build -o $TEST_ROOT/result '' -I foo=file://$tarball + nix-build -o $TEST_ROOT/result file://$tarball -nix-build -o $TEST_ROOT/result -E "import (fetchTarball file://$tarball)" + nix-build -o $TEST_ROOT/result '' -I foo=file://$tarball -nix-instantiate --eval -E '1 + 2' -I fnord=file://no-such-tarball.tar.xz -nix-instantiate --eval -E 'with ; 1 + 2' -I fnord=file://no-such-tarball.tar.xz -(! nix-instantiate --eval -E ' 1' -I fnord=file://no-such-tarball.tar.xz) + nix-build -o $TEST_ROOT/result -E "import (fetchTarball file://$tarball)" -nix-instantiate --eval -E '' -I fnord=file://no-such-tarball.tar.xz -I fnord=. + nix-instantiate --eval -E '1 + 2' -I fnord=file://no-such-tarball.tar$ext + nix-instantiate --eval -E 'with ; 1 + 2' -I fnord=file://no-such-tarball$ext + (! nix-instantiate --eval -E ' 1' -I fnord=file://no-such-tarball$ext) -tarball=$TEST_ROOT/tarball.tar.gz -(cd $TEST_ROOT && tar c tarball) | gzip > $tarball + nix-instantiate --eval -E '' -I fnord=file://no-such-tarball$ext -I fnord=. +} -nix-env -f file://$tarball -qa --out-path | grep -q dependencies - -nix-build -o $TEST_ROOT/result file://$tarball - -nix-build -o $TEST_ROOT/result '' -I foo=file://$tarball - -nix-build -o $TEST_ROOT/result -E "import (fetchTarball file://$tarball)" - -nix-instantiate --eval -E '1 + 2' -I fnord=file://no-such-tarball.tar.gz -nix-instantiate --eval -E 'with ; 1 + 2' -I fnord=file://no-such-tarball.tar.gz -(! nix-instantiate --eval -E ' 1' -I fnord=file://no-such-tarball.tar.gz) - -nix-instantiate --eval -E '' -I fnord=file://no-such-tarball.tar.gz -I fnord=. +test_tarball '' cat +test_tarball .xz xz +test_tarball .gz gzip From 5a6d6da7aea23a48126a77f98612518af66bc203 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 18:11:37 +0100 Subject: [PATCH 08/16] Validate tarball components --- nix-rust/src/c.rs | 5 ++++- nix-rust/src/error.rs | 4 ++++ nix-rust/src/util/tarfile.rs | 18 ++++++++++++++---- tests/bad.tar.xz | Bin 0 -> 228 bytes tests/tarball.sh | 5 +++++ 5 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 tests/bad.tar.xz diff --git a/nix-rust/src/c.rs b/nix-rust/src/c.rs index 19e737a88..3feeb71a3 100644 --- a/nix-rust/src/c.rs +++ b/nix-rust/src/c.rs @@ -11,7 +11,10 @@ pub extern "C" fn unpack_tarfile( source: foreign::Source, dest_dir: &str, ) -> CBox> { - CBox::new(util::tarfile::unpack_tarfile(source, dest_dir).map_err(|err| err.into())) + CBox::new( + util::tarfile::unpack_tarfile(source, std::path::Path::new(dest_dir)) + .map_err(|err| err.into()), + ) } #[no_mangle] diff --git a/nix-rust/src/error.rs b/nix-rust/src/error.rs index c2ba5d504..586a8f53d 100644 --- a/nix-rust/src/error.rs +++ b/nix-rust/src/error.rs @@ -23,6 +23,7 @@ pub enum Error { HttpError(hyper::error::Error), Misc(String), Foreign(CppException), + BadTarFileMemberName(String), } impl From for Error { @@ -64,6 +65,9 @@ impl fmt::Display for Error { Error::HttpError(err) => write!(f, "HTTP error: {}", err), Error::Foreign(_) => write!(f, ""), // FIXME Error::Misc(s) => write!(f, "{}", s), + Error::BadTarFileMemberName(s) => { + write!(f, "tar archive contains illegal file name '{}'", s) + } } } } diff --git a/nix-rust/src/util/tarfile.rs b/nix-rust/src/util/tarfile.rs index 379d9098f..74d60692c 100644 --- a/nix-rust/src/util/tarfile.rs +++ b/nix-rust/src/util/tarfile.rs @@ -2,18 +2,28 @@ use crate::{foreign::Source, Error}; use std::fs; use std::io; use std::os::unix::fs::OpenOptionsExt; -use std::path::Path; +use std::path::{Component, Path}; use tar::Archive; -pub fn unpack_tarfile(source: Source, dest_dir: &str) -> Result<(), Error> { - let dest_dir = Path::new(dest_dir); +pub fn unpack_tarfile(source: Source, dest_dir: &Path) -> Result<(), Error> { + fs::create_dir_all(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()?); + let path = file.path()?; + + for i in path.components() { + if let Component::Prefix(_) | Component::RootDir | Component::ParentDir = i { + return Err(Error::BadTarFileMemberName( + file.path()?.to_str().unwrap().to_string(), + )); + } + } + + let dest_file = dest_dir.join(path); fs::create_dir_all(dest_file.parent().unwrap())?; diff --git a/tests/bad.tar.xz b/tests/bad.tar.xz new file mode 100644 index 0000000000000000000000000000000000000000..250a5ad1a79ee088d5976160664daad6e1a136ff GIT binary patch literal 228 zcmVuwgxWr*GQJ}UxbD$dT zFZcR4Oq`4c+brLa?D6R=fMJjM$?MdlXt0Pp!zwDzlSF=e({Qq}bnc_B9C}zwW!F?< z;h>7LqPxu=kzFWj+$Z*4&0g78Yyp1kn1+nCzFC}~s1p Date: Fri, 13 Dec 2019 18:29:16 +0100 Subject: [PATCH 09/16] Get rid of CBox --- nix-rust/src/c.rs | 11 ++++++----- nix-rust/src/foreign.rs | 19 ------------------- src/libutil/rust-ffi.hh | 20 -------------------- src/libutil/tarfile.cc | 6 ++++-- 4 files changed, 10 insertions(+), 46 deletions(-) diff --git a/nix-rust/src/c.rs b/nix-rust/src/c.rs index 3feeb71a3..e0462742f 100644 --- a/nix-rust/src/c.rs +++ b/nix-rust/src/c.rs @@ -1,20 +1,21 @@ use super::{ error, - foreign::{self, CBox}, + foreign::{self}, store::path, store::StorePath, util, }; #[no_mangle] -pub extern "C" fn unpack_tarfile( +pub unsafe extern "C" fn unpack_tarfile( source: foreign::Source, dest_dir: &str, -) -> CBox> { - CBox::new( + out: *mut Result<(), error::CppException>, +) { + out.write( util::tarfile::unpack_tarfile(source, std::path::Path::new(dest_dir)) .map_err(|err| err.into()), - ) + ); } #[no_mangle] diff --git a/nix-rust/src/foreign.rs b/nix-rust/src/foreign.rs index 8e04280f3..7bce7753c 100644 --- a/nix-rust/src/foreign.rs +++ b/nix-rust/src/foreign.rs @@ -12,22 +12,3 @@ impl std::io::Read for Source { Ok(n) } } - -pub struct CBox { - pub ptr: *mut libc::c_void, - phantom: std::marker::PhantomData, -} - -impl CBox { - pub fn new(t: T) -> Self { - unsafe { - let size = std::mem::size_of::(); - let ptr = libc::malloc(size); - *(ptr as *mut T) = t; // FIXME: probably UB - Self { - ptr, - phantom: std::marker::PhantomData, - } - } - } -} diff --git a/src/libutil/rust-ffi.hh b/src/libutil/rust-ffi.hh index a77f83ac5..1466eabec 100644 --- a/src/libutil/rust-ffi.hh +++ b/src/libutil/rust-ffi.hh @@ -180,24 +180,4 @@ struct Result } }; -template -struct CBox -{ - T * ptr; - - T * operator ->() - { - return ptr; - } - - CBox(T * ptr) : ptr(ptr) { } - CBox(const CBox &) = delete; - CBox(CBox &&) = delete; - - ~CBox() - { - free(ptr); - } -}; - } diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 79fb9f721..1be0ba24c 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -3,7 +3,7 @@ extern "C" { rust::Result> * - unpack_tarfile(rust::Source source, rust::StringSlice dest_dir); + unpack_tarfile(rust::Source source, rust::StringSlice dest_dir, rust::Result> & out); } namespace nix { @@ -11,7 +11,9 @@ namespace nix { void unpackTarfile(Source & source, const Path & destDir) { rust::Source source2(source); - rust::CBox(unpack_tarfile(source2, destDir))->unwrap(); + rust::Result> res; + unpack_tarfile(source2, destDir, res); + res.unwrap(); } void unpackTarfile(const Path & tarFile, const Path & destDir, From e6bd88878e8fc08652b77f0444adcd74321dc6b6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 18:58:34 +0100 Subject: [PATCH 10/16] Improve gzip error message --- src/libutil/compression.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/compression.cc b/src/libutil/compression.cc index 17b506d5d..860b04adb 100644 --- a/src/libutil/compression.cc +++ b/src/libutil/compression.cc @@ -90,8 +90,8 @@ struct GzipDecompressionSink : CompressionSink int ret = inflate(&strm,Z_SYNC_FLUSH); if (ret != Z_OK && ret != Z_STREAM_END) - throw CompressionError("error while decompressing gzip file: %d: %d: %d",ret, len, strm.avail_in); - + throw CompressionError("error while decompressing gzip file: %d (%d, %d)", + zError(ret), len, strm.avail_in); finished = ret == Z_STREAM_END; From b4edc3ca61c1f1a98052ec8ffc41d1b533b08fd7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 19:05:09 +0100 Subject: [PATCH 11/16] Don't leak exceptions --- nix-rust/src/error.rs | 20 ++++++++++++++++++-- src/libutil/rust-ffi.cc | 6 +++++- src/libutil/rust-ffi.hh | 31 +++++++++++++++++++++++++------ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/nix-rust/src/error.rs b/nix-rust/src/error.rs index 586a8f53d..85ac926e9 100644 --- a/nix-rust/src/error.rs +++ b/nix-rust/src/error.rs @@ -76,7 +76,7 @@ impl From for CppException { fn from(err: Error) -> Self { match err { Error::Foreign(ex) => ex, - _ => unsafe { make_error(&err.to_string()) }, + _ => CppException::new(&err.to_string()), } } } @@ -85,7 +85,23 @@ impl From for CppException { #[derive(Debug)] pub struct CppException(*const libc::c_void); // == std::exception_ptr* +impl CppException { + fn new(s: &str) -> Self { + Self(unsafe { make_error(s) }) + } +} + +impl Drop for CppException { + fn drop(&mut self) { + unsafe { + destroy_error(self.0); + } + } +} + extern "C" { #[allow(improper_ctypes)] // YOLO - fn make_error(s: &str) -> CppException; + fn make_error(s: &str) -> *const libc::c_void; + + fn destroy_error(exc: *const libc::c_void); } diff --git a/src/libutil/rust-ffi.cc b/src/libutil/rust-ffi.cc index accc5e22b..6f36b3192 100644 --- a/src/libutil/rust-ffi.cc +++ b/src/libutil/rust-ffi.cc @@ -3,10 +3,14 @@ extern "C" std::exception_ptr * make_error(rust::StringSlice s) { - // FIXME: leak return new std::exception_ptr(std::make_exception_ptr(nix::Error(std::string(s.ptr, s.size)))); } +extern "C" void destroy_error(std::exception_ptr * ex) +{ + free(ex); +} + namespace rust { std::ostream & operator << (std::ostream & str, const String & s) diff --git a/src/libutil/rust-ffi.hh b/src/libutil/rust-ffi.hh index 1466eabec..4fecce606 100644 --- a/src/libutil/rust-ffi.hh +++ b/src/libutil/rust-ffi.hh @@ -152,28 +152,47 @@ struct Source template struct Result { - unsigned int tag; + enum { Ok = 0, Err = 1, Uninit = 2 } tag; union { T data; std::exception_ptr * exc; }; + Result() : tag(Uninit) { }; // FIXME: remove + + Result(const Result &) = delete; + + Result(Result && other) + : tag(other.tag) + { + other.tag = Uninit; + if (tag == Ok) + data = std::move(other.data); + else if (tag == Err) + exc = other.exc; + } + ~Result() { - if (tag == 0) + if (tag == Ok) data.~T(); - else if (tag == 1) - // FIXME: don't leak exc + else if (tag == Err) + free(exc); + else if (tag == Uninit) ; + else + abort(); } /* Rethrow the wrapped exception or return the wrapped value. */ T unwrap() { - if (tag == 0) + if (tag == Ok) { + tag = Uninit; return std::move(data); - else if (tag == 1) + } + else if (tag == Err) std::rethrow_exception(*exc); else abort(); From ac9cc2ec08e95910ecb73745cb011596a33723f0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 19:10:39 +0100 Subject: [PATCH 12/16] Move some code --- src/libutil/rust-ffi.cc | 11 +++++++++++ src/libutil/rust-ffi.hh | 7 +------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/libutil/rust-ffi.cc b/src/libutil/rust-ffi.cc index 6f36b3192..8b8b7b75d 100644 --- a/src/libutil/rust-ffi.cc +++ b/src/libutil/rust-ffi.cc @@ -19,4 +19,15 @@ std::ostream & operator << (std::ostream & str, const String & s) return str; } +size_t Source::sourceWrapper(void * _this, rust::Slice data) +{ + try { + // FIXME: how to propagate exceptions? + auto n = ((nix::Source *) _this)->read((unsigned char *) data.ptr, data.size); + return n; + } catch (...) { + abort(); + } +} + } diff --git a/src/libutil/rust-ffi.hh b/src/libutil/rust-ffi.hh index 4fecce606..3b51661c2 100644 --- a/src/libutil/rust-ffi.hh +++ b/src/libutil/rust-ffi.hh @@ -140,12 +140,7 @@ struct Source : fun(sourceWrapper), _this(&_this) {} - // FIXME: how to propagate exceptions? - static size_t sourceWrapper(void * _this, rust::Slice data) - { - auto n = ((nix::Source *) _this)->read((unsigned char *) data.ptr, data.size); - return n; - } + static size_t sourceWrapper(void * _this, rust::Slice data); }; /* C++ representation of Rust's Result. */ From a70706b0253e3bbd9143fde16e65fb7fe1a4007d Mon Sep 17 00:00:00 2001 From: Albert Safin Date: Sat, 14 Dec 2019 15:37:20 +0000 Subject: [PATCH 13/16] nix-shell: don't check for "nix-shell" in shebang script name --- src/nix-build/nix-build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 73e3a347b..205165a4c 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -106,7 +106,7 @@ static void _main(int argc, char * * argv) // Heuristic to see if we're invoked as a shebang script, namely, // if we have at least one argument, it's the name of an // executable file, and it starts with "#!". - if (runEnv && argc > 1 && !std::regex_search(argv[1], std::regex("nix-shell"))) { + if (runEnv && argc > 1) { script = argv[1]; try { auto lines = tokenizeString(readFile(script), "\n"); From ba6d2093c74f2023394b1b8a4a4d2a1f405c70c1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sat, 14 Dec 2019 23:19:04 +0100 Subject: [PATCH 14/16] Fix progress bar --- src/nix/progress-bar.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/progress-bar.cc b/src/nix/progress-bar.cc index d20c09f26..c445f31cc 100644 --- a/src/nix/progress-bar.cc +++ b/src/nix/progress-bar.cc @@ -155,7 +155,7 @@ public: if (type == actBuild) { auto name = storePathToName(getS(fields, 0)); if (hasSuffix(name, ".drv")) - name = name.substr(name.size() - 4); + name = name.substr(0, name.size() - 4); i->s = fmt("building " ANSI_BOLD "%s" ANSI_NORMAL, name); auto machineName = getS(fields, 1); if (machineName != "") @@ -180,7 +180,7 @@ public: if (type == actPostBuildHook) { auto name = storePathToName(getS(fields, 0)); if (hasSuffix(name, ".drv")) - name = name.substr(name.size() - 4); + name = name.substr(0, name.size() - 4); i->s = fmt("post-build " ANSI_BOLD "%s" ANSI_NORMAL, name); i->name = DrvName(name).name; } From acb71aa5c6a7a55bc1c483c2a6940d85997d482c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 15 Dec 2019 10:44:53 +0100 Subject: [PATCH 15/16] Tweak error message --- src/libexpr/parser.y | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index df55986f6..afa1fd439 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -584,9 +584,9 @@ Path resolveExprPath(Path path) while (true) { // Basic cycle/depth limit to avoid infinite loops. if (++followCount >= maxFollow) - throw Error(format("can't resolve expression. infinite symlink recursion in path '%1%'") % path); + throw Error("too many symbolic links encountered while traversing the path '%s'", path); if (lstat(path.c_str(), &st)) - throw SysError(format("getting status of '%1%'") % path); + throw SysError("getting status of '%s'", path); if (!S_ISLNK(st.st_mode)) break; path = absPath(readLink(path), dirOf(path)); } From 410acd29c046ae7296d882ee4750441d4ff29955 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 15 Dec 2019 10:47:59 +0100 Subject: [PATCH 16/16] Fix cargo test --- nix-rust/src/error.rs | 7 +++++++ nix-rust/src/lib.rs | 1 + 2 files changed, 8 insertions(+) diff --git a/nix-rust/src/error.rs b/nix-rust/src/error.rs index 85ac926e9..9abcacc06 100644 --- a/nix-rust/src/error.rs +++ b/nix-rust/src/error.rs @@ -22,6 +22,7 @@ pub enum Error { #[cfg(unused)] HttpError(hyper::error::Error), Misc(String), + #[cfg(not(test))] Foreign(CppException), BadTarFileMemberName(String), } @@ -63,6 +64,7 @@ impl fmt::Display for Error { Error::IOError(err) => write!(f, "I/O error: {}", err), #[cfg(unused)] Error::HttpError(err) => write!(f, "HTTP error: {}", err), + #[cfg(not(test))] Error::Foreign(_) => write!(f, ""), // FIXME Error::Misc(s) => write!(f, "{}", s), Error::BadTarFileMemberName(s) => { @@ -72,6 +74,7 @@ impl fmt::Display for Error { } } +#[cfg(not(test))] impl From for CppException { fn from(err: Error) -> Self { match err { @@ -81,16 +84,19 @@ impl From for CppException { } } +#[cfg(not(test))] #[repr(C)] #[derive(Debug)] pub struct CppException(*const libc::c_void); // == std::exception_ptr* +#[cfg(not(test))] impl CppException { fn new(s: &str) -> Self { Self(unsafe { make_error(s) }) } } +#[cfg(not(test))] impl Drop for CppException { fn drop(&mut self) { unsafe { @@ -99,6 +105,7 @@ impl Drop for CppException { } } +#[cfg(not(test))] extern "C" { #[allow(improper_ctypes)] // YOLO fn make_error(s: &str) -> *const libc::c_void; diff --git a/nix-rust/src/lib.rs b/nix-rust/src/lib.rs index 0c3cf5276..9935cd27a 100644 --- a/nix-rust/src/lib.rs +++ b/nix-rust/src/lib.rs @@ -9,6 +9,7 @@ extern crate assert_matches; #[macro_use] extern crate proptest; +#[cfg(not(test))] mod c; mod error; mod foreign;