[pacman-dev] [PATCH] Detect potental confict when symlink to directory is changing to directory
When a symlink to a directory is changing to a directory and takes a file with it, pacman does not result the path of the file on the filesystem when checking for conflicts. Reported by Neofytos and Luca from Chakra. Signed-off-by: Allan McRae <allan@archlinux.org> --- This only works based on pacmans return failure. Why is the usr/include/bar being detected as a directory and not a symlink? test/pacman/tests/TESTS | 1 + test/pacman/tests/symlink021.py | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 test/pacman/tests/symlink021.py diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index e330896..bee7c8e 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -150,6 +150,7 @@ TESTS += test/pacman/tests/symlink010.py TESTS += test/pacman/tests/symlink011.py TESTS += test/pacman/tests/symlink012.py TESTS += test/pacman/tests/symlink020.py +TESTS += test/pacman/tests/symlink021.py TESTS += test/pacman/tests/sync-install-assumeinstalled.py TESTS += test/pacman/tests/sync-nodepversion01.py TESTS += test/pacman/tests/sync-nodepversion02.py diff --git a/test/pacman/tests/symlink021.py b/test/pacman/tests/symlink021.py new file mode 100644 index 0000000..800094d --- /dev/null +++ b/test/pacman/tests/symlink021.py @@ -0,0 +1,27 @@ +self.description = "symlink -> dir replacment with file move" + +lp1 = pmpkg("pkg1") +lp1.files = ["usr/include/foo/", + "usr/include/bar -> foo", + "usr/include/foo/header.h"] +self.addpkg2db("local", lp1) + +sp1 = pmpkg("pkg1", "1.0-2") +sp1.files = ["usr/include/foo/"] +self.addpkg2db("sync", sp1) + +sp2 = pmpkg("pkg2", "1.0-2") +sp2.files = ["usr/include/bar/", + "usr/include/bar/header.h"] +self.addpkg2db("sync", sp2) + + +self.args = "-S %s %s" % (sp1.name, sp2.name) + +self.addrule("PACMAN_RETCODE=0") +self.addrule("FILE_TYPE=usr/include/foo|dir") +self.addrule("FILE_TYPE=usr/include/bar|dir") +self.addrule("FILE_EXIST=usr/include/foo/header.h") +self.addrule("FILE_EXIST=usr/include/bar/header.h") + +self.expectfailure = True -- 2.6.3
s/potental/potential/ s/confict/conflict/ On 12/04/15 at 07:41am, Allan McRae wrote:
When a symlink to a directory is changing to a directory and takes a file with it, pacman does not result the path of the file on the filesystem when checking
The paths in question actually just need to be skipped altogether, not resolved.
for conflicts.
Reported by Neofytos and Luca from Chakra.
Signed-off-by: Allan McRae <allan@archlinux.org> ---
This only works based on pacmans return failure. Why is the usr/include/bar being detected as a directory and not a symlink?
test/pacman/tests/TESTS | 1 + test/pacman/tests/symlink021.py | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 test/pacman/tests/symlink021.py
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index e330896..bee7c8e 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -150,6 +150,7 @@ TESTS += test/pacman/tests/symlink010.py TESTS += test/pacman/tests/symlink011.py TESTS += test/pacman/tests/symlink012.py TESTS += test/pacman/tests/symlink020.py +TESTS += test/pacman/tests/symlink021.py TESTS += test/pacman/tests/sync-install-assumeinstalled.py TESTS += test/pacman/tests/sync-nodepversion01.py TESTS += test/pacman/tests/sync-nodepversion02.py diff --git a/test/pacman/tests/symlink021.py b/test/pacman/tests/symlink021.py
Our more recent tests use more descriptive names.
new file mode 100644 index 0000000..800094d --- /dev/null +++ b/test/pacman/tests/symlink021.py @@ -0,0 +1,27 @@ +self.description = "symlink -> dir replacment with file move" + +lp1 = pmpkg("pkg1") +lp1.files = ["usr/include/foo/", + "usr/include/bar -> foo", + "usr/include/foo/header.h"] +self.addpkg2db("local", lp1) + +sp1 = pmpkg("pkg1", "1.0-2") +sp1.files = ["usr/include/foo/"] +self.addpkg2db("sync", sp1) + +sp2 = pmpkg("pkg2", "1.0-2") +sp2.files = ["usr/include/bar/", + "usr/include/bar/header.h"] +self.addpkg2db("sync", sp2) + + +self.args = "-S %s %s" % (sp1.name, sp2.name) + +self.addrule("PACMAN_RETCODE=0") +self.addrule("FILE_TYPE=usr/include/foo|dir") +self.addrule("FILE_TYPE=usr/include/bar|dir") +self.addrule("FILE_EXIST=usr/include/foo/header.h")
usr/include/foo/header.h is not in the updated package's file list.
+self.addrule("FILE_EXIST=usr/include/bar/header.h") + +self.expectfailure = True -- 2.6.3
On 05/12/15 05:13, Andrew Gregory wrote:
s/potental/potential/ s/confict/conflict/
On 12/04/15 at 07:41am, Allan McRae wrote:
When a symlink to a directory is changing to a directory and takes a file with it, pacman does not result the path of the file on the filesystem when checking
The paths in question actually just need to be skipped altogether, not resolved.
for conflicts.
Reported by Neofytos and Luca from Chakra.
Signed-off-by: Allan McRae <allan@archlinux.org> ---
This only works based on pacmans return failure. Why is the usr/include/bar being detected as a directory and not a symlink?
test/pacman/tests/TESTS | 1 + test/pacman/tests/symlink021.py | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 test/pacman/tests/symlink021.py
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index e330896..bee7c8e 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -150,6 +150,7 @@ TESTS += test/pacman/tests/symlink010.py TESTS += test/pacman/tests/symlink011.py TESTS += test/pacman/tests/symlink012.py TESTS += test/pacman/tests/symlink020.py +TESTS += test/pacman/tests/symlink021.py TESTS += test/pacman/tests/sync-install-assumeinstalled.py TESTS += test/pacman/tests/sync-nodepversion01.py TESTS += test/pacman/tests/sync-nodepversion02.py diff --git a/test/pacman/tests/symlink021.py b/test/pacman/tests/symlink021.py
Our more recent tests use more descriptive names.
Thanks for the fixes. This one is a partner to symlink020.py so I kept the name. A
libarchive will not extract a directory over an existing directory symlink, making it impossible to replace a symlink with a directory across packages. Adding the ARCHIVE_EXTRACT_UNLINK and ARCHIVE_EXTRACT_SECURE_SYMLINKS causes libarchive to unlink the existing symlink and prevents it from extracting any paths that contain a symlink, which we should not be doing anyway. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/add.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 2684e94..63eda49 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -112,7 +112,9 @@ static int perform_extraction(alpm_handle_t *handle, struct archive *archive, int ret; const int archive_flags = ARCHIVE_EXTRACT_OWNER | ARCHIVE_EXTRACT_PERM | - ARCHIVE_EXTRACT_TIME; + ARCHIVE_EXTRACT_TIME | + ARCHIVE_EXTRACT_UNLINK | + ARCHIVE_EXTRACT_SECURE_SYMLINKS; archive_entry_set_pathname(entry, filename); @@ -301,13 +303,6 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, strcpy(filename + filename_len, ".pacnew"); } - if(handle->trans->flags & ALPM_TRANS_FLAG_FORCE) { - /* if FORCE was used, unlink() each file (whether it's there - * or not) before extracting. This prevents the old "Text file busy" - * error that crops up if forcing a glibc or pacman upgrade. */ - unlink(filename); - } - _alpm_log(handle, ALPM_LOG_DEBUG, "extracting %s\n", filename); if(perform_extraction(handle, archive, entry, filename)) { errors++; -- 2.6.3
When replacing a file with a directory, any files inside the new directory cannot possibly exist on the filesystem and can be skipped. This allows cross-package symlink-to-directory transitions when there are files with the same name under both the symlinked directory and the new directory. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/conflict.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 41b8393..a70023b 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -566,6 +566,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, /* localp2->files will be removed (target conflicts are handled by CHECK 1) */ if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), relative_path)) { + size_t fslen = strlen(filestr); + /* skip removal of file, but not add. this will prevent a second * package from removing the file when it was already installed * by its new owner (whether the file is in backup array or not */ @@ -574,6 +576,19 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, _alpm_log(handle, ALPM_LOG_DEBUG, "file changed packages, adding to remove skiplist\n"); resolved_conflict = 1; + + if(filestr[fslen - 1] == '/') { + /* replacing a file with a directory: + * go ahead and skip any files inside filestr as they will + * necessarily be resolved by replacing the file with a dir + * NOTE: afterward, j will point to the last file inside filestr */ + for( ; j->next; j = j->next) { + const char *filestr2 = j->next->data; + if(strncmp(filestr, filestr2, fslen) != 0) { + break; + } + } + } } } -- 2.6.3
participants (2)
-
Allan McRae
-
Andrew Gregory