From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25434 invoked by alias); 7 Sep 2016 22:43:11 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 39231 Received: (qmail 7147 invoked from network); 7 Sep 2016 22:43:11 -0000 X-Qmail-Scanner-Diagnostics: from out4-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(66.111.4.28):SA:0(0.0/5.0):. Processed in 0.202259 secs); 07 Sep 2016 22:43:11 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at daniel.shahaf.name does not designate permitted sender hosts) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= daniel.shahaf.name; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=robNWma2QYHMDbaD fZLX3nnPCbE=; b=wYZUD71YBoVKisxL0og64zLnifndzBWltIoUrov39THf8wy4 HtdwcpLt9OXztuzYKsSBWoKmaiOBvmW0LevN5e+t2Z45zuiXiEZfxAY7wwncuwdm JG+vy8kQFSTFCar+yQEID0DlO37/NSVhesiTjA4jqCYaYWz8c0HBeFDDXho= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=smtpout; bh=robNWma2QYHMDba DfZLX3nnPCbE=; b=SzP5yr5tWUEFbfDYDHHgFnfzemqDPigeX7OVZhWc4umiUOB Uw55a6H+SAHd25PU0tKyof5ozV/W3zHUmQZFU+qjA0vR559sQR5Gmfrk2P5ZKBrL 9MU/XZUt65gKjrH3tkmy+BvXu6metJN5BWT/UHBSmTq6IX8lEw4VCRkNOFDE= X-Sasl-enc: dZISwwn8SY3ENZOZjICMGwQZQYmbxwXTrBBjP8grd8Fv 1473288183 Date: Wed, 7 Sep 2016 22:42:07 +0000 From: Daniel Shahaf To: Marko Myllynen Cc: zsh-workers@zsh.org Subject: Re: _libvirt, $opt_args, and noglob Message-ID: <20160907224207.GB7775@fujitsu.shahaf.local2> References: <20160904182622.GA32454@fujitsu.shahaf.local2> <20160906234646.GB17896@fujitsu.shahaf.local2> <23031e84-df6c-90c7-b5c0-9d6eb157dd61@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <23031e84-df6c-90c7-b5c0-9d6eb157dd61@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) See the penultimate paragraph for a question about whether ${(q)} should quote leading '=' signs to guard against the EQUALS option. Marko Myllynen wrote on Wed, Sep 07, 2016 at 12:53:13 +0300: > Hi, > > On 2016-09-07 02:46, Daniel Shahaf wrote: > > Daniel Shahaf wrote on Sun, Sep 04, 2016 at 18:26:22 +0000: > >> In the special case of _libvirt, simply adding noglob to the > >> _call_program arguments would probably fix this. > > > > Done, but the last line touches the sudo invocation from Oliver's > > gain-privileges patch. Oliver: shall I wait with committing this until > > you've committed gain-privileges? > > > > @@ -224,7 +224,7 @@ case $state in > > local srv ; (( ${(k)words[(I)--server]} > 0 )) && srv=${words[1+${(k)words[(I)--server]}]} > > [[ -z $srv ]] && return 1 > > [[ -n ${srv//[[:alnum:]]} ]] && return 1 > > - _wanted clients expl client compadd ${=${${(f):-"$(sudo virt-admin ${(Q)conn_opt} srv-clients-list --server $srv 2>/dev/null)"}/ [a-z]*}//[^0-9]} && return 0 > > + _wanted clients expl client compadd ${=${${(f):-"$(noglob sudo virt-admin ${(Q)conn_opt} srv-clients-list --server $srv 2>/dev/null)"}/ [a-z]*}//[^0-9]} && return 0 > > fi > > [[ -z $_cache_virt_admin_cmd_opts[$cmd] ]] && \ > > _cache_virt_admin_cmd_opts[$cmd]=${(M)${${${${=${(f)"$(_call_program virt-admin virt-admin help $cmd 2>&1)"}}/\[}/\]}/\;}:#-[-0-9A-Za-z]*} > > ]]] > > Do we need any of (Q)'s there any more after your addition of: > > uri=${uri//(#m)\\([\\:])/${MATCH[2]}} # opt_args elements are colon-escaped > > Based on a quick test looks like they could be dropped (or > perhaps even changed to (q))? _call_program expects its argument to be (q)'d. (The interface is like 'eval', not like execve(2).) The values of $opt_args are *already* (q)'d. If the command line is «virsh -c foo\\bar» then the value of $pot_args[-c] includes two backslashes, not one as virsh(1)'s argv[2] would see.¹ Therefore, I think the correct way to pass values derived from $opt_args — this includes $conn_opt — to _call_program is to use neither (q) nor (Q). That is the general answer. For the specific case of $conn_opt, the URI format validator ensures it has nothing that needs quoting, apart from '?' and '=', and the patch in the grandparent message removes the need for '?' to be quoted. However, there's no easy to to quote '=' when it's the first character of a word, and _call_program does honour the EQUALS option: . % _f() { compadd - $(_call_program x 'echo =true') } % f % f /bin/true Again, this is harmless as far as _libvirt is concerned, but it might not be so in other callsites. I think the right general solution to this would be to write ${(q)${(Q)conn_opts}} in the _call_program invocation, and teach the (q) flag to escape leading equals signs. Anyway, back to Marko's question: I assume even if the EQUALS option did affect the command executed, that wouldn't cause any harm, so I think the primary reason to get _libvirt's quoting right is in case somebody ever uses _libvirt as a basis for some other completion function. That's part of why I committed the (b) patch, too. Cheers, Daniel ¹ This assumes that the word being completed, $CURRENT, is not the argument to -c, in which cae $opt_args[-c] could have an opening quote but no matching closing quote.