[PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

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

[PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
---
 winsup/cygwin/fhandler_tty.cc | 21 ++++++++++++++++++++-
 winsup/cygwin/tty.cc          |  1 +
 winsup/cygwin/tty.h           |  1 +
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index da6119dfb..163f93f35 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -1305,6 +1305,20 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
   if (bg <= bg_eof)
     return (ssize_t) bg;
 
+  if (get_ttyp ()->need_clear_screen_on_write)
+    {
+      const char *term = getenv ("TERM");
+      if (term && strcmp (term, "dumb") && !strstr (term, "emacs") &&
+  wcsstr (myself->progname, L"\\usr\\sbin\\sshd.exe"))
+ {
+  /* FIXME: Clearing sequence may not be "^[[H^[[J"
+     depending on the terminal type. */
+  DWORD n;
+  WriteFile (get_output_handle_cyg (), "\033[H\033[J", 6, &n, NULL);
+ }
+      get_ttyp ()->need_clear_screen_on_write = false;
+    }
+
   termios_printf ("pty%d, write(%p, %lu)", get_minor (), ptr, len);
 
   push_process_state process_state (PID_TTYOU);
@@ -2668,7 +2682,12 @@ fhandler_pty_slave::fixup_after_attach (bool native_maybe, int fd_set)
   if (get_ttyp ()->num_pcon_attached_slaves == 0
       && !ALWAYS_USE_PCON)
     /* Assume this is the first process using this pty slave. */
-    get_ttyp ()->need_clear_screen = true;
+    {
+      if (wcsstr (myself->progname, L"\\usr\\sbin\\sshd.exe"))
+ get_ttyp ()->need_clear_screen_on_write = true;
+      else
+ get_ttyp ()->need_clear_screen = true;
+    }
 
   get_ttyp ()->num_pcon_attached_slaves ++;
  }
diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc
index 460153cdb..1595d0278 100644
--- a/winsup/cygwin/tty.cc
+++ b/winsup/cygwin/tty.cc
@@ -245,6 +245,7 @@ tty::init ()
   num_pcon_attached_slaves = 0;
   term_code_page = 0;
   need_clear_screen = false;
+  need_clear_screen_on_write = false;
 }
 
 HANDLE
diff --git a/winsup/cygwin/tty.h b/winsup/cygwin/tty.h
index 927d7afd9..c7aeef85b 100644
--- a/winsup/cygwin/tty.h
+++ b/winsup/cygwin/tty.h
@@ -106,6 +106,7 @@ private:
   int num_pcon_attached_slaves;
   UINT term_code_page;
   bool need_clear_screen;
+  bool need_clear_screen_on_write;
 
 public:
   HANDLE from_master () const { return _from_master; }
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Corinna Vinschen-2
Hi Takashi,

On Oct 18 20:37, Takashi Yano wrote:

> ---
>  winsup/cygwin/fhandler_tty.cc | 21 ++++++++++++++++++++-
>  winsup/cygwin/tty.cc          |  1 +
>  winsup/cygwin/tty.h           |  1 +
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index da6119dfb..163f93f35 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -1305,6 +1305,20 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
>    if (bg <= bg_eof)
>      return (ssize_t) bg;
>  
> +  if (get_ttyp ()->need_clear_screen_on_write)
> +    {
> +      const char *term = getenv ("TERM");
> +      if (term && strcmp (term, "dumb") && !strstr (term, "emacs") &&
> +  wcsstr (myself->progname, L"\\usr\\sbin\\sshd.exe"))
Sorry, but this doesn't look feasible.

You can't base the behaviour on the name of an application.  What about
other applications like telnetd, rshd, just to name the first ones
coming to mind?  What about a renamed sshd, or sshd installed into
another directory, or just an sshd in the build dir during testing?

Is this workaround really necessary at all?  Even basing this on the
terminal name looks pretty fragile.

Why exactly is the clear screen necessary?  You wrote something about
synchronizing the pseudo console and the pseudo tty content, IIRC, but
it still seems artificial to enforce a clear screen.  Is there no
other way to make the pseudo console happy?


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
Hi Corinna,

On Fri, 18 Oct 2019 16:33:06 +0200
Corinna Vinschen wrote:
> Sorry, but this doesn't look feasible.
>
> You can't base the behaviour on the name of an application.  What about
> other applications like telnetd, rshd, just to name the first ones
> coming to mind?  What about a renamed sshd, or sshd installed into
> another directory, or just an sshd in the build dir during testing?
>
> Is this workaround really necessary at all?  Even basing this on the
> terminal name looks pretty fragile.

I agree with you. However, I couldn't come up with better method.
Now I have come up with another implementation. Could you please
have a look at v2 patch?

As a caution, this patch is for:
https://www.cygwin.com/ml/cygwin/2019-10/msg00074.html
therefore, telnetd or rshd is not targeted.

> Why exactly is the clear screen necessary?  You wrote something about
> synchronizing the pseudo console and the pseudo tty content, IIRC, but
> it still seems artificial to enforce a clear screen.  Is there no
> other way to make the pseudo console happy?

Using cygwin 3.1.0-0.7 (TEST), by the following steps, you can see
what happens if clear screen is not done.

1) Execute ls or ps to draw something to screen.
2) env TERM=dumb script
3) Execute cmd.exe.

