Hi, Not sure I agree with the commit below. (In particular as I'm looking at converting this code into using unquote_first_word.)
On Mon, Jun 1, 2015 at 9:10 AM, Daniel Mack <[email protected]> wrote: > commit 22874a348fb1540c1a2b7907748fc57c9756a7ed > Author: Daniel Mack <[email protected]> > Date: Mon Jun 1 17:49:04 2015 +0200 > > load-fragment: use UNESCAPE_RELAX flag to parse exec directives > > The cunescape() helper function used to handle unknown escaping sequences > gracefully by copying them over verbatim. > > Commit 527b7a42 ("util: rework cunescape(), improve error handling") added > a flag to make that behavior optional, and changed to default to error out > with -EINVAL otherwise. > > However, config_parse_exec(), which is used to parse the > Exec{Start,Stop}{Post,Pre,} directives of unit files, was not changed > along > with that commit, which means that directives with improperly escaped > command line strings are no longer parsed. > > Relevant bugreports include: > > https://bugs.freedesktop.org/show_bug.cgi?id=90794 > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=787256 > > Fix this by passing UNESCAPE_RELAX to config_parse_exec() in order to > restore the original behavior. > > diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c > index c95c110..df5fe6f 100644 > --- a/src/core/load-fragment.c > +++ b/src/core/load-fragment.c > @@ -610,7 +610,7 @@ int config_parse_exec( > else > skip = strneq(word, "\\;", MAX(l, 1U)); > > - r = cunescape_length(word + skip, l - skip, 0, &c); > + r = cunescape_length(word + skip, l - skip, > UNESCAPE_RELAX, &c); > if (r < 0) { > log_syntax(unit, LOG_ERR, filename, line, r, > "Failed to unescape command line, ignoring: %s", rvalue); > r = 0; So, my problem with it is that the bug's expectation is that backslashes inside single quotes will remain as backslashes, as the example is a regexp '\w+@\K[\d.]+'. But this is not true here!!! It's only fixing it for the particular cases that are not escape sequences yet. For instance, what if I'm doing a parameter that is a regexp that is looking for a word boundary and I want to use '\b'? systemd with the current patch will (still) turn this into a backspace character. Right now the systemd quoting rules do *not* match the shell quoting rules. (In fact, this is akin to a bug complaining that variables in systemd do not match shell variables. That's indeed the case, but it doesn't make it a bug. It's working as documented and as intended.) I'd be ok with changing the rules so that backslash inside single quotes remains a literal backslash, as I think we have the two kinds of quotes (single quotes and double quotes) and I don't think it would hurt to make them work a little bit closer to how the shell works... (Though we'll keep expanding variables inside single quotes?) In that case (of making backslashes stay literal inside single quotes) I think the best way forward is complete the conversion to unquote_first_word and then update unquote_first_word to introduce those rules (essentially, just get rid of the SINGLE_QUOTE_ESCAPE rule would do.) Cheers, Filipe _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
