Bug #79805 [Asn->Csd]: sapi_windows_vt100_support throws TypeError when not able to analyze the stream
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
+Status: Closed
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:
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
Previous Comments:
------------------------------------------------------------------------
[2020-07-15 17:23:38] 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-08 10:36:13] 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-08 08:29:17] 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-07 17:46:45] 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-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.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=79805
--
Edit this bug report at https://bugs.php.net/bug.php?id=79805&edit=1
Thread (8 messages)