Preserve literal value of quoted %files entries

Turn off the special meaning of metacharacters within a file name
enclosed in single or double quotes.  Do it by automatically escaping
such characters at parse time, as that internally reduces the scenario
to the one where the user manually escapes the file name in the spec
file (i.e. fewer code paths to worry about), as well as allowing for
implementing partial quoting (e.g. /foo"*") in the future if we so
choose.  Currently, only fully quoted file names are supported.  Quotes
within quotes of the same type can be escaped.

This is a change in quote semantics that should bring it closer to user
expectations as well as providing a more convenient way of escaping a
lot of characters at once.  The negative impact on existing spec files
should be minimal, if any, since the only reason to quote a file name
right now would be to escape spaces, and that continues to work as
expected.

One use case that could break is a file that's quoted but contains
metacharacters which would previously be expanded (e.g. "/foo[abc]").
Depending on the actual build root contents, this may cause an
unpredictable and/or silent change in the resulting file list at build
time, compared to the previous builds of the same package.

Note that we must keep the behavior of rpmEscapeSpaces() exactly as it
is and since isspace(3) is locale-dependent, we need to allow for it to
still be used through rpmEscapeChars() while also allowing the caller to
specify a static string of characters to escape, hence accepting two
argument types to choose from.

Fixes: rpm-software-management#1749

build/files.c

@@ -238,10 +238,11 @@ static void FileEntryFree(FileEntry entry)
 
 /**
  * strtokWithQuotes.
- * @param s
- * @param delim
+ * @param s		string to tokenize
+ * @param delim		delimiter chars
+ * @param quotes	0 if unquoted, 1 if missing quote, 2 if quoted
  */