If we can accept this behaviour, clear screen is not necessary.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Corinna Vinschen-2
Hi Takashi,

On Oct 19 08:50, Takashi Yano wrote:

> Hi Corinna,
>
> On Fri, 18 Oct 2019 16:33:06 +0200
> Corinna Vinschen wrote:
> > Sorry, but this doesn't look feasible.
> >
> > You can't base the behaviour on the name of an application.  What about
> > other applications like telnetd, rshd, just to name the first ones
> > coming to mind?  What about a renamed sshd, or sshd installed into
> > another directory, or just an sshd in the build dir during testing?
> >
> > Is this workaround really necessary at all?  Even basing this on the
> > terminal name looks pretty fragile.
>
> I agree with you. However, I couldn't come up with better method.
> Now I have come up with another implementation. Could you please
> have a look at v2 patch?
>
> As a caution, this patch is for:
> https://www.cygwin.com/ml/cygwin/2019-10/msg00074.html
> therefore, telnetd or rshd is not targeted.
>
> > Why exactly is the clear screen necessary?  You wrote something about
> > synchronizing the pseudo console and the pseudo tty content, IIRC, but
> > it still seems artificial to enforce a clear screen.  Is there no
> > other way to make the pseudo console happy?
>
> Using cygwin 3.1.0-0.7 (TEST), by the following steps, you can see
> what happens if clear screen is not done.
>
> 1) Execute ls or ps to draw something to screen.
> 2) env TERM=dumb script
> 3) Execute cmd.exe.
>
> If we can accept this behaviour, clear screen is not necessary.
I just tried it and what I see is that starting cmd.exe clears the
screen.  While starting e.g. reg.exe or sc.exe or w32tm.exe does not
clear the screen.  Starting cmd.exe after reg.exe clears the screen and
positions the output of reg.exe at the top.  Same if I call a Cygwin
tool between reg.exe and cmd.exe.

So it seems cmd.exe is the only (or one of few) native CLI tools
actually trying to manipulate the screen buffer.  And what it does is
not so much clearing the screen, but to align buffer line 1 with the top
of the screen, even if line 1 has been produced before cmd.exe started.

I didn't look deeper into this yet, but the question coming to mind is,
what does GetConsoleScreenBufferInfo return right after starting
`env TERM=dumb script`, how does it look like right after running
`reg.exe' and before `cmd.exe', and how does it look after cmd.exe
changed it?

The (admittedly vague) idea is, maybe cmd.exe can be cheated into
not changing the console buffer by changing it to what it expects
right after creating the pseudo console...

Also, maybe this effect has something to do with the screen buffer
size in relation to the window size?

