cp: Never follow symbolic links in destination.

Historically, BSD cp has followed symbolic links in the destination
when copying recursively, while GNU cp has not.  POSIX is somewhat
vague on the topic, but both interpretations are within bounds.  In
33ad990ce9, cp was changed to apply the same logic for symbolic
links in the destination as for symbolic links in the source: follow
if not recursing (which is moot, as this situation can only arise
while recursing) or if the `-L` option was given.  There is no support
for this in POSIX.  We can either switch back, or go all the way.

Having carefully weighed the kind of trouble you can run into by
following unexpected symlinks up against the kind of trouble you can
run into by not following symlinks you expected to follow, we choose
to go all the way.

Note that this means we need to stat the destination twice: once,
following links, to check if it is or references the same file as the
source, and a second time, not following links, to set the dne flag
and determine the destination's type.

While here, remove a needless complication in the dne logic.  We don't
need to explicitly reject overwriting a directory with a non-directory,
because it will fail anyway.

Finally, add test cases for copying a directory to a symlink and
overwriting a directory with a non-directory.

MFC after:	never
Relnotes: 	yes
Sponsored by:	Klara, Inc.
Reviewed by:	kevans
Differential Revision:	https://reviews.freebsd.org/D44578
This commit is contained in:
Dag-Erling Smørgrav 2024-04-09 00:41:33 +02:00
parent d80a97def9
commit 0729d1e8fd
2 changed files with 44 additions and 39 deletions

View File

@ -260,22 +260,6 @@ main(int argc, char *argv[])
&to_stat)));
}
/* Does the right thing based on -R + -H/-L/-P */
static int
copy_stat(const char *path, struct stat *sb)
{
/*
* For -R -H/-P, we need to lstat() instead; copy() cares about the link
* itself rather than the target if we're not following links during the
* traversal.
*/
if (!Rflag || Lflag)
return (stat(path, sb));
return (lstat(path, sb));
}
static int
copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
{
@ -403,7 +387,6 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
continue;
}
if (asprintf(&recurse_path, "%s/%s", to.p_path,
rootname) == -1)
err(1, "asprintf");
@ -452,30 +435,21 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
continue;
}
/* Not an error but need to remember it happened. */
if (copy_stat(to.p_path, &to_stat) == -1)
dne = 1;
else {
if (to_stat.st_dev == curr->fts_statp->st_dev &&
to_stat.st_ino == curr->fts_statp->st_ino) {
warnx("%s and %s are identical (not copied).",
to.p_path, curr->fts_path);
badcp = rval = 1;
if (S_ISDIR(curr->fts_statp->st_mode))
(void)fts_set(ftsp, curr, FTS_SKIP);
continue;
}
if (!S_ISDIR(curr->fts_statp->st_mode) &&
S_ISDIR(to_stat.st_mode)) {
warnx("cannot overwrite directory %s with "
"non-directory %s",
to.p_path, curr->fts_path);
badcp = rval = 1;
continue;
}
dne = 0;
/* Check if source and destination are identical. */
if (stat(to.p_path, &to_stat) == 0 &&
to_stat.st_dev == curr->fts_statp->st_dev &&
to_stat.st_ino == curr->fts_statp->st_ino) {
warnx("%s and %s are identical (not copied).",
to.p_path, curr->fts_path);
badcp = rval = 1;
if (S_ISDIR(curr->fts_statp->st_mode))
(void)fts_set(ftsp, curr, FTS_SKIP);
continue;
}
/* Not an error but need to remember it happened. */
dne = lstat(to.p_path, &to_stat) != 0;
switch (curr->fts_statp->st_mode & S_IFMT) {
case S_IFLNK:
/* Catch special case of a non-dangling symlink. */

View File

@ -363,6 +363,35 @@ symlink_exists_force_body()
atf_check -o inline:"foo\n" readlink bar
}
atf_test_case directory_to_symlink
directory_to_symlink_body()
{
mkdir -p foo
ln -s .. foo/bar
mkdir bar
touch bar/baz
atf_check -s not-exit:0 -e match:"Not a directory" \
cp -R bar foo
atf_check -s not-exit:0 -e match:"Not a directory" \
cp -r bar foo
}
atf_test_case overwrite_directory
overwrite_directory_body()
{
mkdir -p foo/bar/baz
touch bar
atf_check -s not-exit:0 -e match:"Is a directory" \
cp bar foo
rm bar
mkdir bar
touch bar/baz
atf_check -s not-exit:0 -e match:"Is a directory" \
cp -R bar foo
atf_check -s not-exit:0 -e match:"Is a directory" \
cp -r bar foo
}
atf_init_test_cases()
{
atf_add_test_case basic
@ -388,4 +417,6 @@ atf_init_test_cases()
atf_add_test_case symlink
atf_add_test_case symlink_exists
atf_add_test_case symlink_exists_force
atf_add_test_case directory_to_symlink
atf_add_test_case overwrite_directory
}