threads / patch / 22823

v2, 3 partsgit-svn: Support retrieving passwords with GIT_ASKPASS

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

## tl;dr

5 messages between Feb 26, 2010 and Feb 26, 2010. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Frank Li· Feb 26, 2010, 00:07 UTC · lore

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(-)

Show changes to git-svn.perl +27 −10

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

Junio C Hamano· Feb 26, 2010, 06:33 UTC · re: Frank Li · lore

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

Frank Li <lznuaa@gmail.com> writes:

Show 9 quoted lines

> +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.

Frank Li· Feb 26, 2010, 08:55 UTC · re: Junio C Hamano · lore

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

2010/2/26 Junio C Hamano <gitster@pobox.com>:

Show 14 quoted lines

> 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.

Show 7 quoted lines

>
> 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.

Show 5 quoted lines

>
> 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.

Eric Wong· Feb 26, 2010, 10:05 UTC · re: Frank Li · lore

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

Frank Li <lznuaa@gmail.com> wrote:

Show 31 quoted lines

> 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

Johannes Sixt· Feb 26, 2010, 17:41 UTC · re: Frank Li · lore

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

Frank Li schrieb:

Show 6 quoted lines

> +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