Other than that, my very personal opinion here is, not clearing the
screen is more desired than the terminal type and application name (or
SID) hacks just to pamper cmd.exe.  Others may disagree, so I'm open to
discussion.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
On Mon, 21 Oct 2019 11:43:56 +0200
Corinna Vinschen wrote:
> So it seems cmd.exe is the only (or one of few) native CLI tools
> actually trying to manipulate the screen buffer.  And what it does is
> not so much clearing the screen, but to align buffer line 1 with the top
> of the screen, even if line 1 has been produced before cmd.exe started.

Powershell also redraws the screen.
netsh is even worse. The cursor position will be broken by the follwing
steps.

1) env TERM=dumb script
2) netsh
3) winhttp show proxy

> Other than that, my very personal opinion here is, not clearing the
> screen is more desired than the terminal type and application name (or
> SID) hacks just to pamper cmd.exe.  Others may disagree, so I'm open to
> discussion.

Even with Microsoft-provided OpenSSH server, the screen is cleared
upon login. Therefore, clearing screen is not so strange, I suppose.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
On Mon, 21 Oct 2019 19:55:15 +0900
Takashi Yano wrote:
> netsh is even worse. The cursor position will be broken by the follwing
> steps.
> 1) env TERM=dumb script
> 2) netsh
> 3) winhttp show proxy

wmic also has the cursor position problem.

1) env TERM=dumb script
2) /cygdrive/c/windows/system32/wbem/wmic
3) computersystem list brief

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Corinna Vinschen-2
In reply to this post by Takashi Yano
On Oct 21 19:55, Takashi Yano wrote:

> On Mon, 21 Oct 2019 11:43:56 +0200
> Corinna Vinschen wrote:
> > So it seems cmd.exe is the only (or one of few) native CLI tools
> > actually trying to manipulate the screen buffer.  And what it does is
> > not so much clearing the screen, but to align buffer line 1 with the top
> > of the screen, even if line 1 has been produced before cmd.exe started.
>
> Powershell also redraws the screen.
> netsh is even worse. The cursor position will be broken by the follwing
> steps.
>
> 1) env TERM=dumb script
> 2) netsh
> 3) winhttp show proxy
>
> > Other than that, my very personal opinion here is, not clearing the
> > screen is more desired than the terminal type and application name (or
> > SID) hacks just to pamper cmd.exe.  Others may disagree, so I'm open to
> > discussion.
>
> Even with Microsoft-provided OpenSSH server, the screen is cleared
> upon login. Therefore, clearing screen is not so strange, I suppose.
Yes, but I assume this is done in MSFT's OpenSSH server, not in the
kernel or system lib

To me the problem is if it breaks Cygwin scenarios.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
In reply to this post by Corinna Vinschen-2
On Mon, 21 Oct 2019 11:43:56 +0200
Corinna Vinschen wrote:
> So it seems cmd.exe is the only (or one of few) native CLI tools
> actually trying to manipulate the screen buffer.  And what it does is
> not so much clearing the screen, but to align buffer line 1 with the top
> of the screen, even if line 1 has been produced before cmd.exe started.

What is done is not clearing the screen, but redrawing the screen
based on the screen buffer. This is done not by cmd.exe but by pseudo
console, I believe. The trigger for redrawing is not clear to me.

You can see what is done by pseudo console by checking "typescript"
generated by script.

> I didn't look deeper into this yet, but the question coming to mind is,
> what does GetConsoleScreenBufferInfo return right after starting
> `env TERM=dumb script`, how does it look like right after running
> `reg.exe' and before `cmd.exe', and how does it look after cmd.exe
> changed it?

I confirmed the dwSize has right screen size and dwCursorPosition
is (0,0) just after creating pty even though the cursor position
in real screen is not at top left.

Clearing screen fixes this mismatch.

> The (admittedly vague) idea is, maybe cmd.exe can be cheated into
> not changing the console buffer by changing it to what it expects
> right after creating the pseudo console...

To do this, it is necessary to log past data written to pty and
push them into console screen buffer when pseudo console is started.

The console screen buffer is empty just after creating pseudo console,
therefore, clearing screen is the simplest way to match the real screen
with the console screen buffer.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Corinna Vinschen-2
On Oct 22 09:09, Takashi Yano wrote:

