From: Tim Kientzle Date: Thu, 15 Jan 2009 05:17:08 +0000 (-0500) Subject: In archive_write_disk: If archive_write_header() fails to create X-Git-Tag: v2.7.0~438 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7d4496e9d2da3decc8e605b3a2494411a58112c3;p=thirdparty%2Flibarchive.git In archive_write_disk: If archive_write_header() fails to create the file, that's a failure and should return ARCHIVE_FAILED. Metadata restore failures still return ARCHIVE_WARN, because that's non-critical. Fix test_write_disk_secure test to verify the correct return code in one case; add test_write_disk_failures to do another very simple test of restore failure. In particular, this fixes the problem found with xattr restores that a failure in setting an xattr would prevent the file and other metadata from being written. Now, an xattr restore failure generates a warning but does not prevent the rest of that file's data and metadata from being properly written. SVN-Revision: 419 --- diff --git a/libarchive/archive_write_disk.c b/libarchive/archive_write_disk.c index 38f780b83..c72572f3e 100644 --- a/libarchive/archive_write_disk.c +++ b/libarchive/archive_write_disk.c @@ -500,7 +500,7 @@ _archive_write_header(struct archive *_a, struct archive_entry *entry) } /* We've created the object and are ready to pour data into it. */ - if (ret == ARCHIVE_OK) + if (ret >= ARCHIVE_WARN) a->archive.state = ARCHIVE_STATE_DATA; /* * If it's not open, tell our client not to try writing. @@ -865,7 +865,7 @@ edit_deep_directories(struct archive_write_disk *a) *tail = '\0'; /* Terminate dir portion */ ret = create_dir(a, a->name); if (ret == ARCHIVE_OK && chdir(a->name) != 0) - ret = ARCHIVE_WARN; + ret = ARCHIVE_FAILED; *tail = '/'; /* Restore the / we removed. */ if (ret != ARCHIVE_OK) return; @@ -906,7 +906,7 @@ restore_entry(struct archive_write_disk *a) /* We tried, but couldn't get rid of it. */ archive_set_error(&a->archive, errno, "Could not unlink"); - return(ARCHIVE_WARN); + return(ARCHIVE_FAILED); } } @@ -925,7 +925,7 @@ restore_entry(struct archive_write_disk *a) && (a->flags & ARCHIVE_EXTRACT_NO_OVERWRITE)) { /* If we're not overwriting, we're done. */ archive_set_error(&a->archive, en, "Already exists"); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } /* @@ -940,7 +940,7 @@ restore_entry(struct archive_write_disk *a) if (rmdir(a->name) != 0) { archive_set_error(&a->archive, errno, "Can't remove already-existing dir"); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } a->pst = NULL; /* Try again. */ @@ -967,7 +967,7 @@ restore_entry(struct archive_write_disk *a) if (r != 0) { archive_set_error(&a->archive, errno, "Can't stat existing object"); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } /* @@ -996,7 +996,7 @@ restore_entry(struct archive_write_disk *a) if (unlink(a->name) != 0) { archive_set_error(&a->archive, errno, "Can't unlink already-existing object"); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } a->pst = NULL; /* Try again. */ @@ -1006,7 +1006,7 @@ restore_entry(struct archive_write_disk *a) if (rmdir(a->name) != 0) { archive_set_error(&a->archive, errno, "Can't remove already-existing dir"); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } /* Try again. */ en = create_filesystem_object(a); @@ -1030,7 +1030,7 @@ restore_entry(struct archive_write_disk *a) /* Everything failed; give up here. */ archive_set_error(&a->archive, en, "Can't create '%s'", a->name); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } a->pst = NULL; /* Cached stat data no longer valid. */ @@ -1420,7 +1420,7 @@ check_symlinks(struct archive_write_disk *a) "Could not remove symlink %s", a->name); pn[0] = c; - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } a->pst = NULL; /* @@ -1444,7 +1444,7 @@ check_symlinks(struct archive_write_disk *a) "Cannot remove intervening symlink %s", a->name); pn[0] = c; - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } a->pst = NULL; } else { @@ -1452,7 +1452,7 @@ check_symlinks(struct archive_write_disk *a) "Cannot extract through symlink %s", a->name); pn[0] = c; - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } } } @@ -1634,7 +1634,7 @@ create_parent_dir(struct archive_write_disk *a, char *path) * Create the specified dir, recursing to create parents as necessary. * * Returns ARCHIVE_OK if the path exists when we're done here. - * Otherwise, returns ARCHIVE_WARN. + * Otherwise, returns ARCHIVE_FAILED. * Assumes path is in mutable storage; path is unchanged on exit. */ static int @@ -1679,18 +1679,18 @@ create_dir(struct archive_write_disk *a, char *path) if ((a->flags & ARCHIVE_EXTRACT_NO_OVERWRITE)) { archive_set_error(&a->archive, EEXIST, "Can't create directory '%s'", path); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } if (unlink(path) != 0) { archive_set_error(&a->archive, errno, "Can't create directory '%s': " "Conflicting file cannot be removed"); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } } else if (errno != ENOENT && errno != ENOTDIR) { /* Stat failed? */ archive_set_error(&a->archive, errno, "Can't test directory '%s'", path); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } else if (slash != NULL) { *slash = '\0'; r = create_dir(a, path); @@ -1732,7 +1732,7 @@ create_dir(struct archive_write_disk *a, char *path) archive_set_error(&a->archive, errno, "Failed to create dir '%s'", path); - return (ARCHIVE_WARN); + return (ARCHIVE_FAILED); } /* diff --git a/libarchive/test/Makefile b/libarchive/test/Makefile index 52434da2e..aff43fdf8 100644 --- a/libarchive/test/Makefile +++ b/libarchive/test/Makefile @@ -61,6 +61,7 @@ TESTS= \ test_write_compress.c \ test_write_compress_program.c \ test_write_disk.c \ + test_write_disk_failures.c \ test_write_disk_hardlink.c \ test_write_disk_perms.c \ test_write_disk_secure.c \ diff --git a/libarchive/test/test_write_disk_failures.c b/libarchive/test/test_write_disk_failures.c new file mode 100644 index 000000000..fb1a67593 --- /dev/null +++ b/libarchive/test/test_write_disk_failures.c @@ -0,0 +1,62 @@ +/*- + * Copyright (c) 2003-2007 Tim Kientzle + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +#include "test.h" +__FBSDID("$FreeBSD: src/lib/libarchive/test/test_write_disk.c,v 1.15 2008/09/30 04:02:36 kientzle Exp $"); + +#if ARCHIVE_VERSION_NUMBER >= 1009000 + +#define UMASK 022 + + +#endif + +DEFINE_TEST(test_write_disk_failures) +{ +#if ARCHIVE_VERSION_NUMBER < 1009000 + skipping("archive_write_disk interface"); +#else + struct archive_entry *ae; + struct archive *a; + + /* Force the umask to something predictable. */ + umask(UMASK); + + /* A directory that we can't write to. */ + assertEqualInt(0, mkdir("dir", 0555)); + + /* Try to extract a regular file into the directory above. */ + assert((ae = archive_entry_new()) != NULL); + archive_entry_copy_pathname(ae, "dir/file"); + archive_entry_set_mode(ae, S_IFREG | 0755); + archive_entry_set_size(ae, 8); + assert((a = archive_write_disk_new()) != NULL); + archive_write_disk_set_options(a, ARCHIVE_EXTRACT_TIME); + archive_entry_set_mtime(ae, 123456789, 0); + assertEqualIntA(a, ARCHIVE_FAILED, archive_write_header(a, ae)); + assertEqualIntA(a, 0, archive_write_finish_entry(a)); + assertEqualInt(0, archive_write_finish(a)); + archive_entry_free(ae); +#endif +} diff --git a/libarchive/test/test_write_disk_secure.c b/libarchive/test/test_write_disk_secure.c index 2b0d9e964..e61c6ca2a 100644 --- a/libarchive/test/test_write_disk_secure.c +++ b/libarchive/test/test_write_disk_secure.c @@ -81,7 +81,7 @@ DEFINE_TEST(test_write_disk_secure) archive_entry_set_mode(ae, S_IFREG | 0777); archive_write_disk_set_options(a, ARCHIVE_EXTRACT_SECURE_SYMLINKS); failure("Extracting a file through a symlink should fail here."); - assertEqualInt(ARCHIVE_WARN, archive_write_header(a, ae)); + assertEqualInt(ARCHIVE_FAILED, archive_write_header(a, ae)); archive_entry_free(ae); assert(0 == archive_write_finish_entry(a));