From fce13af5d20cad8dcb2d0e47bcf01b6960f08e55 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 11 Sep 2017 14:44:24 +0900 Subject: cvsserver: move safe_pipe_capture() to the main package As a preparation for replacing `command` with a call to this function from outside GITCVS::updater package, move it to the main package. Signed-off-by: Junio C Hamano --- git-cvsserver.perl | 47 ++++++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 25 deletions(-) (limited to 'git-cvsserver.perl') diff --git a/git-cvsserver.perl b/git-cvsserver.perl index d50c85ed7b..8229d9d198 100755 --- a/git-cvsserver.perl +++ b/git-cvsserver.perl @@ -3406,6 +3406,22 @@ sub refHashEqual return $out; } +# an alternative to `command` that allows input to be passed as an array +# to work around shell problems with weird characters in arguments + +sub safe_pipe_capture { + + my @output; + + if (my $pid = open my $child, '-|') { + @output = (<$child>); + close $child or die join(' ',@_).": $! $?"; + } else { + exec(@_) or die "$! $?"; # exec() can fail the executable can't be found + } + return wantarray ? @output : join('',@output); +} + package GITCVS::log; @@ -3882,7 +3898,7 @@ sub update # several candidate merge bases. let's assume # that the first one is the best one. my $base = eval { - safe_pipe_capture('git', 'merge-base', + ::safe_pipe_capture('git', 'merge-base', $lastpicked, $parent); }; # The two branches may not be related at all, @@ -4749,7 +4765,7 @@ sub getMetaFromCommithash return $retVal; } - my($fileHash)=safe_pipe_capture("git","rev-parse","$revCommit:$filename"); + my($fileHash) = ::safe_pipe_capture("git","rev-parse","$revCommit:$filename"); chomp $fileHash; if(!($fileHash=~/^[0-9a-f]{40}$/)) { @@ -4844,8 +4860,8 @@ sub lookupCommitRef return $commitHash; } - $commitHash=safe_pipe_capture("git","rev-parse","--verify","--quiet", - $self->unescapeRefName($ref)); + $commitHash = ::safe_pipe_capture("git","rev-parse","--verify","--quiet", + $self->unescapeRefName($ref)); $commitHash=~s/\s*$//; if(!($commitHash=~/^[0-9a-f]{40}$/)) { @@ -4854,7 +4870,7 @@ sub lookupCommitRef if( defined($commitHash) ) { - my $type=safe_pipe_capture("git","cat-file","-t",$commitHash); + my $type = ::safe_pipe_capture("git","cat-file","-t",$commitHash); if( ! ($type=~/^commit\s*$/ ) ) { $commitHash=undef; @@ -4907,7 +4923,7 @@ sub commitmessage return $message; } - my @lines = safe_pipe_capture("git", "cat-file", "commit", $commithash); + my @lines = ::safe_pipe_capture("git", "cat-file", "commit", $commithash); shift @lines while ( $lines[0] =~ /\S/ ); $message = join("",@lines); $message .= " " if ( $message =~ /\n$/ ); @@ -5056,25 +5072,6 @@ sub in_array return $retval; } -=head2 safe_pipe_capture - -an alternative to `command` that allows input to be passed as an array -to work around shell problems with weird characters in arguments - -=cut -sub safe_pipe_capture { - - my @output; - - if (my $pid = open my $child, '-|') { - @output = (<$child>); - close $child or die join(' ',@_).": $! $?"; - } else { - exec(@_) or die "$! $?"; # exec() can fail the executable can't be found - } - return wantarray ? @output : join('',@output); -} - =head2 mangle_dirname create a string from a directory name that is suitable to use as -- cgit v1.2.3 From 27dd73871f814062737c327103ee43f1eb7f30d9 Mon Sep 17 00:00:00 2001 From: joernchen Date: Mon, 11 Sep 2017 14:45:09 +0900 Subject: cvsserver: use safe_pipe_capture instead of backticks This makes the script pass arguments that are derived from end-user input in safer way when invoking subcommands. Reported-by: joernchen Signed-off-by: joernchen Signed-off-by: Junio C Hamano --- git-cvsserver.perl | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'git-cvsserver.perl') diff --git a/git-cvsserver.perl b/git-cvsserver.perl index 8229d9d198..bd29b26cc2 100755 --- a/git-cvsserver.perl +++ b/git-cvsserver.perl @@ -841,7 +841,7 @@ sub req_Modified # Save the file data in $state $state->{entries}{$state->{directory}.$data}{modified_filename} = $filename; $state->{entries}{$state->{directory}.$data}{modified_mode} = $mode; - $state->{entries}{$state->{directory}.$data}{modified_hash} = `git hash-object $filename`; + $state->{entries}{$state->{directory}.$data}{modified_hash} = safe_pipe_capture('git','hash-object',$filename); $state->{entries}{$state->{directory}.$data}{modified_hash} =~ s/\s.*$//s; #$log->debug("req_Modified : file=$data mode=$mode size=$size"); @@ -1463,7 +1463,7 @@ sub req_update # transmit file, format is single integer on a line by itself (file # size) followed by the file contents # TODO : we should copy files in blocks - my $data = `cat $mergedFile`; + my $data = safe_pipe_capture('cat', $mergedFile); $log->debug("File size : " . length($data)); print length($data) . "\n"; print $data; @@ -1579,7 +1579,7 @@ sub req_ci $branchRef = "refs/heads/$stickyInfo->{tag}"; } - $parenthash = `git show-ref -s $branchRef`; + $parenthash = safe_pipe_capture('git', 'show-ref', '-s', $branchRef); chomp $parenthash; if ($parenthash !~ /^[0-9a-f]{40}$/) { @@ -1704,7 +1704,7 @@ sub req_ci } close $msg_fh; - my $commithash = `git commit-tree $treehash -p $parenthash < $msg_filename`; + my $commithash = safe_pipe_capture('git', 'commit-tree', $treehash, '-p', $parenthash, '-F', $msg_filename); chomp($commithash); $log->info("Commit hash : $commithash"); @@ -2854,12 +2854,12 @@ sub transmitfile die "Need filehash" unless ( defined ( $filehash ) and $filehash =~ /^[a-zA-Z0-9]{40}$/ ); - my $type = `git cat-file -t $filehash`; + my $type = safe_pipe_capture('git', 'cat-file', '-t', $filehash); chomp $type; die ( "Invalid type '$type' (expected 'blob')" ) unless ( defined ( $type ) and $type eq "blob" ); - my $size = `git cat-file -s $filehash`; + my $size = safe_pipe_capture('git', 'cat-file', '-s', $filehash); chomp $size; $log->debug("transmitfile($filehash) size=$size, type=$type"); @@ -3040,7 +3040,7 @@ sub ensureWorkTree chdir $work->{emptyDir} or die "Unable to chdir to $work->{emptyDir}\n"; - my $ver = `git show-ref -s refs/heads/$state->{module}`; + my $ver = safe_pipe_capture('git', 'show-ref', '-s', "refs/heads/$state->{module}"); chomp $ver; if ($ver !~ /^[0-9a-f]{40}$/) { @@ -3287,7 +3287,7 @@ sub open_blob_or_die die "Need filehash\n"; } - my $type = `git cat-file -t $name`; + my $type = safe_pipe_capture('git', 'cat-file', '-t', $name); chomp $type; unless ( defined ( $type ) and $type eq "blob" ) @@ -3296,7 +3296,7 @@ sub open_blob_or_die die ( "Invalid type '$type' (expected 'blob')" ) } - my $size = `git cat-file -s $name`; + my $size = safe_pipe_capture('git', 'cat-file', '-s', $name); chomp $size; $log->debug("open_blob_or_die($name) size=$size, type=$type"); @@ -3813,10 +3813,10 @@ sub update # first lets get the commit list $ENV{GIT_DIR} = $self->{git_path}; - my $commitsha1 = `git rev-parse $self->{module}`; + my $commitsha1 = ::safe_pipe_capture('git', 'rev-parse', $self->{module}); chomp $commitsha1; - my $commitinfo = `git cat-file commit $self->{module} 2>&1`; + my $commitinfo = ::safe_pipe_capture('git', 'cat-file', 'commit', $self->{module}); unless ( $commitinfo =~ /tree\s+[a-zA-Z0-9]{40}/ ) { die("Invalid module '$self->{module}'"); -- cgit v1.2.3 From 46203ac24dc7e6b5a8d4f1b024ed93591705d47b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 11 Sep 2017 14:45:54 +0900 Subject: cvsserver: use safe_pipe_capture for `constant commands` as well This is not strictly necessary, but it is a good code hygiene. Signed-off-by: Junio C Hamano --- git-cvsserver.perl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'git-cvsserver.perl') diff --git a/git-cvsserver.perl b/git-cvsserver.perl index bd29b26cc2..ae1044273d 100755 --- a/git-cvsserver.perl +++ b/git-cvsserver.perl @@ -356,7 +356,7 @@ sub req_Root return 0; } - my @gitvars = `git config -l`; + my @gitvars = safe_pipe_capture(qw(git config -l)); if ($?) { print "E problems executing git-config on the server -- this is not a git repository or the PATH is not set correctly.\n"; print "E \n"; @@ -943,7 +943,7 @@ sub req_co # Provide list of modules, if -c was used. if (exists $state->{opt}{c}) { - my $showref = `git show-ref --heads`; + my $showref = safe_pipe_capture(qw(git show-ref --heads)); for my $line (split '\n', $showref) { if ( $line =~ m% refs/heads/(.*)$% ) { print "M $1\t$1\n"; @@ -1181,7 +1181,7 @@ sub req_update # projects (heads in this case) to checkout. # if ($state->{module} eq '') { - my $showref = `git show-ref --heads`; + my $showref = safe_pipe_capture(qw(git show-ref --heads)); print "E cvs update: Updating .\n"; for my $line (split '\n', $showref) { if ( $line =~ m% refs/heads/(.*)$% ) { @@ -1687,7 +1687,7 @@ sub req_ci return; } - my $treehash = `git write-tree`; + my $treehash = safe_pipe_capture(qw(git write-tree)); chomp $treehash; $log->debug("Treehash : $treehash, Parenthash : $parenthash"); -- cgit v1.2.3