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

3 files changed, 38 insertions, 30 deletions

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);
diff --git a/tests/data/SPECS/globesctest.spec b/tests/data/SPECS/globesctest.spec
index 334d1197e..36ca516c6 100644
--- a/tests/data/SPECS/globesctest.spec
+++ b/tests/data/SPECS/globesctest.spec
@@ -18,6 +18,7 @@ BuildArch:	noarch
 %build
 touch 'foo[bar]' bar baz 'foo bar' foo%%name 'docfb[123]' 'licfb[123]'
 touch 'foo b' 'foo a' 'foo r'
+touch 'foo & bar !'
 
 %install
 mkdir -p %{buildroot}/opt
@@ -119,3 +120,6 @@ touch '%{buildroot}/opt/foo[123]'
 %doc docfb[123]
 %license licfb[123]
 /opt/foo[123]
+
+# Shell control chars (#1294)
+%doc "foo & bar !"
diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at
index e2e6c5be1..b8f39649f 100644
--- a/tests/rpmbuild.at
+++ b/tests/rpmbuild.at
@@ -527,6 +527,7 @@ runroot rpm -qpl /build/RPMS/noarch/globesctest-1.0-1.noarch.rpm
 /opt/share/doc/globesctest-1.0/bar
 /opt/share/doc/globesctest-1.0/baz
 /opt/share/doc/globesctest-1.0/docfb[[123]]
+/opt/share/doc/globesctest-1.0/foo & bar !
 /opt/share/doc/globesctest-1.0/foo a
 /opt/share/doc/globesctest-1.0/foo b
 /opt/share/doc/globesctest-1.0/foo bar
@@ -2013,12 +2014,12 @@ runroot rpmbuild -bb --quiet /data/SPECS/filemiss.spec
 [error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/doc/filemisstest-1.0/CREDITS
 error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/foo
 error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/bar{a,b}
-cp: cannot stat 'INSTALL': No such file or directory
-cp: cannot stat 'README*': No such file or directory
+cp: cannot stat '/build/BUILD/INSTALL': No such file or directory
+cp: cannot stat '/build/BUILD/README*': No such file or directory
 error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/doc/filemisstest-1.0/INSTALL
 error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/doc/filemisstest-1.0/README*
-cp: cannot stat 'LICENSE': No such file or directory
-cp: cannot stat 'OTHERLICENSE?': No such file or directory
+cp: cannot stat '/build/BUILD/LICENSE': No such file or directory
+cp: cannot stat '/build/BUILD/OTHERLICENSE?': No such file or directory
 error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/licenses/filemisstest-1.0/LICENSE
 error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/licenses/filemisstest-1.0/OTHERLICENSE?
 ],