Bug #79805 [Asn]: sapi_windows_vt100_support throws TypeError when not able to analyze the stream

From: Date: Tue, 07 Jul 2020 17:46:45 +0000
Subject: Bug #79805 [Asn]: sapi_windows_vt100_support throws TypeError when not able to analyze the stream
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-227874@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=79805&edit=1

 ID:                 79805
 Updated by:         cmb@php.net
 Reported by:        ondrej at mirtes dot cz
 Summary:            sapi_windows_vt100_support throws TypeError when not
                     able to analyze the stream
 Status:             Assigned
 Type:               Bug
 Package:            Streams related
 Operating System:   Windows
 PHP Version:        8.0.0alpha1
 Assigned To:        cmb
 Block user comment: N
 Private report:     N

 New Comment:

Ugh.  First, it should be noted that the function throws a
TypeError instead of a warning with PHP 7 if strict_types are
enabled.  That suggest that the function was intended to only be
called on valid "ttys", so I tend to change this ticket to doc
bug.  I'm not really sure about this, so further input would be
welcome.


Previous Comments:
------------------------------------------------------------------------
[2020-07-07 15:29:17] requinix@php.net

It appears to be deliberate in the sense that the problem was classified as a sort of "type
error" and was thus updated to throw actual TypeErrors.

Quickly scanning over the source, the logic for whether that exception is thrown is also used with
whether stream_isatty() returns true. Thus I would suggest a change like

  if (\DIRECTORY_SEPARATOR === '\\') {
      return (\function_exists('sapi_windows_vt100_support')
+         && stream_isatty($this->stream)
-         && @sapi_windows_vt100_support($this->stream))
+         && sapi_windows_vt100_support($this->stream))
          || false !== getenv('ANSICON')
          || 'ON' === getenv('ConEmuANSI')
          || 'xterm' === getenv('TERM');
  }

Before I found stream_isatty() I would have agreed that a warning is more appropriate, but now
I'm not so sure.

Either way, I think the docs for sapi_windows_vt100_support() should mention stream_isatty() too.

------------------------------------------------------------------------
[2020-07-07 15:26:50] nicolasgrekas@php.net

I may be missing something, but neither the warning nor the TypeError look legit to me.
The function should just return false instead IIUC.

------------------------------------------------------------------------
[2020-07-07 15:14:37] ondrej at mirtes dot cz

Description:
------------
Hi, Symfony Console contains this code:

        if (\DIRECTORY_SEPARATOR === '\\') {
            return (\function_exists('sapi_windows_vt100_support')
                && @sapi_windows_vt100_support($this->stream))
                || false !== getenv('ANSICON')
                || 'ON' === getenv('ConEmuANSI')
                || 'xterm' === getenv('TERM');
        }

(https://github.com/symfony/symfony/blob/104a8f8cc99d49f15a67c3014e644740bc4d41d9/src/Symfony/Component/Console/Output/StreamOutput.php#L105-L111)

The @ is there because on PHP 7.x, the function throws this warning:

E_WARNING: sapi_windows_vt100_support() was not able to analyze the specified stream

In PHP 8, this is now a TypeError:

TypeError: sapi_windows_vt100_support() was not able to analyze the specified stream

I'm opening this bug to make sure that this changed is done on purpose (or not) so that the
code in Symfony can be fixed.



------------------------------------------------------------------------



--
Edit this bug report at https://bugs.php.net/bug.php?id=79805&edit=1


Thread (8 messages)

« previous php.bugs (#227874) next »