[libvirt] [patch 2/5] Instantiate comments in ip(6)tables rules

[Stefan Berger]

Eric Blake <eblake at redhat.com> wrote on 09/24/2010 06:16:35 PM:


> On 09/24/2010 02:22 PM, Stefan Berger wrote:
> > I just tried the TCK test without and with double-escaping in libvirtd
> > and double-escaping does seem to be necessary otherwise `ls` and $(ls)
> > do get executed and their results end up in the comment. The spaces
> > are preserved, though, so I can revert the change to IFS.
> 
> Hmm.
> 
> > "res=`eval \"$cmd\"" CMD_SEPARATOR
> 
> > +        virBufferVSprintf(buf,
> > +                          " -m comment --comment \\\"%s\\\"",
> > +                          cmt);
> 
> Thinking about it more:
> 
[...] 
> 
> My suggestion is to assign cmd using '' rather than "" (fewer things to 
> quote), as well as moving the eval outside of the `` (so it becomes 
> obvious which \ are interpreted by eval rather than by ``:
> 
> cmd='iptables -m comment --comment '\''user  $comment'\''
> eval res=\`"$cmd"\`
> res=`iptables -m comment --comment 'user  $comment'`
> 
> And the nice part of that is the implementation:
> 
> virBufferVSprintf(buf, " -m comment --comment '%s'",
>    escapeSingleQuotes(user_comment));
> 
> virBufferVSprintf(cmd, "cmd='%s'\nres=\\`\"$cmd\"\\`",
>    escapeSingleQuotes(buf));
> 

Also I followed this. I had to write it like this here to reflect what you 
wrote further above:

virBufferVSprintf(buf, " -m comment --comment '\''%s'\''",
    shellEscapeString(user_comment));

and shellEscapeString() needs to escape ' as well as `, otherwise I can 
execute commands.

Thanks for the help.

> 
> On further thought, gnulib might be doing:
> 
> #define strchr rpl_strchr
> 
> on platforms where strchr is broken, so using #undef strchr is too 
> risky.  So I'd recommend sticking with (strchr)(a, b), which still works 

> if gnulib has to replace a broken strchr.

Ok. This also works. Wished they left a note in the man pages about 
this... 


   Stefan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100924/cd0927a1/attachment-0001.htm>