bug symfony#53576 [Console] Only execute additional checks for color support if the output (theofidry)

fabpot · fabpot · commit 1c7c7303fb26 · 2024-01-19T12:11:37.000+01:00
This PR was merged into the 5.4 branch. Discussion ---------- [Console] Only execute additional checks for color support if the output is a TTY | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#53460 | License | MIT As reported in symfony#53460, the modifications done in symfony#52940 are incorrect as it results in detecting that the stream can support colors despite not being a TTY. Rather than doing a simple revert which would re-introduce the pre-existing issue that symfony#52940 attempted to fix, this PR checks if the output is a TTY based on Composer's code and does this check before anything else. Indeed a TTY only means that colors _may_ be supported, so the various checks that we were doing do make sense to be done but should be done after this TTY (so `Hyper` is not an exception, it can be a TTY or not). Commits ------- 2e96b22 [Console] Only execute additional checks for color support if the output is a TTY
diff --git a/src/Symfony/Component/Console/Output/StreamOutput.php b/src/Symfony/Component/Console/Output/StreamOutput.php
@@ -95,6 +95,10 @@ protected function hasColorSupport()
             return false;
         }
 
+        if (!$this->isTty()) {
+            return false;
+        }
+
         if (\DIRECTORY_SEPARATOR === '\\'
             && \function_exists('sapi_windows_vt100_support')
             && @sapi_windows_vt100_support($this->stream)
@@ -105,7 +109,36 @@ protected function hasColorSupport()
         return 'Hyper' === getenv('TERM_PROGRAM')
             || false !== getenv('ANSICON')
             || 'ON' === getenv('ConEmuANSI')
-            || str_starts_with((string) getenv('TERM'), 'xterm')
-            || stream_isatty($this->stream);
+            || str_starts_with((string) getenv('TERM'), 'xterm');
+    }
+
+    /**
+     * Checks if the stream is a TTY, i.e; whether the output stream is connected to a terminal.
+     *
+     * Reference: Composer\Util\Platform::isTty
+     * https://github.com/composer/composer
+     */
+    private function isTty(): bool
+    {
+        // Detect msysgit/mingw and assume this is a tty because detection
+        // does not work correctly, see https://github.com/composer/composer/issues/9690
+        if (\in_array(strtoupper((string) getenv('MSYSTEM')), ['MINGW32', 'MINGW64'], true)) {
+            return true;
+        }
+
+        // Modern cross-platform function, includes the fstat fallback so if it is present we trust it
+        if (\function_exists('stream_isatty')) {
+            return stream_isatty($this->stream);
+        }
+
+        // Only trusting this if it is positive, otherwise prefer fstat fallback.
+        if (\function_exists('posix_isatty') && posix_isatty($this->stream)) {
+            return true;
+        }
+
+        $stat = @fstat($this->stream);
+
+        // Check if formatted mode is S_IFCHR
+        return $stat ? 0020000 === ($stat['mode'] & 0170000) : false;
     }
 }
