From owner-svn-src-all@FreeBSD.ORG Thu Dec 11 05:56:48 2008 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 265C71065670; Thu, 11 Dec 2008 05:56:48 +0000 (UTC) (envelope-from kientzle@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 13BE18FC1D; Thu, 11 Dec 2008 05:56:48 +0000 (UTC) (envelope-from kientzle@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id mBB5ulvG055768; Thu, 11 Dec 2008 05:56:47 GMT (envelope-from kientzle@svn.freebsd.org) Received: (from kientzle@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id mBB5ulMx055764; Thu, 11 Dec 2008 05:56:47 GMT (envelope-from kientzle@svn.freebsd.org) Message-Id: <200812110556.mBB5ulMx055764@svn.freebsd.org> From: Tim Kientzle Date: Thu, 11 Dec 2008 05:56:47 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r185909 - in stable/7/usr.bin/tar: . test X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Dec 2008 05:56:48 -0000 Author: kientzle Date: Thu Dec 11 05:56:47 2008 New Revision: 185909 URL: http://svn.freebsd.org/changeset/base/185909 Log: MFC r183009, r184807, r184808: Fix --strip-components, plus a handful of test suite improvements. PR: bin/128562 Submitted by: Jaakko Heinonen Approved by: re Added: stable/7/usr.bin/tar/test/test_strip_components.c - copied, changed from r184807, head/usr.bin/tar/test/test_strip_components.c stable/7/usr.bin/tar/test/test_symlink_dir.c - copied unchanged from r183009, head/usr.bin/tar/test/test_symlink_dir.c Modified: stable/7/usr.bin/tar/ (props changed) stable/7/usr.bin/tar/test/Makefile stable/7/usr.bin/tar/util.c Modified: stable/7/usr.bin/tar/test/Makefile ============================================================================== --- stable/7/usr.bin/tar/test/Makefile Thu Dec 11 05:56:25 2008 (r185908) +++ stable/7/usr.bin/tar/test/Makefile Thu Dec 11 05:56:47 2008 (r185909) @@ -18,6 +18,8 @@ TESTS= \ test_option_q.c \ test_patterns.c \ test_stdio.c \ + test_strip_components.c \ + test_symlink_dir.c \ test_version.c # Build the test program Copied and modified: stable/7/usr.bin/tar/test/test_strip_components.c (from r184807, head/usr.bin/tar/test/test_strip_components.c) ============================================================================== --- head/usr.bin/tar/test/test_strip_components.c Mon Nov 10 05:04:55 2008 (r184807, copy source) +++ stable/7/usr.bin/tar/test/test_strip_components.c Thu Dec 11 05:56:47 2008 (r185909) @@ -63,15 +63,44 @@ DEFINE_TEST(test_strip_components) assertEqualInt(-1, lstat("target/d0", &st)); failure("d0/d1/ is too short and should not get restored"); assertEqualInt(-1, lstat("target/d1", &st)); - failure("d0/l1 is too short and should not get restored"); - assertEqualInt(-1, lstat("target/l1", &st)); - failure("d0/d1/l2 is a hardlink to file whose name was too short"); - assertEqualInt(-1, lstat("target/l2", &st)); - assertEqualInt(0, lstat("target/s2", &st)); failure("d0/d1/s2 is a symlink to something that won't be extracted"); assertEqualInt(-1, stat("target/s2", &st)); + assertEqualInt(0, lstat("target/s2", &st)); failure("d0/d1/d2 should be extracted"); assertEqualInt(0, lstat("target/d2", &st)); + + /* + * This next is a complicated case. d0/l1, d0/d1/l2, and + * d0/d1/d2/f1 are all hardlinks to the same file; d0/l1 can't + * be extracted with --strip-components=2 and the other two + * can. Remember that tar normally stores the first file with + * a body and the other as hardlink entries to the first + * appearance. So the final result depends on the order in + * which these three names get archived. If d0/l1 is first, + * none of the three can be restored. If either of the longer + * names are first, then the two longer ones can both be + * restored. + * + * The tree-walking code used by bsdtar always visits files + * before subdirectories, so bsdtar's behavior is fortunately + * deterministic: d0/l1 will always get stored first and the + * other two will be stored as hardlinks to d0/l1. Since + * d0/l1 can't be extracted, none of these three will be + * extracted. + * + * It may be worth extending this test to force a particular + * archiving order so as to exercise both of the cases described + * above. + * + * Of course, this is all totally different for cpio and newc + * formats because the hardlink management is different. + * TODO: Rename this to test_strip_components_tar and create + * parallel tests for cpio and newc formats. + */ + failure("d0/l1 is too short and should not get restored"); + assertEqualInt(-1, lstat("target/l1", &st)); + failure("d0/d1/l2 is a hardlink to file whose name was too short"); + assertEqualInt(-1, lstat("target/l2", &st)); failure("d0/d1/d2/f1 is a hardlink to file whose name was too short"); assertEqualInt(-1, lstat("target/d2/f1", &st)); } Copied: stable/7/usr.bin/tar/test/test_symlink_dir.c (from r183009, head/usr.bin/tar/test/test_symlink_dir.c) ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ stable/7/usr.bin/tar/test/test_symlink_dir.c Thu Dec 11 05:56:47 2008 (r185909, copy of r183009, head/usr.bin/tar/test/test_symlink_dir.c) @@ -0,0 +1,172 @@ +/*- + * 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$"); + +/* + * tar -x -P should follow existing symlinks for dirs, but not other + * content. Plain tar -x should remove symlinks when they're in the + * way of a dir extraction. + */ + +static int +mkfile(const char *name, int mode, const char *contents, ssize_t size) +{ + int fd = open(name, O_CREAT | O_WRONLY, mode); + if (fd < 0) + return (-1); + if (size != write(fd, contents, size)) { + close(fd); + return (-1); + } + close(fd); + return (0); +} + +DEFINE_TEST(test_symlink_dir) +{ + struct stat st, st2; + int oldumask; + + oldumask = umask(0); + + assertEqualInt(0, mkdir("source", 0755)); + assertEqualInt(0, mkfile("source/file", 0755, "a", 1)); + assertEqualInt(0, mkfile("source/file2", 0755, "ab", 2)); + assertEqualInt(0, mkdir("source/dir", 0755)); + assertEqualInt(0, mkdir("source/dir/d", 0755)); + assertEqualInt(0, mkfile("source/dir/f", 0755, "abc", 3)); + assertEqualInt(0, mkdir("source/dir2", 0755)); + assertEqualInt(0, mkdir("source/dir2/d2", 0755)); + assertEqualInt(0, mkfile("source/dir2/f2", 0755, "abcd", 4)); + assertEqualInt(0, mkdir("source/dir3", 0755)); + assertEqualInt(0, mkdir("source/dir3/d3", 0755)); + assertEqualInt(0, mkfile("source/dir3/f3", 0755, "abcde", 5)); + + assertEqualInt(0, + systemf("%s -cf test.tar -C source dir dir2 dir3 file file2", + testprog)); + + /* + * Extract with -x and without -P. + */ + assertEqualInt(0, mkdir("dest1", 0755)); + /* "dir" is a symlink to an existing "real_dir" */ + assertEqualInt(0, mkdir("dest1/real_dir", 0755)); + assertEqualInt(0, symlink("real_dir", "dest1/dir")); + /* "dir2" is a symlink to a non-existing "real_dir2" */ + assertEqualInt(0, symlink("real_dir2", "dest1/dir2")); + /* "dir3" is a symlink to an existing "non_dir3" */ + assertEqualInt(0, mkfile("dest1/non_dir3", 0755, "abcdef", 6)); + assertEqualInt(0, symlink("non_dir3", "dest1/dir3")); + /* "file" is a symlink to existing "real_file" */ + assertEqualInt(0, mkfile("dest1/real_file", 0755, "abcdefg", 7)); + assertEqualInt(0, symlink("real_file", "dest1/file")); + /* "file2" is a symlink to non-existing "real_file2" */ + assertEqualInt(0, symlink("real_file2", "dest1/file2")); + + assertEqualInt(0, systemf("%s -xf test.tar -C dest1", testprog)); + + /* dest1/dir symlink should be removed */ + assertEqualInt(0, lstat("dest1/dir", &st)); + failure("symlink to dir was followed when it shouldn't be"); + assert(S_ISDIR(st.st_mode)); + /* dest1/dir2 symlink should be removed */ + assertEqualInt(0, lstat("dest1/dir2", &st)); + failure("Broken symlink wasn't replaced with dir"); + assert(S_ISDIR(st.st_mode)); + /* dest1/dir3 symlink should be removed */ + assertEqualInt(0, lstat("dest1/dir3", &st)); + failure("Symlink to non-dir wasn't replaced with dir"); + assert(S_ISDIR(st.st_mode)); + /* dest1/file symlink should be removed */ + assertEqualInt(0, lstat("dest1/file", &st)); + failure("Symlink to existing file should be removed"); + assert(S_ISREG(st.st_mode)); + /* dest1/file2 symlink should be removed */ + assertEqualInt(0, lstat("dest1/file2", &st)); + failure("Symlink to non-existing file should be removed"); + assert(S_ISREG(st.st_mode)); + + /* + * Extract with both -x and -P + */ + assertEqualInt(0, mkdir("dest2", 0755)); + /* "dir" is a symlink to existing "real_dir" */ + assertEqualInt(0, mkdir("dest2/real_dir", 0755)); + assertEqualInt(0, symlink("real_dir", "dest2/dir")); + /* "dir2" is a symlink to a non-existing "real_dir2" */ + assertEqualInt(0, symlink("real_dir2", "dest2/dir2")); + /* "dir3" is a symlink to an existing "non_dir3" */ + assertEqualInt(0, mkfile("dest2/non_dir3", 0755, "abcdefgh", 8)); + assertEqualInt(0, symlink("non_dir3", "dest2/dir3")); + /* "file" is a symlink to existing "real_file" */ + assertEqualInt(0, mkfile("dest2/real_file", 0755, "abcdefghi", 9)); + assertEqualInt(0, symlink("real_file", "dest2/file")); + /* "file2" is a symlink to non-existing "real_file2" */ + assertEqualInt(0, symlink("real_file2", "dest2/file2")); + + assertEqualInt(0, systemf("%s -xPf test.tar -C dest2", testprog)); + + /* dest2/dir symlink should be followed */ + assertEqualInt(0, lstat("dest2/dir", &st)); + failure("tar -xP removed symlink instead of following it"); + if (assert(S_ISLNK(st.st_mode))) { + /* Only verify what the symlink points to if it + * really is a symlink. */ + failure("The symlink should point to a directory"); + assertEqualInt(0, stat("dest2/dir", &st)); + assert(S_ISDIR(st.st_mode)); + failure("The pre-existing directory should still be there"); + assertEqualInt(0, lstat("dest2/real_dir", &st2)); + assert(S_ISDIR(st2.st_mode)); + assertEqualInt(st.st_dev, st2.st_dev); + failure("symlink should still point to the existing directory"); + assertEqualInt(st.st_ino, st2.st_ino); + } + /* Contents of 'dir' should be restored */ + assertEqualInt(0, lstat("dest2/dir/d", &st)); + assert(S_ISDIR(st.st_mode)); + assertEqualInt(0, lstat("dest2/dir/f", &st)); + assert(S_ISREG(st.st_mode)); + assertEqualInt(3, st.st_size); + /* dest2/dir2 symlink should be removed */ + assertEqualInt(0, lstat("dest2/dir2", &st)); + failure("Broken symlink wasn't replaced with dir"); + assert(S_ISDIR(st.st_mode)); + /* dest2/dir3 symlink should be removed */ + assertEqualInt(0, lstat("dest2/dir3", &st)); + failure("Symlink to non-dir wasn't replaced with dir"); + assert(S_ISDIR(st.st_mode)); + /* dest2/file symlink should be removed; + * even -P shouldn't follow symlinks for files */ + assertEqualInt(0, lstat("dest2/file", &st)); + failure("Symlink to existing file should be removed"); + assert(S_ISREG(st.st_mode)); + /* dest2/file2 symlink should be removed */ + assertEqualInt(0, lstat("dest2/file2", &st)); + failure("Symlink to non-existing file should be removed"); + assert(S_ISREG(st.st_mode)); +} Modified: stable/7/usr.bin/tar/util.c ============================================================================== --- stable/7/usr.bin/tar/util.c Thu Dec 11 05:56:25 2008 (r185908) +++ stable/7/usr.bin/tar/util.c Thu Dec 11 05:56:47 2008 (r185909) @@ -51,6 +51,7 @@ __FBSDID("$FreeBSD$"); static void bsdtar_vwarnc(struct bsdtar *, int code, const char *fmt, va_list ap); +static const char *strip_components(const char *path, int elements); /* * Print a string, taking care with any non-printable characters. @@ -346,6 +347,31 @@ do_chdir(struct bsdtar *bsdtar) bsdtar->pending_chdir = NULL; } +const char * +strip_components(const char *path, int elements) +{ + const char *p = path; + + while (elements > 0) { + switch (*p++) { + case '/': + elements--; + path = p; + break; + case '\0': + /* Path is too short, skip it. */ + return (NULL); + } + } + + while (*path == '/') + ++path; + if (*path == '\0') + return (NULL); + + return (path); +} + /* * Handle --strip-components and any future path-rewriting options. * Returns non-zero if the pathname should not be extracted. @@ -402,24 +428,20 @@ edit_pathname(struct bsdtar *bsdtar, str #endif /* Strip leading dir names as per --strip-components option. */ - if ((r = bsdtar->strip_components) > 0) { - const char *p = name; + if (bsdtar->strip_components > 0) { + const char *linkname = archive_entry_hardlink(entry); + + name = strip_components(name, bsdtar->strip_components); + if (name == NULL) + return (1); - while (r > 0) { - switch (*p++) { - case '/': - r--; - name = p; - break; - case '\0': - /* Path is too short, skip it. */ + if (linkname != NULL) { + linkname = strip_components(linkname, + bsdtar->strip_components); + if (linkname == NULL) return (1); - } + archive_entry_copy_hardlink(entry, linkname); } - while (*name == '/') - ++name; - if (*name == '\0') - return (1); } /* Strip redundant leading '/' characters. */