Implement stream_vt100_support user function

[mlocati]

This is a followup of https://bugs.php.net/bug.php?id=72768

TL;DR Console apps of newer Windows 10 versions support colors, but we need to manually set the ENABLE_VIRTUAL_TERMINAL_PROCESSING console flag.

Here's my proposal of a new userland function: bool stream_vt100_support(resource stream[, bool enable])

Where:

• stream should be STDOUT orSTDERR
• if enable missing: the function returns true if the specified stream supports colors
• if enable is true: the function tries to enable color support for the stream (return true on success)
• if enable is false: the function tries to disable color support for the stream (return true on success)

To be done:

• currently the function accepts a string (eg php://stdout): it should instead accept a stream resource
• tests

[mlocati]

Any hint about how to retrieve the standard Windows handles (STD_OUTPUT_HANDLE/STD_ERROR_HANDLE) starting from a php_stream?

[mlocati]

• currently the function accepts a string (eg php://stdout): it should instead accept a stream resource

Done in bd93781 😉

[mlocati]

• tests

Done in d9b1c99 😉

[KalleZ]

Hi

Wouldn't it be easier if this was added as a context option instead?

[mlocati]

Wouldn't it be easier if this was added as a context option instead?

What exactly do you mean? An example?

[mlocati]

Sample code:

<?php
$sampleText = "\033[101;93m Yellow text on red background \033[0m\n";
echo "Enabled: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo $sampleText;
echo "Enable: ", stream_vt100_support(STDOUT, true) ? "ok" : "failed", "\n";
echo "Enabled: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo $sampleText;
echo "Disable: ", stream_vt100_support(STDOUT, false) ? "ok" : "failed", "\n";
echo "Enabled: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo $sampleText;

Output:

[mlocati]

Remark 1: since the ENABLE_VIRTUAL_TERMINAL_PROCESSING flag is associated to the whole console, if both STDOUT and STDERR are bound to the same console (ie: almost always if there's no redirection), the following code

<?php
echo "Enabled on STDOUT: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";

echo "Enable on STDERR : ", stream_vt100_support(STDERR, true) ? "ok" : "failed", "\n";
echo "Enabled on STDOUT: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";

echo "Disable on STDERR: ", stream_vt100_support(STDERR, false) ? "ok" : "failed", "\n";
echo "Enabled on STDOUT: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";

ouputs

Enabled on STDOUT: no
Enable on STDERR : ok
Enabled on STDOUT: yes
Disable on STDERR: ok
Enabled on STDOUT: no

Remark 2: this function checks if the specified stream is redirected.
So, let's assume we have a test.php file like this:

<?php
echo "Enable on STDOUT: ", stream_vt100_support(STDOUT, true) ? "ok" : "failed", "\n";
echo "Enable on STDERR: ", stream_vt100_support(STDERR, true) ? "ok" : "failed", "\n";
echo "\n";
echo "Enabled on STDOUT: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo "Enabled on STDERR: ", stream_vt100_support(STDERR) ? "yes" : "no", "\n";

• running php test.php 2>out.txt results in

Enable on STDOUT: ok
Enable on STDERR: failed

Enabled on STDOUT: yes
Enabled on STDERR: no

• running php test.php >out.txt, the contents of out.txt is

Enable on STDOUT: failed
Enable on STDERR: ok

Enabled on STDOUT: no
Enabled on STDERR: yes

• running php test.php >out.txt 2>&1, the contents of out.txt is

Enable on STDOUT: failed
Enable on STDERR: failed

Enabled on STDOUT: no
Enabled on STDERR: no

[weltling]

The void* cast will cause the stack corruption on 64-bit.

[mlocati]

The void* cast will cause the stack corruption on 64-bit.

Fixed in 694e207

[weltling]

These functions should be exported. Later it can be integrated with phpdbg, but can be useful for non core as well. Easy done like in win32/winutil.h.

[mlocati]

Fixed in 2f05643

[weltling]

It's a nice approach, but i'd rather avoid adding the new section to run-tests.php. It is not a general case and probably only useful for this concrete case, so IMO a new section is not justified.

What could be done instead, were doing an extra PHP run inside the test. Fe we do this for the CLI server, you can cehck sapi/cli/tests/php_cli_server.inc for a simple example framework. It is also better to be close to the real word, when testing redirection. Like

php.exe -r "if (stream_vt100_support(STDOUT, true)) echo \"\033[101;93msupported\033[0m\n\"; else echo 'not supported'; " > tmp

should should out "not supported" into a file on both linux and windows. Or i don't get this part right?

Thanks.

[weltling]

@mlocati for the stream option, it could be similar like I've mentioned in the ticket, like

stream_context_set_option($fd, "stdio", "enable_vt100", true);

It were same as using the function. As for me, the stream option is not absolutely necessary in the current implementation. It would become necessary, if we'd go for an automation to strip the ANSI controls.

Thanks.

[johnstevenson]

@mlocati Great work in trying to get this incorporated (I have been following this via Symfony and Composer issues).

However, your implementation is incorrect in testing if the handle is redirected, rather than testing if it is a terminal (which is what isatty does on posix systems). Your function will only return true on a file, which means that when you give a pipe handle to GetConsoleMode it will fail (as it will on a file).

Microsoft have updated their vt100 documentation and state that:

SetConsoleMode returning with STATUS_INVALID_PARAMETER is the current mechanism to determine when running on a down-level system

so there is no need for any version checking, which will make the code simpler. Likewise, you can use GetConsoleMode as the equivalent of isatty, as mentioned above (which is what Docker does).

[mlocati]

This should be zend_long64, at least for PHP 7.x series, see Zend/zend_types.h for more global types :)

Did you mean zend_long, right? I can't find any definition of zend_long64...

[mlocati]

@johnstevenson Thanks! I'd keep a "is this stream a valid console" function, so that we can update it in case the current implementation needs future adjustments (btw, I replaced GetFinalPathNameByHandleW with GetConsole as you suggested: it now works with pipes too, thanks!)

[mlocati]

@weltling So, instead of

#ifdef _WIN64
    (zend_long or long long) fileno;
#else
    int fileno;
#endif

I should write

    intptr_t fileno;

?

[weltling]

@mlocati yeah, that's fine. The (void *) would be exact same size platform dependent.

Thanks.

[mlocati]

@weltling I used zend_long...

About stream_vt100_support() vs stream_context_set_option(): let's assume this PHP code:

$ctx1 = stream_context_create();
stream_context_set_option($ctx1, 'stdio', 'enable_vt100', true);

$ctx2 = stream_context_create();
stream_context_set_option($ctx2, 'stdio', 'enable_vt100', false);

The problem here is that one may think that the two contexts are isolated, but they are not...

[weltling]

@mlocati you're right. I was just testing and looking yet more over the patch. Please consider a snippet like

$fd = fopen("php://stdout", "w");
if (stream_vt100_support($fd, true)) {
    fwrite($fd, "\033[101;93msupported\033[0m\n"); 
} else {
    fwrite($fd, 'not supported');
}

In the current implementation it won't work, as the checks are focused on the original stdio handles. The $fd stream is however still associated with some valid console. IMO we should consider this as a bug. So far I see, the checks whether the underlying stream fd is the exact std handle is not necessary. The fileno should be simply passed to php_win32_console*() functions, and then _get_osfhandle should be used. The Windows console API should handle it correctly then.

Any php stream may point to a valid console, not just STDIO. I guess that set/get console mode will anyway apply to the given console object disregarding the fd used. Full isolation seems not to be possible therefore, as that's the way how the low level API works. But on the PHP side it should be possible to operate on $fd same way it's possible for STDIO directly. That will allow then to append filters to any vt100 enabled/disabled stream.

Thanks

[mlocati]

_get_osfhandle returns -2 if the handle is invalid (rather than -1).

@johnstevenson I did some tests, and it always returned -1 (INVALID_HANDLE_VALUE). Do you have some use case when _get_osfhandle returns -2?

[johnstevenson]

@mlocati If you debug php-win.exe and step through php_win32_console_fileno_set_vt100 you can see the handle being returned as -2 (which makes GetConsoleMode fail anyway).

@weltling Blame me for suggesting the PHP_CLI_WIN32_NO_CONSOLE guard because I noticed that it was used around the php_win32_cp_cli_do_setup() code page stuff.

[weltling]

@mlocati ok, in that case it might be interesting with appveyor. You rule actually :) There is a couple other things going on yet. Like the work on the new SDK tools, which interferes with related areas. All in all, IMO it's worth it to create a PR for the matter and discuss there.

@johnstevenson yeah, that's a similar case, as SetConsoleCP and co would fail as well.

Thanks.

[weltling]

Merged with 33301d5.

I've also added some basic doc to UPGRADING. @mlocati, if you've time for more doc - please either do a new PR, or gist the change and ping me, so it could be added there. Also, if you've time, you might check to integrate the enablement and handling into phpdbg.

Thanks!

[mlocati]

Great!!
In these days my spare time is over, but I think I'll be more free in a week or so.

[mlocati]

PS: what are the plans for these fubctions? I mean, for which php versions will them be available?

[weltling]

@mlocati, it's merged into master, which is the future 7.2 nowadays. 7.1 was already freezed by the discussion time, anyway. And otherwise, it is fine to go through a good QA for the patches of such extent. The 7.2 cycle will start spring 2017, so there's enough time for possible fix ups and doc. I've already positive feedback from yet another tester, the PR is therefore fine for master. One thing standing out yet is the newly released Nano Server, as it has quite minimalist console capabilities. More or less, that's the approx plan for this feature.

Thanks.

[mlocati]

@weltling / @johnstevenson It seems that the stream_isatty function may have some issue when PHP is executed in Cygwin with Mintty:

$ php -r "var_export(stream_isatty(STDOUT));"
false

I'm not very familiar with the _php_stream_cast function...

[weltling]

@mlocati so far i get with the git shell

weltling@w530-php-dev MINGW64 /c/php-sdk/phpmaster/vc15/x64/php-src (master)
$ x64/Release/php.exe -r "var_export(stream_isatty(STDOUT));"
true

it's based on some parts of Cygwin. Otherwise, no chance for me with pure Cygwin, as it's principally a workspace breaker when it comes to the native VS environment.

Is that a Cygwin build of PHP? The result above is with the native build. FYI, we don't officially support Cygwin and other similar build flavors.

Thanks.

[mlocati]

Is that a Cygwin build of PHP?

Nope: it's the php.exe version from https://windows.php.net/download/#php-7.2-ts-VC15-x86

[mlocati]

$ uname -a
CYGWIN_NT-10.0 MYPC 2.10.0(0.325/5/3) 2018-02-02 15:16 x86_64 Cygwin

$ which php
/cygdrive/c/Dev/PHP/php

$ php -v
PHP 7.2.4 (cli) (built: Mar 28 2018 04:46:46) ( ZTS MSVC15 (Visual C++ 2017) x86 )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
    with Xdebug v2.6.0, Copyright (c) 2002-2018, by Derick Rethans
    with Zend OPcache v7.2.4, Copyright (c) 1999-2018, by Zend Technologies

$ php -r "var_export(stream_isatty(STDOUT));"
false

[johnstevenson]

Cygwin, Msys2 etc emulate pseudo terminals via named pipes, so this is correct (if unhelpful).

[mlocati]

Cygwin, Msys2 etc emulate pseudo terminals via named pipes, so this is correct (if unhelpful).

I've noticed the same problem in WLS. By the way, quite interestingly, with Git Bash this command

php.exe -r "var_export([stream_isatty(STDOUT), sapi_windows_vt100_support(STDOUT)]);"

outputs

array (
  0 => true,
  1 => true,
)

[iworker]

This function (stream_isatty) is working very strange with Docker: when I'm running make test using specific RUN command:

RUN cd php-src-php-7.2.5 && TEST_PHP_ARGS="--show-out" TEST_PHP_DETAILED=1 TESTS=tests/output make test

from Dockerfile:

docker build -t app-php-72 .

I receive errors:

=====================================================================
Number of tests :   88                74
Tests skipped   :   14 ( 15.9%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    5 (  5.7%) (  6.8%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   69 ( 78.4%) ( 93.2%)
---------------------------------------------------------------------
Time taken      :    2 seconds
=====================================================================

FAILED TEST SUMMARY
---------------------------------------------------------------------
Test stream_isatty with redirected STDERR [tests/output/stream_isatty_err.phpt]
Test stream_isatty with redirected STDIN/STDERR [tests/output/stream_isatty_in-err.phpt]
Test stream_isatty with redirected STDIN/STDOUT [tests/output/stream_isatty_in-out.phpt]
Test stream_isatty with redirected STDOUT/STDERR [tests/output/stream_isatty_out-err.phpt]
Test stream_isatty with redirected STDOUT [tests/output/stream_isatty_out.phpt]

When I'm running using docker exec, php and run-tests.php:

docker exec -it 0dd46d08dc29 sh -c "cd /php-src-php-7.2.5 && TEST_PHP_ARGS="--show-out" TEST_PHP_DETAILED=1 TESTS=tests/output make test"

I receive no errors:

=====================================================================
Number of tests :   88                74
Tests skipped   :   14 ( 15.9%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   74 ( 84.1%) (100.0%)
---------------------------------------------------------------------
Time taken      :    3 seconds
=====================================================================

Is somebody knows what is the cause of such strange situation? :)

P.S.:
There is one of the failed tests:

=================
TEST /php-src-php-7.2.5/tests/output/stream_isatty_out.phpt
TEST 88/88 [tests/output/stream_isatty_out.phpt]
CONTENT_LENGTH  = 
CONTENT_TYPE    = 
PATH_TRANSLATED = /php-src-php-7.2.5/tests/output/stream_isatty_out.php
QUERY_STRING    = 
REDIRECT_STATUS = 1
REQUEST_METHOD  = GET
SCRIPT_FILENAME = /php-src-php-7.2.5/tests/output/stream_isatty_out.php
HTTP_COOKIE     = 
COMMAND /php-src-php-7.2.5/sapi/cli/php  -n -c '/php-src-php-7.2.5/tmp-php.ini'  -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "memory_limit=128M" -d "log_errors_max_len=0" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "zend.assertions=1" -d "extension_dir=/php-src-php-7.2.5/modules/" -d "zend_extension=/php-src-php-7.2.5/modules/opcache.so" -d "session.auto_start=0" -d "mbstring.func_overload=0" -f "/php-src-php-7.2.5/tests/output/stream_isatty_out.php" 

========OUT========
STDIN (constant): bool(false)
STDIN (fopen): bool(false)
STDIN (php://fd/0): bool(false)
STDOUT (constant): bool(false)
STDOUT (fopen): bool(false)
STDOUT (php://fd/1): bool(false)
STDERR (constant): bool(false)
STDERR (fopen): bool(false)
STDERR (php://fd/2): bool(false)
Not a stream: 
Warning: stream_isatty() expects parameter 1 to be resource, string given in /php-src-php-7.2.5/tests/output/stream_isatty.inc on line 23
bool(false)
Invalid stream (php://temp): bool(false)
Invalid stream (php://input): bool(false)
Invalid stream (php://memory): bool(false)
File stream: bool(false)
========DONE========
FAIL Test stream_isatty with redirected STDOUT [tests/output/stream_isatty_out.phpt] 

and the same when it passed:

=================
TEST /php-src-php-7.2.5/tests/output/stream_isatty_out.phpt
TEST 88/88 [tests/output/stream_isatty_out.phpt]
CONTENT_LENGTH  = 
CONTENT_TYPE    = 
PATH_TRANSLATED = /php-src-php-7.2.5/tests/output/stream_isatty_out.php
QUERY_STRING    = 
REDIRECT_STATUS = 1
REQUEST_METHOD  = GET
SCRIPT_FILENAME = /php-src-php-7.2.5/tests/output/stream_isatty_out.php
HTTP_COOKIE     = 
COMMAND /php-src-php-7.2.5/sapi/cli/php  -n -c '/php-src-php-7.2.5/tmp-php.ini'  -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "memory_limit=128M" -d "log_errors_max_len=0" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "zend.assertions=1" -d "extension_dir=/php-src-php-7.2.5/modules/" -d "zend_extension=/php-src-php-7.2.5/modules/opcache.so" -d "session.auto_start=0" -d "mbstring.func_overload=0" -f "/php-src-php-7.2.5/tests/output/stream_isatty_out.php" 

========OUT========
STDIN (constant): bool(true)
STDIN (fopen): bool(true)
STDIN (php://fd/0): bool(true)
STDOUT (constant): bool(false)
STDOUT (fopen): bool(false)
STDOUT (php://fd/1): bool(false)
STDERR (constant): bool(true)
STDERR (fopen): bool(true)
STDERR (php://fd/2): bool(true)
Not a stream: 
Warning: stream_isatty() expects parameter 1 to be resource, string given in /php-src-php-7.2.5/tests/output/stream_isatty.inc on line 23
bool(false)
Invalid stream (php://temp): bool(false)
Invalid stream (php://input): bool(false)
Invalid stream (php://memory): bool(false)
File stream: bool(false)
========DONE========
PASS Test stream_isatty with redirected STDOUT [tests/output/stream_isatty_out.phpt]