Skip to content

Commit

Permalink
Preserve literal value of quoted %files entries
Browse files Browse the repository at this point in the history
Turn off the special meaning of glob characters 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 globs
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
  • Loading branch information
dmnks committed Apr 22, 2022
1 parent a23411d commit 3044d55
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 13 deletions.
36 changes: 29 additions & 7 deletions build/files.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -258,8 +259,10 @@ static char *strtokWithQuotes(char *s, const char *delim)
return NULL;

/* Leading quote escapes all of delim until the next one */
if (*s == '"') {
delim = "\"";
*quotes = 0;
if (*s == '"' || *s == '\'') {
*quotes = 1;
delim = (*s == '"') ? "\"" : "'";
s++;
}

Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions include/rpm/rpmfileutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 13 additions & 3 deletions rpmio/rpmfileutil.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
26 changes: 24 additions & 2 deletions tests/data/SPECS/globesctest.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -44,6 +43,18 @@ 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"'
touch '%{buildroot}/opt/foo"bar"'

# Regression checks
touch '%{buildroot}/opt/foo-bar1'
touch '%{buildroot}/opt/foo-bar2'
Expand All @@ -63,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
Expand All @@ -84,6 +95,17 @@ 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\""
/opt/foo\"bar\"

# Regression checks
/opt/foo-bar*
/opt/foo?bar?baz
Expand Down
11 changes: 10 additions & 1 deletion tests/rpmbuild.at
Original file line number Diff line number Diff line change
Expand Up @@ -442,13 +442,19 @@ 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"bar"
/opt/foo"bay"
/opt/foo"baz"
/opt/foo%globesctest
/opt/foo%name
/opt/foo'bax'
/opt/foo'baz
/opt/foo*
/opt/foo-bar1
Expand All @@ -459,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
Expand Down

0 comments on commit 3044d55

Please sign in to comment.