-static char *strtokWithQuotes(char *s, const char *delim)
+static char *strtokWithQuotes(char *s, const char *delim, int *quotes)
 {
     static char *olds = NULL;
     char *token, *r;
@@ -257,9 +258,11 @@ static char *strtokWithQuotes(char *s, const char *delim)
     if (*s == '\0')
 	return NULL;
 
-    /* Leading quote escapes all of delim until next quote */
-    if (*s == '"') {
-	delim = "\"";
+    /* Leading quote escapes all of delim until next quote of same type */
+    *quotes = 0;
+    if (*s == '"' || *s == '\'') {
+	*quotes = 1;
+	delim = (*s == '"') ? "\"" : "'";
 	s++;
     }
 
@@ -275,6 +278,8 @@ static char *strtokWithQuotes(char *s, const char *delim)
 	/* This token finishes the string */
 	olds = s;
     } else {
+	if (*s == '"' || *s == '\'')
+	    *quotes = 2;
 	/* Terminate the token and make olds point past it */
 	*s = '\0';
 	olds = s+1;
@@ -883,14 +888,21 @@ static rpmRC parseForSimple(char * buf, FileEntry cur, ARGV_t * fileNames)
 {
     char *s, *t, *end;
     char *delim = " \t\n";
+    int quotes = 0;
     rpmRC res = RPMRC_OK;
     int allow_relative = (RPMFILE_PUBKEY|RPMFILE_DOC|RPMFILE_LICENSE);
 
     t = buf;
-    while ((s = strtokWithQuotes(t, delim)) != NULL) {
+    while ((s = strtokWithQuotes(t, delim, &quotes)) != NULL) {
 	t = NULL;
 	end = s + strlen(s) - 1;
 
+	/* Syntax checks */
+	if (quotes == 1) {
+	    rpmlog(RPMLOG_ERR, _("Missing quote: %s\n"), s);
+	    res = RPMRC_FAIL;
+	    continue;
+	}
 	if (*end == '\n') {
 	    /* Newline escaping is not supported */
 	    rpmlog(RPMLOG_ERR, _("Trailing backslash: %s\n"), s);
@@ -913,8 +925,18 @@ static rpmRC parseForSimple(char * buf, FileEntry cur, ARGV_t * fileNames)
 	    if (cur->attrFlags & (RPMFILE_DOC | RPMFILE_LICENSE))
 		cur->attrFlags |= RPMFILE_SPECIALDIR;
 	}
-	rpmUnescape(s, delim);
+
+	if (quotes == 0)
+	    rpmUnescape(s, delim);
+	else if (quotes == 2) {
+	    rpmUnescape(s, "\"'");
+	    s = rpmEscape(s, "?*[]{}");
+	}
+
 	argvAdd(fileNames, s);
+
+	if (quotes == 2)
+	    free(s);
     }
 
     return res;

include/rpm/rpmfileutil.h

@@ -131,6 +131,8 @@ int rpmGlob(const char * patterns, int * argcPtr, ARGV_t * argvPtr);
  */
 char * rpmEscapeSpaces(const char * s);
 
+char * rpmEscape(const char *s, const char *accept);
+
 /** \ingroup rpmfileutil
  * Unescape each char listed in accept by removing a backslash preceding it.
  * @param s		string

rpmio/rpmfileutil.c

@@ -386,30 +386,40 @@ char * rpmGetPath(const char *path, ...)
     return rpmCleanPath(res);
 }
 
-char * rpmEscapeSpaces(const char * s)
+static char * rpmEscapeChars(const char *s, const char *accept, int (*fn)(int))
 {
     const char * se;
     char * t;
     char * te;
     size_t nb = 0;
 
     for (se = s; *se; se++) {
-	if (isspace(*se))
+	if ((accept && strchr(accept, *se)) || (fn && fn(*se)))
 	    nb++;
 	nb++;
     }
     nb++;
 
     t = te = xmalloc(nb);
     for (se = s; *se; se++) {
-	if (isspace(*se))
+	if ((accept && strchr(accept, *se)) || (fn && fn(*se)))
 	    *te++ = '\\';
 	*te++ = *se;
     }
     *te = '\0';
     return t;
 }
 
+char * rpmEscapeSpaces(const char *s)
+{
+    return rpmEscapeChars(s, NULL, isspace);
+}
+
+char * rpmEscape(const char *s, const char *accept)
+{
+    return rpmEscapeChars(s, accept, NULL);
+}
+
 void rpmUnescape(char *s, const char *accept)
 {
     char *p, *q;

tests/data/SPECS/globesctest.spec

@@ -24,7 +24,6 @@ mkdir -p %{buildroot}/opt
 
 # Glob escaping
 touch '%{buildroot}/opt/foo[bar]'
-touch '%{buildroot}/opt/foo[bar baz]'
 touch '%{buildroot}/opt/foo\[bar\]'
 touch '%{buildroot}/opt/foo*'
 touch '%{buildroot}/opt/foo\bar'
@@ -45,6 +44,17 @@ touch '%{buildroot}/opt/foo r'
 touch '%{buildroot}/opt/foo%%name'
 touch '%{buildroot}/opt/foo%mymacro'
 
+# Quoting
+touch '%{buildroot}/opt/foo bar.conf'
+touch '%{buildroot}/opt/foo [baz]'
+touch '%{buildroot}/opt/foo[bar baz]'
+touch '%{buildroot}/opt/foo\[baz\]'
+touch "%{buildroot}/opt/foo'bax'"
+touch '%{buildroot}/opt/foo"bay"'
+touch '%{buildroot}/opt/foo\baz'
+touch '%{buildroot}/opt/foo\bay'
+touch '%{buildroot}/opt/foo"baz"'
+
 # Regression checks
 touch '%{buildroot}/opt/foo-bar1'
 touch '%{buildroot}/opt/foo-bar2'
@@ -64,10 +74,10 @@ touch '%{buildroot}/opt/foobawb'
 %files
 
 %doc foo\[bar\] ba* "foo bar" foo\ [bar] doc%%name
+%config "/opt/foo bar.conf"
 
 # Glob escaping
 /opt/foo\[bar\]
-"/opt/foo\[bar baz\]"
 /opt/foo\\\[bar\\\]
 /opt/foo\*
 /opt/foo\\bar
@@ -86,6 +96,16 @@ touch '%{buildroot}/opt/foobawb'
 /opt/foo%%name
 /opt/foo%mymacro
 
+# Quoting
+"/opt/foo [baz]"
+"/opt/foo[bar baz]"
+"/opt/foo\\[baz\\]"
+"/opt/foo'bax'"
+'/opt/foo"bay"'
+'/opt/foo\\baz'
+"/opt/foo\\bay"
+"/opt/foo\"baz\""
+
 # Regression checks
 /opt/foo-bar*
 /opt/foo?bar?baz

tests/rpmbuild.at

@@ -442,15 +442,20 @@ runroot rpmbuild -bb --quiet /data/SPECS/globesctest.spec
 runroot rpm -qpl /build/RPMS/noarch/globesctest-1.0-1.noarch.rpm
 ],
 [0],
-[/opt/foo a
+[/opt/foo [[baz]]
+/opt/foo a
 /opt/foo b
 /opt/foo bar
+/opt/foo bar.conf
 /opt/foo r
 /opt/foo" bar"
 /opt/foo"'baz"'
 /opt/foo"bar"
+/opt/foo"bay"
+/opt/foo"baz"
 /opt/foo%globesctest
 /opt/foo%name
+/opt/foo'bax'
 /opt/foo*
 /opt/foo-bar1
 /opt/foo-bar2
@@ -460,7 +465,10 @@ runroot rpm -qpl /build/RPMS/noarch/globesctest-1.0-1.noarch.rpm
 /opt/foo[[bax bay]]
 /opt/foo\
 /opt/foo\[[bar\]]
+/opt/foo\[[baz\]]
 /opt/foo\bar
+/opt/foo\bay
+/opt/foo\baz
 /opt/foobar
 /opt/foobara
 /opt/foobarb