git calls SSH_ASKPASS even if DISPLAY is not set

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

git calls SSH_ASKPASS even if DISPLAY is not set

Xin Wang
Hi all,

I'm using git 1.7.3.2 in Fedora 14. In Fedora, SSH_ASKPASS will set to
be /usr/libexec/openssh/gnome-ssh-askpass in
/etc/profile.d/gnome-ssh-askpass.sh, so this environment is set by
login shell, and it will still be set even when X11 is not inuse.

According to ssh's manpage: "If ssh does not have a terminal
associated with it but DISPLAY and SSH_ASKPASS are set, it will
execute the program specified by SSH_ASKPASS and open an X11 window to
read the passphrase." But git will call SSH_ASKPASS even if there is a
terminal associated with it and DISPLAY is not set, then following
warning is displayed and git failed to go through.

$ git fetch

(gnome-ssh-askpass:1487): Gtk-WARNING **: cannot open display:

I think it‘s better if git could implement behavior conforming to ssh.


Thanks,
Xin Wang
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[RFC/PATCH] RE: git calls SSH_ASKPASS even if DISPLAY is not set

Alexander Sulfrian
Hi,
I prepared a patch to fix this behaviour. It is a simple patch that
adds another check for the DISPLAY environment variable.

But I do not know, if this behaviour breaks something...


Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH] git_getpass: fix ssh-askpass behaviour

Alexander Sulfrian
In reply to this post by Xin Wang
call ssh-askpass only if the display environment variable is also set
---
 connect.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/connect.c b/connect.c
index 57dc20c..2810e3b 100644
--- a/connect.c
+++ b/connect.c
@@ -621,7 +621,7 @@ int finish_connect(struct child_process *conn)
 
 char *git_getpass(const char *prompt)
 {
- const char *askpass;
+ const char *askpass, *display;
  struct child_process pass;
  const char *args[3];
  static struct strbuf buffer = STRBUF_INIT;
@@ -631,7 +631,10 @@ char *git_getpass(const char *prompt)
  askpass = askpass_program;
  if (!askpass)
  askpass = getenv("SSH_ASKPASS");
- if (!askpass || !(*askpass)) {
+
+ /* only call askpass if display is set */
+ display = getenv("DISPLAY");
+ if (!display || !(*display) || !askpass || !(*askpass))
  char *result = getpass(prompt);
  if (!result)
  die_errno("Could not read password");
--
1.7.2.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] git_getpass: fix ssh-askpass behaviour

Alexander Sulfrian
On Sun, 12 Dec 2010 13:32:54 +0100
Alexander Sulfrian <[hidden email]> wrote:

> call ssh-askpass only if the display environment variable is also set

Oh forgot to sign-off... I'll wait if there are other comments and
resend it in a few days.

Alex

signature.asc (205 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] git_getpass: fix ssh-askpass behaviour

Junio C Hamano
In reply to this post by Alexander Sulfrian
Alexander Sulfrian <[hidden email]> writes:

> call ssh-askpass only if the display environment variable is also set
> ---

I do not use it at all so I don't know for sure, but doesn't this break
OSX?

  20f3490 (web--browse: fix Mac OS X GUI detection for 10.6, 2009-09-14)

is an example that you can be fully graphical without having DISPLAY set
in some environment.  MinGW folks may want to chime in as well.

>  connect.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index 57dc20c..2810e3b 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -621,7 +621,7 @@ int finish_connect(struct child_process *conn)
>  
>  char *git_getpass(const char *prompt)
>  {
> - const char *askpass;
> + const char *askpass, *display;
>   struct child_process pass;
>   const char *args[3];
>   static struct strbuf buffer = STRBUF_INIT;
> @@ -631,7 +631,10 @@ char *git_getpass(const char *prompt)
>   askpass = askpass_program;
>   if (!askpass)
>   askpass = getenv("SSH_ASKPASS");
> - if (!askpass || !(*askpass)) {
> +
> + /* only call askpass if display is set */
> + display = getenv("DISPLAY");
> + if (!display || !(*display) || !askpass || !(*askpass))
>   char *result = getpass(prompt);
>   if (!result)
>   die_errno("Could not read password");

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] git_getpass: fix ssh-askpass behaviour

Xin Wang
In reply to this post by Alexander Sulfrian
Junio C Hamano <gitster <at> pobox.com> writes:

>
> Alexander Sulfrian <alexander <at> sulfrian.net> writes:
>
> > call ssh-askpass only if the display environment variable is also set
> > ---
>
> I do not use it at all so I don't know for sure, but doesn't this break
> OSX?
>
>   20f3490 (web--browse: fix Mac OS X GUI detection for 10.6, 2009-09-14)
>
> is an example that you can be fully graphical without having DISPLAY set
> in some environment.  MinGW folks may want to chime in as well.
>

How about test whether git is associated with a terminal or not?

> >  connect.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/connect.c b/connect.c
> > index 57dc20c..2810e3b 100644
> > --- a/connect.c
> > +++ b/connect.c
> > @@ -621,7 +621,7 @@ int finish_connect(struct child_process *conn)
> >
> >  char *git_getpass(const char *prompt)
> >  {
> > - const char *askpass;
> > + const char *askpass, *display;
> >   struct child_process pass;
> >   const char *args[3];
> >   static struct strbuf buffer = STRBUF_INIT;
> > @@ -631,7 +631,10 @@ char *git_getpass(const char *prompt)
> >   askpass = askpass_program;
> >   if (!askpass)
> >   askpass = getenv("SSH_ASKPASS");
> > - if (!askpass || !(*askpass)) {
> > +
> > + /* only call askpass if display is set */
> > + display = getenv("DISPLAY");
> > + if (!display || !(*display) || !askpass || !(*askpass))
> >   char *result = getpass(prompt);
> >   if (!result)
> >   die_errno("Could not read password");
>
>

Xin Wang
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] git_getpass: fix ssh-askpass behaviour

Johannes Sixt-2
In reply to this post by Alexander Sulfrian
Am 12/12/2010 13:32, schrieb Alexander Sulfrian:
> call ssh-askpass only if the display environment variable is also set

Not good: On Windows, we want to call out to SSH_ASKPASS even if DISPLAY
is not set (it almost never is).

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: git calls SSH_ASKPASS even if DISPLAY is not set

LarryMartell
In reply to this post by Xin Wang
Xin Wang wrote
Hi all,

I'm using git 1.7.3.2 in Fedora 14. In Fedora, SSH_ASKPASS will set to
be /usr/libexec/openssh/gnome-ssh-askpass in
/etc/profile.d/gnome-ssh-askpass.sh, so this environment is set by
login shell, and it will still be set even when X11 is not inuse.

According to ssh's manpage: "If ssh does not have a terminal
associated with it but DISPLAY and SSH_ASKPASS are set, it will
execute the program specified by SSH_ASKPASS and open an X11 window to
read the passphrase." But git will call SSH_ASKPASS even if there is a
terminal associated with it and DISPLAY is not set, then following
warning is displayed and git failed to go through.

$ git fetch

(gnome-ssh-askpass:1487): Gtk-WARNING **: cannot open display:

I think it‘s better if git could implement behavior conforming to ssh.

We are getting this error on a new CentOS system we just set up. Was there ever a fix or workaround for this?

-larry