# [PATCH] send-email: improve recipients_cmd() 6 messages from 2013-04-27 to 2013-04-29. Participants: Felipe Contreras, Junio C Hamano. Thread: https://gitlist.dev/t/33659 ## Felipe Contreras, 2013-04-27 22:26 Subject: [PATCH] send-email: improve recipients_cmd() Message-ID: <1367101561-14671-1-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367101561-14671-1-git-send-email-felipe.contreras%40gmail.com ``` We don't need to quote the filename to pass to the command, we can use an array of all the arguments to pass to the command, which is safer, and more extensible. Commit a47eab0 (send-email: use the three-arg form of open in recipients_cmd) stated we couldn't pass $file directly, but in fact, we can, the multi-word string is passed as is, and we can pass an array too. Signed-off-by: Felipe Contreras --- git-send-email.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index bd13cc8..7880d12 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1452,11 +1452,11 @@ foreach my $t (@files) { # Execute a command (e.g. $to_cmd) to get a list of email addresses # and return a results array sub recipients_cmd { - my ($prefix, $what, $cmd, $file) = @_; + my ($prefix, $what, $cmd, @args) = @_; my $sanitized_sender = sanitize_address($sender); my @addresses = (); - open my $fh, "-|", "$cmd \Q$file\E" + open my $fh, "-|", $cmd, @args or die "($prefix) Could not execute '$cmd'"; while (my $address = <$fh>) { $address =~ s/^\s*//g; -- 1.8.2.1.1031.g2ee5873 ``` ## Junio C Hamano, 2013-04-28 19:18 Subject: Re: [PATCH] send-email: improve recipients_cmd() Message-ID: <7vbo8yqxas.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vbo8yqxas.fsf%40alter.siamese.dyndns.org In-Reply-To: <1367101561-14671-1-git-send-email-felipe.contreras@gmail.com> ``` Felipe Contreras writes: > We don't need to quote the filename to pass to the command, we can use > an array of all the arguments to pass to the command, which is safer, > and more extensible. > > Commit a47eab0 (send-email: use the three-arg form of open in > recipients_cmd) stated we couldn't pass $file directly, but in fact, we > can, the multi-word string is passed as is, and we can pass an array > too. I think the comment is not about passing $file directly, but is about passing $cmd that could be multi-word string directly. The caller expects it be split into command and its earlier part of command line parameters, so that you can say $cmd = "cccmd --frotz --nitfol" but the non-string form of open would not give you that, unless you rewrite it to open $fh, "-|", qw(sh -c), $cmd, @args or something, no? > Signed-off-by: Felipe Contreras > --- > git-send-email.perl | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index bd13cc8..7880d12 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -1452,11 +1452,11 @@ foreach my $t (@files) { > # Execute a command (e.g. $to_cmd) to get a list of email addresses > # and return a results array > sub recipients_cmd { > - my ($prefix, $what, $cmd, $file) = @_; > + my ($prefix, $what, $cmd, @args) = @_; > > my $sanitized_sender = sanitize_address($sender); > my @addresses = (); > - open my $fh, "-|", "$cmd \Q$file\E" > + open my $fh, "-|", $cmd, @args > or die "($prefix) Could not execute '$cmd'"; > while (my $address = <$fh>) { > $address =~ s/^\s*//g; ``` ## Felipe Contreras, 2013-04-29 05:14 Subject: Re: [PATCH] send-email: improve recipients_cmd() Message-ID: URL: https://gitlist.dev/e/CAMP44s0x8fK-6vbY20eKA9QjWqhiOCvx%2BKFOgKOZ8q58bwk8Tw%40mail.gmail.com In-Reply-To: <7vbo8yqxas.fsf@alter.siamese.dyndns.org> ``` On Sun, Apr 28, 2013 at 2:18 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> We don't need to quote the filename to pass to the command, we can use >> an array of all the arguments to pass to the command, which is safer, >> and more extensible. >> >> Commit a47eab0 (send-email: use the three-arg form of open in >> recipients_cmd) stated we couldn't pass $file directly, but in fact, we >> can, the multi-word string is passed as is, and we can pass an array >> too. > > I think the comment is not about passing $file directly, but is > about passing $cmd that could be multi-word string directly. The > caller expects it be split into command and its earlier part of > command line parameters, so that you can say > > $cmd = "cccmd --frotz --nitfol" I see. > but the non-string form of open would not give you that, unless you > rewrite it to > > open $fh, "-|", qw(sh -c), $cmd, @args That doesn't seem to work for me. It would have to be: open $fh, "-|", qw(sh -c), "$cmd \Q$args\E" So we end up in the same place. -- Felipe Contreras ``` ## Junio C Hamano, 2013-04-29 05:24 Subject: Re: [PATCH] send-email: improve recipients_cmd() Message-ID: <7vd2tdoqna.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd2tdoqna.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Felipe Contreras writes: >> open $fh, "-|", qw(sh -c), $cmd, @args > > That doesn't seem to work for me. My fault. It needs to form a command line like this: sh -c 'cccmd --frotz --nitfol "$@"' - a r g s [jc: goes and tries $ sh -c 'echo X Y "$@"' - a r g s X Y a r g s ] so if we want to get rid of \Q\E, it would be: open $fh, '-|', qw(sh -c), "$cmd " . '"$@"', '-', @args ``` ## Felipe Contreras, 2013-04-29 05:32 Subject: Re: [PATCH] send-email: improve recipients_cmd() Message-ID: URL: https://gitlist.dev/e/CAMP44s3QkiQhYr7pKHgKFoXH5m9_ZHLHmwC4c9WC2OGjv7LHdg%40mail.gmail.com In-Reply-To: <7vd2tdoqna.fsf@alter.siamese.dyndns.org> ``` On Mon, Apr 29, 2013 at 12:24 AM, Junio C Hamano wrote: > Felipe Contreras writes: > >>> open $fh, "-|", qw(sh -c), $cmd, @args >> >> That doesn't seem to work for me. > > My fault. It needs to form a command line like this: > > sh -c 'cccmd --frotz --nitfol "$@"' - a r g s > > [jc: goes and tries > $ sh -c 'echo X Y "$@"' - a r g s > X Y a r g s > ] > > so if we want to get rid of \Q\E, it would be: > > open $fh, '-|', qw(sh -c), "$cmd " . '"$@"', '-', @args > I don't know if that would be better, or converting @args to a list of quoted strings, essentially keeping the current behavior. -- Felipe Contreras ``` ## Junio C Hamano, 2013-04-29 08:15 Subject: Re: [PATCH] send-email: improve recipients_cmd() Message-ID: <7vhaipn45f.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhaipn45f.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Felipe Contreras writes: >> so if we want to get rid of \Q\E, it would be: >> >> open $fh, '-|', qw(sh -c), "$cmd " . '"$@"', '-', @args >> > > I don't know if that would be better, or converting @args to a list of > quoted strings, essentially keeping the current behavior. I tend to agree, so let's drop it for now. ```