Skip to content

Commit 7f7c833

Browse files
mmatuskammatuska
committed
Fix various symlink bugs with Windows symlinks
tests: extend assertMakeSymlink with targetIsDir
1 parent c182493 commit 7f7c833

16 files changed

Lines changed: 130 additions & 77 deletions

‎cpio/test/test_basic.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ DEFINE_TEST(test_basic)
173173

174174
/* Symlink to above file. */
175175
if (canSymlink()) {
176-
assertMakeSymlink("symlink", "file");
176+
assertMakeSymlink("symlink", "file", 0);
177177
fprintf(filelist, "symlink\n");
178178
if (is_LargeInode("symlink")) {
179179
strncat(result,

‎cpio/test/test_format_newc.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ DEFINE_TEST(test_format_newc)
114114

115115
/* "symlink" */
116116
if (canSymlink()) {
117-
assertMakeSymlink("symlink", "file1");
117+
assertMakeSymlink("symlink", "file1", 0);
118118
fprintf(list, "symlink\n");
119119
}
120120

‎cpio/test/test_option_L_upper.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ DEFINE_TEST(test_option_L_upper)
5151
fprintf(filelist, "file\n");
5252

5353
/* Symlink to above file. */
54-
assertMakeSymlink("symlink", "file");
54+
assertMakeSymlink("symlink", "file", 0);
5555
fprintf(filelist, "symlink\n");
5656

5757
fclose(filelist);

‎cpio/test/test_option_c.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ DEFINE_TEST(test_option_c)
8585

8686
/* "symlink" */
8787
if (canSymlink()) {
88-
assertMakeSymlink("symlink", "file");
88+
assertMakeSymlink("symlink", "file", 0);
8989
fprintf(filelist, "symlink\n");
9090
}
9191

‎libarchive/archive_write_disk_windows.c‎

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,28 +1454,45 @@ restore_entry(struct archive_write_disk *a)
14541454
en = create_filesystem_object(a);
14551455
} else if (en == EEXIST) {
14561456
mode_t st_mode;
1457+
mode_t lst_mode;
1458+
BY_HANDLE_FILE_INFORMATION lst;
14571459
/*
14581460
* We know something is in the way, but we don't know what;
14591461
* we need to find out before we go any further.
14601462
*/
14611463
int r = 0;
1464+
int dirlnk = 0;
1465+
14621466
/*
14631467
* The SECURE_SYMLINK logic has already removed a
14641468
* symlink to a dir if the client wants that. So
14651469
* follow the symlink if we're creating a dir.
1466-
*/
1467-
if (S_ISDIR(a->mode))
1468-
r = file_information(a, a->name, &a->st, &st_mode, 0);
1469-
/*
14701470
* If it's not a dir (or it's a broken symlink),
14711471
* then don't follow it.
1472+
*
1473+
* Windows distinguishes file and directory symlinks.
1474+
* A file symlink may erroneously point to a directory
1475+
* and a directory symlink to a file. Windows does not follow
1476+
* such symlinks. We always need both source and target
1477+
* information.
14721478
*/
1473-
if (r != 0 || !S_ISDIR(a->mode))
1474-
r = file_information(a, a->name, &a->st, &st_mode, 1);
1479+
r = file_information(a, a->name, &lst, &lst_mode, 1);
14751480
if (r != 0) {
14761481
archive_set_error(&a->archive, errno,
14771482
"Can't stat existing object");
14781483
return (ARCHIVE_FAILED);
1484+
} else if (S_ISLNK(lst_mode)) {
1485+
if (lst.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
1486+
dirlnk = 1;
1487+
/* In case of a symlink we need target information */
1488+
r = file_information(a, a->name, &a->st, &st_mode, 0);
1489+
if (r != 0) {
1490+
a->st = lst;
1491+
st_mode = lst_mode;
1492+
}
1493+
} else {
1494+
a->st = lst;
1495+
st_mode = lst_mode;
14791496
}
14801497

14811498
/*
@@ -1499,8 +1516,15 @@ restore_entry(struct archive_write_disk *a)
14991516
}
15001517

15011518
if (!S_ISDIR(st_mode)) {
1502-
/* A non-dir is in the way, unlink it. */
1503-
if (disk_unlink(a->name) != 0) {
1519+
/* Edge case: a directory symlink pointing to a file */
1520+
if (dirlnk) {
1521+
if (disk_rmdir(a->name) != 0) {
1522+
archive_set_error(&a->archive, errno,
1523+
"Can't unlink directory symlink");
1524+
return (ARCHIVE_FAILED);
1525+
}
1526+
} else if (disk_unlink(a->name) != 0) {
1527+
/* A non-dir is in the way, unlink it. */
15041528
archive_set_error(&a->archive, errno,
15051529
"Can't unlink already-existing object");
15061530
return (ARCHIVE_FAILED);
@@ -1927,6 +1951,9 @@ check_symlinks(struct archive_write_disk *a)
19271951
p = a->path_safe.s;
19281952
while ((*pn != '\0') && (*p == *pn))
19291953
++p, ++pn;
1954+
/* Skip leading backslashes */
1955+
while (*pn == '\\')
1956+
++pn;
19301957
c = pn[0];
19311958
/* Keep going until we've checked the entire name. */
19321959
while (pn[0] != '\0' && (pn[0] != '\\' || pn[1] != '\0')) {
@@ -1944,8 +1971,8 @@ check_symlinks(struct archive_write_disk *a)
19441971
} else if (S_ISLNK(st_mode)) {
19451972
if (c == '\0') {
19461973
/*
1947-
* Last element is symlink; remove it
1948-
* so we can overwrite it with the
1974+
* Last element is a file or directory symlink.
1975+
* Remove it so we can overwrite it with the
19491976
* item being extracted.
19501977
*/
19511978
if (st.dwFileAttributes &
@@ -1978,7 +2005,13 @@ check_symlinks(struct archive_write_disk *a)
19782005
return (0);
19792006
} else if (a->flags & ARCHIVE_EXTRACT_UNLINK) {
19802007
/* User asked us to remove problems. */
1981-
if (disk_unlink(a->name) != 0) {
2008+
if (st.dwFileAttributes &
2009+
FILE_ATTRIBUTE_DIRECTORY) {
2010+
r = disk_rmdir(a->name);
2011+
} else {
2012+
r = disk_unlink(a->name);
2013+
}
2014+
if (r != 0) {
19822015
archive_set_error(&a->archive, 0,
19832016
"Cannot remove intervening "
19842017
"symlink %ls", a->name);
@@ -1994,6 +2027,8 @@ check_symlinks(struct archive_write_disk *a)
19942027
return (ARCHIVE_FAILED);
19952028
}
19962029
}
2030+
pn[0] = c;
2031+
pn++;
19972032
}
19982033
pn[0] = c;
19992034
/* We've checked and/or cleaned the whole path, so remember it. */

‎libarchive/test/test_read_disk_directory_traversals.c‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -570,13 +570,13 @@ test_symlink_hybrid(void)
570570
assertMakeDir("h", 0755);
571571
assertChdir("h");
572572
assertMakeDir("d1", 0755);
573-
assertMakeSymlink("ld1", "d1/");
573+
assertMakeSymlink("ld1", "d1", 1);
574574
assertMakeFile("d1/file1", 0644, "d1/file1");
575575
assertMakeFile("d1/file2", 0644, "d1/file2");
576-
assertMakeSymlink("d1/link1", "file1");
577-
assertMakeSymlink("d1/linkX", "fileX");
578-
assertMakeSymlink("link2", "d1/file2");
579-
assertMakeSymlink("linkY", "d1/fileY");
576+
assertMakeSymlink("d1/link1", "file1", 0);
577+
assertMakeSymlink("d1/linkX", "fileX", 0);
578+
assertMakeSymlink("link2", "d1/file2", 0);
579+
assertMakeSymlink("linkY", "d1/fileY", 0);
580580
assertChdir("..");
581581

582582
assert((ae = archive_entry_new()) != NULL);
@@ -727,13 +727,13 @@ test_symlink_logical(void)
727727
assertMakeDir("l", 0755);
728728
assertChdir("l");
729729
assertMakeDir("d1", 0755);
730-
assertMakeSymlink("ld1", "d1/");
730+
assertMakeSymlink("ld1", "d1", 1);
731731
assertMakeFile("d1/file1", 0644, "d1/file1");
732732
assertMakeFile("d1/file2", 0644, "d1/file2");
733-
assertMakeSymlink("d1/link1", "file1");
734-
assertMakeSymlink("d1/linkX", "fileX");
735-
assertMakeSymlink("link2", "d1/file2");
736-
assertMakeSymlink("linkY", "d1/fileY");
733+
assertMakeSymlink("d1/link1", "file1", 0);
734+
assertMakeSymlink("d1/linkX", "fileX", 0);
735+
assertMakeSymlink("link2", "d1/file2", 0);
736+
assertMakeSymlink("linkY", "d1/fileY", 0);
737737
assertChdir("..");
738738

739739
/* Note: this test uses archive_read_next_header()
@@ -961,8 +961,8 @@ test_symlink_logical_loop(void)
961961
assertMakeDir("d1/d2/d3", 0755);
962962
assertMakeDir("d2", 0755);
963963
assertMakeFile("d2/file1", 0644, "d2/file1");
964-
assertMakeSymlink("d1/d2/ld1", "../../d1/");
965-
assertMakeSymlink("d1/d2/ld2", "../../d2/");
964+
assertMakeSymlink("d1/d2/ld1", "../../d1", 1);
965+
assertMakeSymlink("d1/d2/ld2", "../../d2", 1);
966966
assertChdir("..");
967967

968968
assert((ae = archive_entry_new()) != NULL);

‎tar/test/test_basic.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ make_files(void)
4242

4343
/* Symlink to above file. */
4444
if (canSymlink())
45-
assertMakeSymlink("symlink", "file");
45+
assertMakeSymlink("symlink", "file", 0);
4646

4747
/* Directory. */
4848
assertMakeDir("dir", 0775);

‎tar/test/test_copy.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ create_tree(void)
176176
sprintf(buff, "s/%s", filenames[i]);
177177
sprintf(buff2, "../f/%s", filenames[i]);
178178
failure("buff=\"%s\" buff2=\"%s\"", buff, buff2);
179-
assertMakeSymlink(buff, buff2);
179+
assertMakeSymlink(buff, buff2, 0);
180180
}
181181
/* Create a dir named "d/abcdef...". */
182182
buff[0] = 'd';

‎tar/test/test_option_H_upper.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ DEFINE_TEST(test_option_H_upper)
3939
assertMakeDir("in", 0755);
4040
assertChdir("in");
4141
assertMakeDir("d1", 0755);
42-
assertMakeSymlink("ld1", "d1");
42+
assertMakeSymlink("ld1", "d1", 1);
4343
assertMakeFile("d1/file1", 0644, "d1/file1");
4444
assertMakeFile("d1/file2", 0644, "d1/file2");
45-
assertMakeSymlink("d1/link1", "file1");
46-
assertMakeSymlink("d1/linkX", "fileX");
47-
assertMakeSymlink("link2", "d1/file2");
48-
assertMakeSymlink("linkY", "d1/fileY");
45+
assertMakeSymlink("d1/link1", "file1", 0);
46+
assertMakeSymlink("d1/linkX", "fileX", 0);
47+
assertMakeSymlink("link2", "d1/file2", 0);
48+
assertMakeSymlink("linkY", "d1/fileY", 0);
4949
assertChdir("..");
5050

5151
/* Test 1: Without -H */

‎tar/test/test_option_L_upper.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ DEFINE_TEST(test_option_L_upper)
3939
assertMakeDir("in", 0755);
4040
assertChdir("in");
4141
assertMakeDir("d1", 0755);
42-
assertMakeSymlink("ld1", "d1");
42+
assertMakeSymlink("ld1", "d1", 1);
4343
assertMakeFile("d1/file1", 0644, "d1/file1");
4444
assertMakeFile("d1/file2", 0644, "d1/file2");
45-
assertMakeSymlink("d1/link1", "file1");
46-
assertMakeSymlink("d1/linkX", "fileX");
47-
assertMakeSymlink("link2", "d1/file2");
48-
assertMakeSymlink("linkY", "d1/fileY");
45+
assertMakeSymlink("d1/link1", "file1", 0);
46+
assertMakeSymlink("d1/linkX", "fileX", 0);
47+
assertMakeSymlink("link2", "d1/file2", 0);
48+
assertMakeSymlink("linkY", "d1/fileY", 0);
4949
assertChdir("..");
5050

5151
/* Test 1: Without -L */

0 commit comments

Comments
 (0)