[pacman-dev] [PATCH] Allow query of file owners to work with non-existing files

[Allan McRae]

On 02/08/17 12:54, Vladimir Panteleev wrote:
> Previously, attempting to query the owner of a file owned by some
> package but absent from the filesystem would fail. This could lead to
> a small annoyance - if a user or misbehaving software accidentally
> deleted a file owned by some package, and the user wishes to know
> which package it belonged to so that it can be restored, pacman would
> refuse to provide this information if the file no longer exists.
> 
> Thus, allow file owner query to continue if the file does not exist.
> Print a warning in this situation and continue lookup using the exact
> user-provided path.
> 
> Add test for this functionality accordingly.
> 
> Signed-off-by: Vladimir Panteleev <archlinux at thecybershadow.net>
> ---
>  src/pacman/query.c            | 14 +++++++++++---
>  test/pacman/tests/TESTS       |  1 +
>  test/pacman/tests/query013.py | 10 ++++++++++
>  3 files changed, 22 insertions(+), 3 deletions(-)
>  create mode 100644 test/pacman/tests/query013.py
> 
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 024d3e21..bc27df8e 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -180,9 +180,16 @@ static int query_fileowner(alpm_list_t *targets)
>  					goto targcleanup;
>  				}
>  			} else {
> -				pm_printf(ALPM_LOG_ERROR, _("failed to read file '%s': %s\n"),
> -						filename, strerror(errno));
> -				goto targcleanup;
> +				if (errno == ENOENT) {
> +					pm_printf(ALPM_LOG_WARNING, _("file '%s' does not exist, not canonicalizing\n"),
> +							filename);

We should bail here if the filepath does not start in "/" since we can
not canonicalize the path.  Should we also look for "/../" in the string?

> +					strcpy(rpath, filename);
> +					goto noent;
> +				} else {
> +					pm_printf(ALPM_LOG_ERROR, _("failed to read file '%s': %s\n"),
> +							filename, strerror(errno));
> +					goto targcleanup;
> +				}
>  			}
>  		}
>  
> @@ -192,6 +199,7 @@ static int query_fileowner(alpm_list_t *targets)
>  			goto targcleanup;
>  		}
>  
> +noent:
>  		if(strncmp(rpath, root, rootlen) != 0) {
>  			/* file is outside root, we know nothing can own it */
>  			pm_printf(ALPM_LOG_ERROR, _("No package owns %s\n"), filename);
> diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
> index 309eb17e..344b1b49 100644
> --- a/test/pacman/tests/TESTS
> +++ b/test/pacman/tests/TESTS
> @@ -109,6 +109,7 @@ TESTS += test/pacman/tests/query007.py
>  TESTS += test/pacman/tests/query010.py
>  TESTS += test/pacman/tests/query011.py
>  TESTS += test/pacman/tests/query012.py
> +TESTS += test/pacman/tests/query013.py
>  TESTS += test/pacman/tests/querycheck001.py
>  TESTS += test/pacman/tests/querycheck002.py
>  TESTS += test/pacman/tests/querycheck_fast_file_type.py
> diff --git a/test/pacman/tests/query013.py b/test/pacman/tests/query013.py
> new file mode 100644
> index 00000000..b3c0a335
> --- /dev/null
> +++ b/test/pacman/tests/query013.py
> @@ -0,0 +1,10 @@
> +self.description = "Query ownership of non-existing file"
> +
> +sp = pmpkg("dummy", "1.0-1")
> +sp.files = ["etc/config"]
> +self.addpkg2db("local", sp)
> +
> +self.args = "-Qo %s/etc/config" % self.root
> +
> +self.addrule("PACMAN_RETCODE=0")
> +self.addrule("PACMAN_OUTPUT=/etc/config is owned by dummy 1.0-1$")
> 

This test passes before and after your patch...   And the etc/config
file is installed into the test root, so you are not testing a -Qo on a
missing file.