> On Mon, 21 Oct 2019 11:43:56 +0200
> Corinna Vinschen wrote:
> > So it seems cmd.exe is the only (or one of few) native CLI tools
> > actually trying to manipulate the screen buffer.  And what it does is
> > not so much clearing the screen, but to align buffer line 1 with the top
> > of the screen, even if line 1 has been produced before cmd.exe started.
>
> What is done is not clearing the screen, but redrawing the screen
> based on the screen buffer. This is done not by cmd.exe but by pseudo
> console, I believe. The trigger for redrawing is not clear to me.
>
> You can see what is done by pseudo console by checking "typescript"
> generated by script.
>
> > I didn't look deeper into this yet, but the question coming to mind is,
> > what does GetConsoleScreenBufferInfo return right after starting
> > `env TERM=dumb script`, how does it look like right after running
> > `reg.exe' and before `cmd.exe', and how does it look after cmd.exe
> > changed it?
>
> I confirmed the dwSize has right screen size and dwCursorPosition
> is (0,0) just after creating pty even though the cursor position
> in real screen is not at top left.
>
> Clearing screen fixes this mismatch.
And calling SetConsoleCursorPosition instead does not?


Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Michael Haubenwallner-2
In reply to this post by Takashi Yano
Hi Takashi,

On 10/18/19 1:37 PM, Takashi Yano wrote:

