[PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS

[PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS

[Frank Li]

git-svn reads passwords from an interactive terminal.
This behavior cause GUIs to hang waiting for git-svn to
complete

Fix this problem by allowing a password-retrieving command
to be specified in GIT_ASKPASS. SSH_ASKPASS is supported
as a fallback when GIT_ASKPASS is not provided.

Signed-off-by: Frank Li <lznuaa@gmail.com>
---
 git-svn.perl |   37 +++++++++++++++++++++++++++----------
 1 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 265852f..cd39792 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -31,6 +31,16 @@ if (! exists $ENV{SVN_SSH}) {
 	}
 }
 
+if (! exists $ENV{GIT_ASKPASS}) {
+	if (exists $ENV{SSH_ASKPASS}) {
+		$ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS};
+		if ($^O eq 'msys') {
+                        $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g;
+                        $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/;
+                }
+	}
+}
+
 $Git::SVN::Log::TZ = $ENV{TZ};
 $ENV{TZ} = 'UTC';
 $| = 1; # unbuffer STDOUT
@@ -3966,18 +3976,25 @@ sub username {
 
 sub _read_password {
 	my ($prompt, $realm) = @_;
-	print STDERR $prompt;
-	STDERR->flush;
-	require Term::ReadKey;
-	Term::ReadKey::ReadMode('noecho');
 	my $password = '';
-	while (defined(my $key = Term::ReadKey::ReadKey(0))) {
-		last if $key =~ /[\012\015]/; # \n\r
-		$password .= $key;
+	if (exists $ENV{GIT_ASKPASS}) {
+		open(PH, "$ENV{GIT_ASKPASS} \"$prompt\" |");
+		$password = <PH>;
+		$password =~ s/[\012\015]//; # \n\r
+		close(PH);
+	} else {
+		print STDERR $prompt;
+		STDERR->flush;
+		require Term::ReadKey;
+		Term::ReadKey::ReadMode('noecho');
+		while (defined(my $key = Term::ReadKey::ReadKey(0))) {
+			last if $key =~ /[\012\015]/; # \n\r
+			$password .= $key;
+		}
+		Term::ReadKey::ReadMode('restore');
+		print STDERR "\n";
+		STDERR->flush;
 	}
-	Term::ReadKey::ReadMode('restore');
-	print STDERR "\n";
-	STDERR->flush;
 	$password;
 }
 
-- 
1.7.0.85.g37fda.dirty

Re: [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS

[Junio C Hamano]

Frank Li <lznuaa@gmail.com> writes:

> +if (! exists $ENV{GIT_ASKPASS}) {
> +	if (exists $ENV{SSH_ASKPASS}) {
> +		$ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS};
> +		if ($^O eq 'msys') {
> +                        $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g;
> +                        $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/;
> +                }
> +	}
> +}

I've seen this code before, and you may not be the best person to answer
this question, but this worries me and puzzles me a bit.

On msys (and nowhere else), SSH_ASKPASS can be used as given by the user
to launch the prompter, but GIT_ASKPASS must be quoted in some funny way.

Why is that?  Does this mean they must be given differently by the end
user?  In other words, if the end user wants to set GIT_ASKPASS himself,
s/he needs to do this funny quoting, that is different from SSH_ASKPASS.

I also notice that git-gui has support for SSH_ASKPASS (and its own
implementation).  Does it have the same quoting issues on msys?

The reason I am asking is because:

 (1) if SSH_ASKPASS and GIT_ASKPASS cannot be specified exactly the same
     way, then [PATCH 3/3] would probably need a similar quoting magic?

 (2) With [PATCH 3/3], with quoting magic if necessary, we wouldn't need
     the above hunk, as it has already be done by the "git" potty.

Re: [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS

[Frank Li]

2010/2/26 Junio C Hamano <gitster@pobox.com>:
> Frank Li <lznuaa@gmail.com> writes:
>
>> +if (! exists $ENV{GIT_ASKPASS}) {
>> +     if (exists $ENV{SSH_ASKPASS}) {
>> +             $ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS};
>> +             if ($^O eq 'msys') {
>> +                        $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g;
>> +                        $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/;
>> +                }
>> +     }
>> +}
>
> I've seen this code before, and you may not be the best person to answer
> this question, but this worries me and puzzles me a bit.

Yes, I copy it from fall back SVN_SSH from GIT_SSH at git-svn.perl.

I guess it seems related with windows path using space, such as
c:\program files\bin\xxx.
perl Open ('$ENV{GIT_ASKPASS} |") will be changed to open("c:\program
files\bin\xxx |").
Perl will think c:\program as application, files\bin\xxx as first parameter.

So add ".  it equal to open ( "\"c:\program files\bin\xxx\" |"). perl
can run correct application.

>
> On msys (and nowhere else), SSH_ASKPASS can be used as given by the user
> to launch the prompter, but GIT_ASKPASS must be quoted in some funny way.
>
> Why is that?  Does this mean they must be given differently by the end
> user?  In other words, if the end user wants to set GIT_ASKPASS himself,
> s/he needs to do this funny quoting, that is different from SSH_ASKPASS.

I should add code to check if there are a space at GIT_ASKPASS,
if there are space in prompter path, add quote.
So end user set GIT_ASKPASS and SSH_ASKPASS at the same ways,  NO quoting.

>
> I also notice that git-gui has support for SSH_ASKPASS (and its own
> implementation).  Does it have the same quoting issues on msys?

I think no because msys add prompter to PATH environment and needn't
set full path.

>
> The reason I am asking is because:
>
>  (1) if SSH_ASKPASS and GIT_ASKPASS cannot be specified exactly the same
>     way, then [PATCH 3/3] would probably need a similar quoting magic?

SSH_ASKPASS and GIT_ASKPASS is the same.  C code needn't quoting
because start_command think $GIT_ASKPASS is full path and don't split
$GIT_ASKPASS to
application and parameter by space.

>
>  (2) With [PATCH 3/3], with quoting magic if necessary, we wouldn't need
>     the above hunk, as it has already be done by the "git" potty.
>

quoting magic is not necessary at PATCH 3/3.

Re: [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS

[Eric Wong]

Frank Li <lznuaa@gmail.com> wrote:
> git-svn reads passwords from an interactive terminal.
> This behavior cause GUIs to hang waiting for git-svn to
> complete
> 
> Fix this problem by allowing a password-retrieving command
> to be specified in GIT_ASKPASS. SSH_ASKPASS is supported
> as a fallback when GIT_ASKPASS is not provided.
> 
> Signed-off-by: Frank Li <lznuaa@gmail.com>
> ---
>  git-svn.perl |   37 +++++++++++++++++++++++++++----------
>  1 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 265852f..cd39792 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -31,6 +31,16 @@ if (! exists $ENV{SVN_SSH}) {
>  	}
>  }
>  
> +if (! exists $ENV{GIT_ASKPASS}) {
> +	if (exists $ENV{SSH_ASKPASS}) {
> +		$ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS};
> +		if ($^O eq 'msys') {
> +                        $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g;
> +                        $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/;
> +                }
> +	}
> +}
> +

Hi Frank,

Since this logic isn't SVN-specific, can we get this in Git.pm
and/or git-var so other tools can use it?

Thanks

-- 
Eric Wong

Re: [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS

[Johannes Sixt]

Frank Li schrieb:
> +if (! exists $ENV{GIT_ASKPASS}) {
> +	if (exists $ENV{SSH_ASKPASS}) {
> +		$ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS};
> +		if ($^O eq 'msys') {
> +                        $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g;
> +                        $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/;

Don't quote GIT_ASKPASS here.

> +	if (exists $ENV{GIT_ASKPASS}) {
> +		open(PH, "$ENV{GIT_ASKPASS} \"$prompt\" |");

		open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);

and you don't have to do any quoting at all, no?

-- Hannes