From a06374be2f85e457cb526edb8ead40f57c82bfd6 Mon Sep 17 00:00:00 2001
From: Michal Domonkos <mdomonko@redhat.com>
Date: Mon, 21 Nov 2022 17:36:21 +0100
Subject: Glob special files only once, internally

Currently, special files are globbed twice during build.  When copying
them from builddir to buildroot using the %doc/%license script, we let
the shell handle the globbing as we pass the patterns verbatim to cp(1).
Then, when adding the copied files from buildroot to the internal file
list, we use rpmGlob() to expand the same patterns in builddir again.

This is not only redundant but also potentially error prone as it
assumes both shell globbing and rpmGlob() will yield the same results.
And most importantly, it means we can't just quote the cp commands to
allow the use of shell control chars like '&' in filenames.  Selectively
escaping those would be possible but still too brittle.

To fix all that, just use rpmGlob() first and then pass the matches one
by one to cp, all quoted.

A side effect of this change is that there will be one cp command per
match, as opposed to one cp command per file entry with matches as
arguments, and the paths will be absolute.  This will make build logs a
tiny bit more verbose but also more explicit and safe from hitting the
maximum number of arguments in a single shell command (although that's
more of a theoretical scenario, not likely with real packages).

Fixes: #1294
---
 build/files.c | 55 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

(limited to 'build')

diff --git a/build/files.c b/build/files.c
index aab128aaa..cc9c5993d 100644
--- a/build/files.c
+++ b/build/files.c
@@ -2410,7 +2410,9 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl,
     const char *sdname = (sd->sdtype == RPMFILE_DOC) ? "%doc" : "%license";
     char *mkdocdir = rpmExpand("%{__mkdir_p} $", sdenv, NULL);
     StringBuf docScript = newStringBuf();
-    char *basepath, **files;
+    char *basepath;
+    ARGV_t *files;
+    int count = sd->entriesCount;
     int fi;
 
     appendStringBuf(docScript, sdenv);
@@ -2421,16 +2423,27 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl,
     appendLineStringBuf(docScript, sdenv);
     appendLineStringBuf(docScript, mkdocdir);
 
+    basepath = rpmGenPath(spec->rootDir, "%{_builddir}", "%{?buildsubdir}");
+    files = xmalloc(sizeof(*files) * count);
+    fi = 0;
     for (ARGV_const_t fn = sd->files; fn && *fn; fn++) {
-	/* Quotes would break globs, escape spaces instead */
-	char *efn = rpmEscapeSpaces(*fn);
-	appendStringBuf(docScript, "cp -pr ");
-	appendStringBuf(docScript, efn);
-	appendStringBuf(docScript, " $");
-	appendStringBuf(docScript, sdenv);
-	appendLineStringBuf(docScript, " ||:");
-	free(efn);
+	char *origfile = rpmCleanPath(rstrscat(NULL, basepath, "/", *fn, NULL));
+	ARGV_t globFiles = NULL;
+	int globFilesCount, i;
+	if (rpmGlobPath(origfile, RPMGLOB_NOCHECK,
+			&globFilesCount, &globFiles) == 0) {
+	    for (i = 0; i < globFilesCount; i++) {
+		appendStringBuf(docScript, "cp -pr '");
+		appendStringBuf(docScript, globFiles[i]);
+		appendStringBuf(docScript, "' $");
+		appendStringBuf(docScript, sdenv);
+		appendLineStringBuf(docScript, " ||:");
+	    }
+	}
+	free(origfile);
+	files[fi++] = globFiles;
     }
+    free(basepath);
 
     if (install) {
 	if (doScript(spec, RPMBUILD_STRINGBUF, sdname,
@@ -2439,13 +2452,8 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl,
 	}
     }
 
-    basepath = rpmGenPath(spec->rootDir, "%{_builddir}", "%{?buildsubdir}");
-    files = sd->files;
     fi = 0;
-    while (*files != NULL) {
-	char *origfile = rpmCleanPath(rstrscat(NULL, basepath, "/", *files, NULL));
-	ARGV_t globFiles = NULL;
-	int globFilesCount, i;
+    for (int i = 0; i < count; i++) {
 	char *newfile;
 
 	FileEntryFree(&fl->cur);
@@ -2454,19 +2462,14 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl,
 	copyFileEntry(&sd->entries[fi].defEntry, &fl->def);
 	fi++;
 
-	if (rpmGlobPath(origfile, RPMGLOB_NOCHECK,
-			&globFilesCount, &globFiles) == 0) {
-	    for (i = 0; i < globFilesCount; i++) {
-		rasprintf(&newfile, "%s/%s", sd->dirname, basename(globFiles[i]));
-		processBinaryFile(pkg, fl, newfile, 0);
-		free(newfile);
-	    }
-	    argvFree(globFiles);
+	for (ARGV_const_t fn = files[i]; fn && *fn; fn++) {
+	    rasprintf(&newfile, "%s/%s", sd->dirname, basename(*fn));
+	    processBinaryFile(pkg, fl, newfile, 0);
+	    free(newfile);
 	}
-	free(origfile);
-	files++;
+	argvFree(files[i]);
     }
-    free(basepath);
+    free(files);
 
     FileEntryFree(&fl->cur);
     FileEntryFree(&fl->def);
-- 
cgit v1.2.1