> ---
>  winsup/cygwin/fhandler_tty.cc | 21 ++++++++++++++++++++-
>  winsup/cygwin/tty.cc          |  1 +
>  winsup/cygwin/tty.h           |  1 +
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index da6119dfb..163f93f35 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -1305,6 +1305,20 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
>    if (bg <= bg_eof)
>      return (ssize_t) bg;
>  
> +  if (get_ttyp ()->need_clear_screen_on_write)
> +    {
> +      const char *term = getenv ("TERM");
> +      if (term && strcmp (term, "dumb") && !strstr (term, "emacs") &&
> +  wcsstr (myself->progname, L"\\usr\\sbin\\sshd.exe"))

Again, my real problem does not utilize ssh at all, but is some python script
using multiple pty.openpty() to spawn commands inside, to allow for herding
all the subprocesses started by the commands (Ctrl-C or similar).

The ssh -t is just the sample showing a similar effect.

Unfortunately, I'm not deep enough into that python script to quickly provide
a test case with pty.openpty() combined with all the tty settings used there.

I've started to extract the important bits, but that may take a while.  OTOH,
this is an open source project if you like to try yourself: prefix.gentoo.org

Thanks!
/haubi/
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
In reply to this post by Corinna Vinschen-2
On Tue, 22 Oct 2019 08:55:06 +0200
Corinna Vinschen wrote:
> On Oct 22 09:09, Takashi Yano wrote:
> > I confirmed the dwSize has right screen size and dwCursorPosition
> > is (0,0) just after creating pty even though the cursor position
> > in real screen is not at top left.
> >
> > Clearing screen fixes this mismatch.
>
> And calling SetConsoleCursorPosition instead does not?

For SetConsoleCursorPosition, it is necessary to know the cursor
position of course. I cannot come up with any other way than
using ANSI escape sequence "ESC[6n". Do you think this is
feasible?

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
In reply to this post by Takashi Yano
On Tue, 22 Oct 2019 09:09:30 +0900
Takashi Yano wrote:
> On Mon, 21 Oct 2019 11:43:56 +0200
> Corinna Vinschen wrote:
> > The (admittedly vague) idea is, maybe cmd.exe can be cheated into
> > not changing the console buffer by changing it to what it expects
> > right after creating the pseudo console...
>
> To do this, it is necessary to log past data written to pty and
> push them into console screen buffer when pseudo console is started.

This does not make sence because ssh session may opened from other
systems than cygwin, i.e. Linux. In this case there is no way to
know real screen contents.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Corinna Vinschen-2
In reply to this post by Takashi Yano
On Oct 22 16:23, Takashi Yano wrote:

> On Tue, 22 Oct 2019 08:55:06 +0200
> Corinna Vinschen wrote:
> > On Oct 22 09:09, Takashi Yano wrote:
> > > I confirmed the dwSize has right screen size and dwCursorPosition
> > > is (0,0) just after creating pty even though the cursor position
> > > in real screen is not at top left.
> > >
> > > Clearing screen fixes this mismatch.
> >
> > And calling SetConsoleCursorPosition instead does not?
>
> For SetConsoleCursorPosition, it is necessary to know the cursor
> position of course. I cannot come up with any other way than
> using ANSI escape sequence "ESC[6n". Do you think this is
> feasible?
Hmm, interesting point.  I think that should be ok for a start.
assuming it works.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
On Oct 22 09:20, Michael Haubenwallner wrote:

> Hi Takashi,
>
> On 10/18/19 1:37 PM, Takashi Yano wrote:
> > ---
> >  winsup/cygwin/fhandler_tty.cc | 21 ++++++++++++++++++++-
> >  winsup/cygwin/tty.cc          |  1 +
> >  winsup/cygwin/tty.h           |  1 +
> >  3 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> > index da6119dfb..163f93f35 100644
> > --- a/winsup/cygwin/fhandler_tty.cc
> > +++ b/winsup/cygwin/fhandler_tty.cc
> > @@ -1305,6 +1305,20 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
> >    if (bg <= bg_eof)
> >      return (ssize_t) bg;
> >  
> > +  if (get_ttyp ()->need_clear_screen_on_write)
> > +    {
> > +      const char *term = getenv ("TERM");
> > +      if (term && strcmp (term, "dumb") && !strstr (term, "emacs") &&
> > +  wcsstr (myself->progname, L"\\usr\\sbin\\sshd.exe"))
>
> Again, my real problem does not utilize ssh at all, but is some python script
> using multiple pty.openpty() to spawn commands inside, to allow for herding
> all the subprocesses started by the commands (Ctrl-C or similar).
>
> The ssh -t is just the sample showing a similar effect.
>
> Unfortunately, I'm not deep enough into that python script to quickly provide
> a test case with pty.openpty() combined with all the tty settings used there.
>
> I've started to extract the important bits, but that may take a while.  OTOH,
> this is an open source project if you like to try yourself: prefix.gentoo.org
>
> Thanks!
> /haubi/
In terms of clearing the screen at all, what's your opinion, Michael?


Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
In reply to this post by Corinna Vinschen-2
On Tue, 22 Oct 2019 10:02:42 +0200
Corinna Vinschen wrote:

> On Oct 22 16:23, Takashi Yano wrote:
> > On Tue, 22 Oct 2019 08:55:06 +0200
> > Corinna Vinschen wrote:
> > > On Oct 22 09:09, Takashi Yano wrote:
> > > > I confirmed the dwSize has right screen size and dwCursorPosition
> > > > is (0,0) just after creating pty even though the cursor position
> > > > in real screen is not at top left.
> > > >
> > > > Clearing screen fixes this mismatch.
> > >
> > > And calling SetConsoleCursorPosition instead does not?
> >
> > For SetConsoleCursorPosition, it is necessary to know the cursor
> > position of course. I cannot come up with any other way than
> > using ANSI escape sequence "ESC[6n". Do you think this is
> > feasible?
>
> Hmm, interesting point.  I think that should be ok for a start.
> assuming it works.
Unfortunately, this does not work as expected. Please try
attached patch. Cursor position is kept as expected, but the
screen contents before opening pty are lost when cmd.exe is
executed.

However, this fixes cursor position problem of netsh and WMIC.

--
Takashi Yano <[hidden email]>

cursor-position.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Michael Haubenwallner-2
In reply to this post by Corinna Vinschen-2
On 10/22/19 10:04 AM, Corinna Vinschen wrote:
> On Oct 22 09:20, Michael Haubenwallner wrote:
>> On 10/18/19 1:37 PM, Takashi Yano wrote:

>>> +      const char *term = getenv ("TERM");
>>> +      if (term && strcmp (term, "dumb") && !strstr (term, "emacs") &&
>>> +  wcsstr (myself->progname, L"\\usr\\sbin\\sshd.exe"))

>> Again, my real problem does not utilize ssh at all, but is some python script
>> using multiple pty.openpty() to spawn commands inside, to allow for herding
>> all the subprocesses started by the commands (Ctrl-C or similar).

> In terms of clearing the screen at all, what's your opinion, Michael?

While I do not fully understand TTY handling, clearing the screen because
just opening a PTY doesn't feel correct.

To start with, attached is some python script where I do not expect to see
the initial clear screen code, but the one from /usr/bin/clear only.

This is what I see with python3 on *Linux*:

$ TERM=dumb python3 ./ptytest1.py
select read: [3] except: []
read: b'/home/haubi\r\n'
select read: [3] except: []
quit: [Errno 5] Input/output error

$ TERM=xterm python3 ./ptytest1.py
select read: [3] except: []
read: b'/home/haubi\r\n'
select read: [3] except: []
read: b'\x1b[H\x1b[2J\x1b[3J'
select read: [3] except: []
quit: [Errno 5] Input/output error

$ TERM=screen python3 ./ptytest1.py
select read: [3] except: []
read: b'/home/haubi\r\n'
select read: [3] except: []
read: b'\x1b[H\x1b[J'
select read: [3] except: []
quit: [Errno 5] Input/output error

Note that the clear screen code does depend on the TERM value, and /usr/bin/clear
does even yell if TERM is empty, unknown or unset.

Also note that Linux select() does not yield the fd as exception when it was closed.

Interesting enough, cygwin-3.0.7 does dump core somewhere in between, so the real
python program probably does some additional setup I've not extracted yet.

/haubi/

ptytest1.py (632 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Corinna Vinschen-2
In reply to this post by Takashi Yano
On Oct 22 18:24, Takashi Yano wrote:

> On Tue, 22 Oct 2019 10:02:42 +0200
> Corinna Vinschen wrote:
> > On Oct 22 16:23, Takashi Yano wrote:
> > > On Tue, 22 Oct 2019 08:55:06 +0200
> > > Corinna Vinschen wrote:
> > > > On Oct 22 09:09, Takashi Yano wrote:
> > > > > I confirmed the dwSize has right screen size and dwCursorPosition
> > > > > is (0,0) just after creating pty even though the cursor position
> > > > > in real screen is not at top left.
> > > > >
> > > > > Clearing screen fixes this mismatch.
> > > >
> > > > And calling SetConsoleCursorPosition instead does not?
> > >
> > > For SetConsoleCursorPosition, it is necessary to know the cursor
> > > position of course. I cannot come up with any other way than
> > > using ANSI escape sequence "ESC[6n". Do you think this is
> > > feasible?
> >
> > Hmm, interesting point.  I think that should be ok for a start.
> > assuming it works.
>
> Unfortunately, this does not work as expected. Please try
> attached patch. Cursor position is kept as expected, but the
> screen contents before opening pty are lost when cmd.exe is
> executed.
>
> However, this fixes cursor position problem of netsh and WMIC.
Am I doing something wrong?  This code crashes mintty on my
installation.  At start, a string of "6n6n6n6n..." appears and then
mintty exits.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Takashi Yano
On Tue, 22 Oct 2019 15:40:48 +0200
Corinna Vinschen wrote:
> Am I doing something wrong?  This code crashes mintty on my
> installation.  At start, a string of "6n6n6n6n..." appears and then
> mintty exits.

I cannot reproduce that.... How about this one?

--
Takashi Yano <[hidden email]>

cursor-position-2.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Corinna Vinschen-2
Hi Takashi,

On Oct 23 12:27, Takashi Yano wrote:
> On Tue, 22 Oct 2019 15:40:48 +0200
> Corinna Vinschen wrote:
> > Am I doing something wrong?  This code crashes mintty on my
> > installation.  At start, a string of "6n6n6n6n..." appears and then
> > mintty exits.
>
> I cannot reproduce that.... How about this one?

In my limited testing it seems to work nicely.

> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index da6119dfb2cf..26f99669f4fc 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -1296,6 +1296,30 @@ detach:
>    restore_reattach_pcon ();
>  }
>  
> +/* If master process is running as service, attaching to
> +   pseudo console should be done in fork. If attaching
> +   is done in spawn for inetd or sshd, it fails because
> +   the helper process is running as privileged user while
> +   slave process is not. This function is used to determine
> +   if the process is running as a srvice or not. */
> +static bool
> +is_running_as_service (void)
This function should probably use check_token_membership(PSID).
I'm also not quite sure if checking for mandatory_system_integrity_sid
makes sense.  Are there examples where the service SID is missing
but the integrity is set to system integrity level?

>  ssize_t __stdcall
>  fhandler_pty_slave::write (const void *ptr, size_t len)
>  {
> @@ -1305,6 +1329,30 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
>    if (bg <= bg_eof)
>      return (ssize_t) bg;
>  
> +  if (get_ttyp ()->need_clear_screen_on_write)
> +    {
> +      if (is_running_as_service ())
This check is redundant.  The only way to set need_clear_screen_on_write
to true is if is_running_as_service() was already checked for below.

> + {
> +  struct termios ti, ti_new;
> +  tcgetattr (&ti);
> +  ti_new = ti;
> +  ti_new.c_lflag &= (~ICANON | ECHO);
> +  tcsetattr (TCSANOW, &ti_new);
> +  char buf[32];
> +  DWORD n;
> +  WriteFile (get_output_handle_cyg (), "\033[6n", 4, &n, NULL);
> +  ReadFile (get_handle_cyg (), buf, sizeof(buf)-1, &n, NULL);
> +  ResetEvent (input_available_event);
> +  tcsetattr (TCSANOW, &ti);
> +  buf[n] = '\0';
> +  int rows, cols;
> +  sscanf (buf, "\033[%d;%dR", &rows, &cols);
Wouldn't it be safer to initialize rows and cols to 0 and to check the
result of sscanf?

>  HANDLE
> diff --git a/winsup/cygwin/tty.h b/winsup/cygwin/tty.h
> index 927d7afd9384..c7aeef85b482 100644
> --- a/winsup/cygwin/tty.h
> +++ b/winsup/cygwin/tty.h
> @@ -106,6 +106,7 @@ private:
>    int num_pcon_attached_slaves;
>    UINT term_code_page;
>    bool need_clear_screen;
> +  bool need_clear_screen_on_write;
Maybe the name should be aligned to the fact that it doesn't clear the
screen anymore?


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: [PATCH] Cygwin: pty: Disable clear screen for ssh sessions with -t option.

Michael Haubenwallner-2
In reply to this post by Michael Haubenwallner-2
On 10/22/19 12:06 PM, Michael Haubenwallner wrote:

> On 10/22/19 10:04 AM, Corinna Vinschen wrote:
>> On Oct 22 09:20, Michael Haubenwallner wrote:
>>> On 10/18/19 1:37 PM, Takashi Yano wrote:
>
>>>> +      const char *term = getenv ("TERM");
>>>> +      if (term && strcmp (term, "dumb") && !strstr (term, "emacs") &&
>>>> +  wcsstr (myself->progname, L"\\usr\\sbin\\sshd.exe"))
>
>>> Again, my real problem does not utilize ssh at all, but is some python script
>>> using multiple pty.openpty() to spawn commands inside, to allow for herding
>>> all the subprocesses started by the commands (Ctrl-C or similar).
>
>> In terms of clearing the screen at all, what's your opinion, Michael?
>
> While I do not fully understand TTY handling, clearing the screen because
> just opening a PTY doesn't feel correct.
>
> To start with, attached is some python script where I do not expect to see
> the initial clear screen code, but the one from /usr/bin/clear only.
>

> Interesting enough, cygwin-3.0.7 does dump core somewhere in between, so the real
> python program probably does some additional setup I've not extracted yet.

Sorry, the issue with cygwin-3.0.7 probably was because I've built my python3
against cygwin-3.1. Using Cygwin python 2 or 3 it does work with cygwin-3.0.7.

Here's an improved version, with additional initialization found in the real
world program - doesn't make a significant difference.

/haubi/

ptytest2.py (2K) Download Attachment
12