Sync with 2.10.5

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Documentation/RelNotes/2.10.5.txt

@@ -0,0 +1,17 @@
+Git v2.10.5 Release Notes
+=========================
+
+Fixes since v2.10.4
+-------------------
+
+ * "git cvsserver" no longer is invoked by "git daemon" by default,
+   as it is old and largely unmaintained.
+
+ * Various Perl scripts did not use safe_pipe_capture() instead of
+   backticks, leaving them susceptible to end-user input.  They have
+   been corrected.
+
+Credits go to joernchen <joernchen@phenoelit.de> for finding the
+unsafe constructs in "git cvsserver", and to Jeff King at GitHub for
+finding and fixing instances of the same issue in other scripts.
+

Documentation/git-shell.txt

@@ -79,6 +79,22 @@ EOF
 $ chmod +x $HOME/git-shell-commands/no-interactive-login
 ----------------
 
+To enable git-cvsserver access (which should generally have the
+`no-interactive-login` example above as a prerequisite, as creating
+the git-shell-commands directory allows interactive logins):
+
+----------------
+$ cat >$HOME/git-shell-commands/cvs <<\EOF
+if ! test $# = 1 && test "$1" = "server"
+then
+	echo >&2 "git-cvsserver only handles \"server\""
+	exit 1
+fi
+exec git cvsserver server
+EOF
+$ chmod +x $HOME/git-shell-commands/cvs
+----------------
+
 SEE ALSO
 --------
 ssh(1),

git-archimport.perl

@@ -983,7 +983,7 @@ sub find_parents {
 	# check that we actually know about the branch
 	next unless -e "$git_dir/refs/heads/$branch";
 
-	my $mergebase = `git-merge-base $branch $ps->{branch}`;
+	my $mergebase = safe_pipe_capture(qw(git-merge-base), $branch, $ps->{branch});
 	if ($?) {
 	    # Don't die here, Arch supports one-way cherry-picking
 	    # between branches with no common base (or any relationship
@@ -1074,7 +1074,7 @@ sub find_parents {
 
 sub git_rev_parse {
     my $name = shift;
-    my $val  = `git-rev-parse $name`;
+    my $val  = safe_pipe_capture(qw(git-rev-parse), $name);
     die "Error: git-rev-parse $name" if $?;
     chomp $val;
     return $val;

git-cvsimport.perl

@@ -642,6 +642,7 @@ sub is_sha1 {
 
 sub get_headref ($) {
 	my $name = shift;
+	$name =~ s/'/'\\''/;
 	my $r = `git rev-parse --verify '$name' 2>/dev/null`;
 	return undef unless $? == 0;
 	chomp $r;

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";
@@ -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");
@@ -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/(.*)$% ) {
@@ -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}$/)
             {
@@ -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");
@@ -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");
@@ -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;
 
@@ -3797,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}'");
@@ -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",
+    $commitHash = ::safe_pipe_capture("git","rev-parse","--verify","--quiet",
-                                  $self->unescapeRefName($ref));
+				      $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

shell.c

@@ -25,19 +25,6 @@ static int do_generic_cmd(const char *me, char *arg)
 	return execv_git_cmd(my_argv);
 }
 
-static int do_cvs_cmd(const char *me, char *arg)
-{
-	const char *cvsserver_argv[3] = {
-		"cvsserver", "server", NULL
-	};
-
-	if (!arg || strcmp(arg, "server"))
-		die("git-cvsserver only handles server: %s", arg);
-
-	setup_path();
-	return execv_git_cmd(cvsserver_argv);
-}
-
 static int is_valid_cmd_name(const char *cmd)
 {
 	/* Test command contains no . or / characters */
@@ -134,7 +121,6 @@ static struct commands {
 	{ "git-receive-pack", do_generic_cmd },
 	{ "git-upload-pack", do_generic_cmd },
 	{ "git-upload-archive", do_generic_cmd },
-	{ "cvs", do_cvs_cmd },
 	{ NULL },
 };
 

t/t9400-git-cvsserver-server.sh

@@ -588,4 +588,52 @@ test_expect_success 'cvs annotate' '
     test_cmp ../expect ../actual
 '
 
+#------------
+# running via git-shell
+#------------
+
+cd "$WORKDIR"
+
+test_expect_success 'create remote-cvs helper' '
+	write_script remote-cvs <<-\EOF
+	exec git shell -c "cvs server"
+	EOF
+'
+
+test_expect_success 'cvs server does not run with vanilla git-shell' '
+	(
+		cd cvswork &&
+		CVS_SERVER=$WORKDIR/remote-cvs &&
+		export CVS_SERVER &&
+		test_must_fail cvs log merge
+	)
+'
+
+test_expect_success 'configure git shell to run cvs server' '
+	mkdir "$HOME"/git-shell-commands &&
+
+	write_script "$HOME"/git-shell-commands/cvs <<-\EOF &&
+	if ! test $# = 1 && test "$1" = "server"
+	then
+		echo >&2 "git-cvsserver only handles \"server\""
+		exit 1
+	fi
+	exec git cvsserver server
+	EOF
+
+	# Should not be used, but part of the recommended setup
+	write_script "$HOME"/git-shell-commands/no-interactive-login <<-\EOF
+	echo Interactive login forbidden
+	EOF
+'
+
+test_expect_success 'cvs server can run with recommended config' '
+	(
+		cd cvswork &&
+		CVS_SERVER=$WORKDIR/remote-cvs &&
+		export CVS_SERVER &&
+		cvs log merge
+	)
+'
+
 test_done