Hi Konrad,
You're right, it's better to include a test. I've added one to the patch.
Aaron
On Tue, Feb 11, 2020 at 2:01 PM Konrad Kleine <kkle...@redhat.com> wrote:
>
> Can we have a test for this?
From de46a13aa67d65d61ae206d1ecf8b20f5d3623cf Mon Sep 17 00:00:00 2001
From: Aaron Merey <ame...@redhat.com>
Date: Tue, 11 Feb 2020 15:48:46 -0500
Subject: [PATCH] debuginfod-client: restrict cleanup to client-pattern files
Avoid deleting general files and directories in case a user
mis-sets $DEBUGINFOD_CACHE_PATH or accidentally moves a file
into the cache.
Signed-off-by: Aaron Merey <ame...@redhat.com>
---
debuginfod/ChangeLog | 5 +++++
debuginfod/debuginfod-client.c | 13 ++++++++++++-
tests/ChangeLog | 5 +++++
tests/run-debuginfod-find.sh | 12 ++++++++++++
4 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index d812e6d7..b5ff2525 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2020-02-11 Aaron Merey <ame...@redhat.com>
+
+ * debuginfod-client.c (debuginfod_clean_cache): Restrict
+ cleanup to client-pattern files.
+
2020-02-05 Frank Ch. Eigler <f...@redhat.com>
* debuginfod.cxx (argp options): Add -Z option.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index e5a2e824..186aa90a 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -50,6 +50,7 @@
#include <errno.h>
#include <fcntl.h>
#include <fts.h>
+#include <regex.h>
#include <string.h>
#include <stdbool.h>
#include <linux/limits.h>
@@ -241,10 +242,19 @@ debuginfod_clean_cache(debuginfod_client *c,
if (fts == NULL)
return -errno;
+ regex_t re;
+ const char * pattern = ".*/[a-f0-9]+/(debuginfo|executable|source.*)$";
+ if (regcomp (&re, pattern, REG_EXTENDED | REG_NOSUB) != 0)
+ return -ENOMEM;
+
FTSENT *f;
long files = 0;
while ((f = fts_read(fts)) != NULL)
{
+ /* ignore any files that do not match the pattern. */
+ if (regexec (&re, f->fts_path, 0, NULL, 0) != 0)
+ continue;
+
files++;
if (c->progressfn) /* inform/check progress callback */
if ((c->progressfn) (c, files, 0))
@@ -268,7 +278,8 @@ debuginfod_clean_cache(debuginfod_client *c,
;
}
}
- fts_close(fts);
+ fts_close (fts);
+ regfree (&re);
/* Update timestamp representing when the cache was last cleaned. */
utime (interval_path, NULL);
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 84953adb..49b93352 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2020-02-11 Aaron Merey <ame...@redhat.com>
+
+ * run-debuginfod-find.sh: Test that files unrelated to debuginfod
+ survive cache cleaning.
+
2020-02-05 Frank Ch. Eigler <f...@redhat.com>
* debuginfo-tars/*: New test files from Eli Schwartz of ArchLinux.
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 1cc8f406..2964e7c0 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -365,6 +365,12 @@ testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog2 1
########################################################################
+# Add some files to the cache that do not fit its naming format.
+# They should survive cache cleaning.
+mkdir $DEBUGINFOD_CACHE_PATH/malformed
+touch $DEBUGINFOD_CACHE_PATH/malformed0
+touch $DEBUGINFOD_CACHE_PATH/malformed/malformed1
+
# Trigger a cache clean and run the tests again. The clients should be unable to
# find the target.
echo 0 > $DEBUGINFOD_CACHE_PATH/cache_clean_interval_s
@@ -374,6 +380,12 @@ testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1
testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID2 && false || true
+if [ ! -f $DEBUGINFOD_CACHE_PATH/malformed0 ] \
+ || [ ! -f $DEBUGINFOD_CACHE_PATH/malformed/malformed1 ]; then
+ echo "unrelated files did not survive cache cleaning"
+ exit 1
+fi
+
# Test debuginfod without a path list; reuse $PORT1
env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../debuginfod/debuginfod $VERBOSE -F -U -d :memory: -p $PORT1 -L -F &
PID3=$!
--
2.24.1
