diff --git a/README.md b/README.md index fa48ca3..a68d5ce 100644 --- a/README.md +++ b/README.md @@ -50,8 +50,9 @@ zget -1 URL [MEMBER] Archive operations follow `unzip` where practical, while output conventions follow `curl`. `zget URL MEMBER` therefore streams to standard output, while -`-o FILE` names a local output. Unlike ordinary redirection, named output is -validated completely before publication and never overwrites an existing path. +`-o FILE` streams directly to a local output using normal file-open semantics: +an existing file is truncated and overwritten, and `-o -` selects standard +output. `-l` requests an `unzip`-style listing; the compact `-1` form follows `zipinfo` and emits only names. @@ -244,7 +245,8 @@ The output callback borrows each buffer only for that callback invocation. Extraction may emit data before a later decompression, size, or CRC failure, so only a `ZGET_OK` return guarantees complete validated output. Applications that need atomic publication should stream to temporary storage and publish it only -after success, as the CLI does for `-o`. +after success. The CLI deliberately gives `-o` ordinary curl-style streaming +semantics instead. ### Listing @@ -332,9 +334,11 @@ redirects cannot downgrade to HTTP. A strong ETag from the first accepted response is used for later `If-Match` requests. Without one, consistency is best-effort and still checks the object size. -`-o` writes a temporary file in the destination directory and publishes it only -after decompression and CRC validation. Existing paths are never overwritten. -Stdout cannot be rolled back if a late error occurs. +`-o FILE` opens the requested path normally and streams member data into it. +Existing regular files are truncated, and paths such as symlinks, FIFOs, and +devices follow the platform's normal open behavior. If extraction fails after +writing begins, the partial output remains. `-o -` and the default output both +write to stdout, which likewise cannot be rolled back after a late error. ## Scope diff --git a/cli/zget.c b/cli/zget.c index e8c5786..1e33f21 100644 --- a/cli/zget.c +++ b/cli/zget.c @@ -5,10 +5,7 @@ #include #include #include -#include #include -#include -#include struct file_output { FILE *file; @@ -129,32 +126,14 @@ static void usage(FILE *file) " zget -1 URL [MEMBER]\n"); } -static char *temporary_name(const char *path) -{ - /* Same-directory placement keeps final publication on one filesystem. */ - const char *slash = strrchr(path, '/'); - const char *base = slash == NULL ? path : slash + 1; - size_t dir_len = slash == NULL ? 0 : (size_t)(slash - path + 1); - size_t n = dir_len + 1 + strlen(base) + sizeof(".zget.tmp.XXXXXX"); - char *name = malloc(n); - if (name == NULL) - return NULL; - if (dir_len != 0) - memcpy(name, path, dir_len); - (void)snprintf(name + dir_len, n - dir_len, ".%s.zget.tmp.XXXXXX", base); - return name; -} - int main(int argc, char **argv) { const char *output_path = NULL, *url, *member = NULL; - char *temp_path = NULL; - struct stat st; struct file_output output = {stdout, 0}; struct list_output listing = {{stdout, 0}, 0, 0, 0, 0}; zget_ctx *ctx = NULL; zget_options options; - int arg = 1, fd = -1, list_mode = 0, rc, exit_status = 1; + int arg = 1, close_output = 0, list_mode = 0, rc, exit_status = 1; if (argc == 2 && !strcmp(argv[1], "--version")) { printf("zget %s\n", zget_version()); @@ -261,33 +240,13 @@ int main(int argc, char **argv) goto done; } - if (output_path != NULL) { - /* - * Never stream into the destination itself: decompression or CRC may - * fail after substantial output. mkstemp gives this invocation unique, - * private scratch storage which every failure path removes below. - */ - if (lstat(output_path, &st) == 0) { - fprintf(stderr, "zget: output already exists: %s\n", output_path); - goto done; - } - if (errno != ENOENT) { - fprintf(stderr, "zget: cannot inspect output: %s\n", strerror(errno)); - goto done; - } - temp_path = temporary_name(output_path); - if (temp_path == NULL || (fd = mkstemp(temp_path)) < 0) { - fprintf(stderr, "zget: cannot create temporary output: %s\n", strerror(errno)); - goto done; - } - output.file = fdopen(fd, "wb"); + if (output_path != NULL && strcmp(output_path, "-") != 0) { + output.file = fopen(output_path, "wb"); if (output.file == NULL) { - fprintf(stderr, "zget: cannot open temporary output: %s\n", strerror(errno)); - close(fd); - fd = -1; + fprintf(stderr, "zget: cannot open output: %s\n", strerror(errno)); goto done; } - fd = -1; + close_output = 1; } /* Keep ZIP-specific lookup details behind libzget's format-neutral API. */ @@ -297,26 +256,20 @@ int main(int argc, char **argv) goto done; goto zget_failure; } - /* Publish only after library validation and durable file contents succeed. */ - if (fflush(output.file) != 0 || (output_path != NULL && fsync(fileno(output.file)) != 0)) { + if (fflush(output.file) != 0) { if (errno == EPIPE) goto done; fprintf(stderr, "zget: cannot flush output: %s\n", strerror(errno)); goto done; } - if (output_path != NULL) { + if (close_output) { if (fclose(output.file) != 0) { output.file = NULL; fprintf(stderr, "zget: cannot close output: %s\n", strerror(errno)); goto done; } output.file = NULL; - /* link(2) publishes atomically and cannot overwrite an existing path. */ - if (link(temp_path, output_path) != 0) { - fprintf(stderr, "zget: cannot publish output: %s\n", strerror(errno)); - goto done; - } - (void)unlink(temp_path); + close_output = 0; } exit_status = 0; goto done; @@ -326,15 +279,9 @@ int main(int argc, char **argv) ctx != NULL && zget_last_error_message(ctx)[0] != '\0' ? ": " : "", ctx != NULL ? zget_last_error_message(ctx) : ""); done: - /* stdout is borrowed; only a stream opened for -o is owned and closed here. */ - if (output_path != NULL && output.file != NULL) + /* stdout is borrowed; only a stream opened for named output is owned. */ + if (close_output && output.file != NULL) (void)fclose(output.file); - if (fd >= 0) - (void)close(fd); - if (temp_path != NULL) { - (void)unlink(temp_path); - free(temp_path); - } zget_close(ctx); zget_global_cleanup(); return exit_status; diff --git a/docs/zget.1.in b/docs/zget.1.in index 66c1c96..3d26e88 100644 --- a/docs/zget.1.in +++ b/docs/zget.1.in @@ -45,9 +45,11 @@ are required; a URL by itself does not request an archive listing. .PP By default, member data is written to standard output. When .B \-o -is used, output is first written to a temporary file in the destination -directory. The destination is published only after extraction and CRC -validation succeed, and an existing path is never overwritten. +is used with a file name, that path is opened normally and member data is +streamed directly to it. An existing regular file is truncated. If extraction +fails after writing begins, partial output remains. The special file name +.B \- +selects standard output. .PP .B \-l streams a listing directly from the Central Directory. Each output row contains @@ -69,7 +71,12 @@ listing. This form follows the convention established by .BI \-o " FILE" Write the member to .I FILE -instead of standard output. +instead of standard output. Existing files are overwritten using normal +file-open semantics; a +.I FILE +of +.B \- +selects standard output. .TP .B \-l List archive members to standard output. With @@ -121,7 +128,7 @@ Write a member to standard output: zget https://example.com/archive.zip README.txt .fi .PP -Publish a validated member as a local file: +Write a member to a local file: .PP .nf zget -o report.pdf https://example.com/documents.zip path/to/report.pdf diff --git a/tests/integration.py b/tests/integration.py index 5b14e92..031a481 100644 --- a/tests/integration.py +++ b/tests/integration.py @@ -294,7 +294,7 @@ def empty_listing(base): assert run_server(empty_archive(), "normal", empty_listing) == 2 def normal(base): - """Cover codecs, redirects, varied names, and safe file publication.""" + """Cover codecs, redirects, varied names, and file output semantics.""" got = subprocess.run([binary, base + "/archive.zip", "stored.txt"], check=True, stdout=subprocess.PIPE).stdout assert got == b"stored payload" @@ -312,12 +312,51 @@ def normal(base): assert got == b"long" with tempfile.TemporaryDirectory() as d: path = os.path.join(d, "out") + with open(path, "wb") as stream: + stream.write(b"old content that must be truncated") subprocess.run([binary, "-o", path, base + "/archive.zip", "stored.txt"], check=True) - assert open(path, "rb").read() == b"stored payload" - again = subprocess.run( - [binary, "-o", path, base + "/archive.zip", "stored.txt"]) - assert again.returncode != 0 + with open(path, "rb") as stream: + assert stream.read() == b"stored payload" + + dash = subprocess.run( + [binary, "-o", "-", base + "/archive.zip", "stored.txt"], + cwd=d, check=True, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + assert dash.stdout == b"stored payload" + assert not os.path.exists(os.path.join(d, "-")) + + target = os.path.join(d, "target") + link = os.path.join(d, "link") + with open(target, "wb") as stream: + stream.write(b"old symlink target") + os.symlink(target, link) + subprocess.run( + [binary, "-o", link, base + "/archive.zip", "stored.txt"], + check=True) + assert os.path.islink(link) + with open(target, "rb") as stream: + assert stream.read() == b"stored payload" + + fifo = os.path.join(d, "fifo") + os.mkfifo(fifo) + fifo_fd = os.open(fifo, os.O_RDWR) + try: + process = subprocess.Popen( + [binary, "-o", fifo, base + "/archive.zip", "stored.txt"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + stdout, stderr = process.communicate(timeout=10) + assert process.returncode == 0, stderr + assert stdout == b"" + assert os.read(fifo_fd, 1024) == b"stored payload" + finally: + os.close(fifo_fd) + + rejected = subprocess.run( + [binary, "-o", d, base + "/archive.zip", "stored.txt"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + assert rejected.returncode != 0 + assert b"cannot open output" in rejected.stderr run_server(data, "normal", normal) def broken_pipe(base): @@ -414,6 +453,22 @@ def explicit_http_status(base, mode=mode): # the intended unsupported-method or encryption check. bad_crc = mutate(data, cd + 16, b"\x00\x00\x00\x00") expect_failure(bad_crc) + + def late_named_output_failure(base): + """Retain bytes written to named output before a late CRC failure.""" + with tempfile.TemporaryDirectory() as d: + path = os.path.join(d, "out") + with open(path, "wb") as stream: + stream.write(b"old content") + result = subprocess.run( + [binary, "-o", path, base + "/archive.zip", "stored.txt"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + assert result.returncode != 0 + assert result.stdout == b"" + with open(path, "rb") as stream: + assert stream.read() == b"stored payload" + run_server(bad_crc, "normal", late_named_output_failure) + unsupported = mutate(mutate(data, cd + 10, (99).to_bytes(2, "little")), local + 8, (99).to_bytes(2, "little")) expect_failure(unsupported)