php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #79805 sapi_windows_vt100_support throws TypeError when not able to analyze the stream
Submitted: 2020-07-07 15:14 UTC Modified: 2020-07-08 10:36 UTC
From: ondrej at mirtes dot cz Assigned: cmb (profile)
Status: Closed Package: Streams related
PHP Version: 8.0.0alpha1 OS: Windows
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: ondrej at mirtes dot cz
New email:
PHP Version: OS:

 

 [2020-07-07 15:14 UTC] 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.


Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-07-07 15:26 UTC] 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:29 UTC] 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:34 UTC] nikic@php.net
-Assigned To: +Assigned To: cmb
 [2020-07-07 17:46 UTC] cmb@php.net
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.
 [2020-07-08 08:29 UTC] nikic@php.net
I think I agree with @nicolasgrekas here that this should just return false (without warning) for non-consoles (in fact it already does for some of them -- but only those that are castable to FDs).
 [2020-07-08 10:36 UTC] cmb@php.net
Not even emitting a warning when used as getter is fine for me as
well, but I think consequently when used as setter, the function
shouldn't throw (but raise a warning only).
 [2020-07-15 17:23 UTC] cmb@php.net
The following pull request has been associated:

Patch Name: Fix #79805: sapi_windows_vt100_support throws TypeError
On GitHub:  https://github.com/php/php-src/pull/5863
Patch:      https://github.com/php/php-src/pull/5863.patch
 [2020-07-16 16:36 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=ae7554270f5dc4bb2bcf4e7ead0c519d98d74bd0
Log: Fix #79805: sapi_windows_vt100_support throws TypeError
 [2020-07-16 16:36 UTC] cmb@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Oct 31 22:01:27 2024 UTC