Fix bug #68207: Setting fastcgi.error_header can result in a WARNING

NEWS

@@ -47,6 +47,8 @@ PHP                                                                        NEWS
     php-fpm 8.1.11). (Jakub Zelenka)
   . Fixed bug GH-9959 (Solaris port event mechanism is still broken after bug
     #66694). (Petr Sumbera)
+  . Fixed bug #68207 (Setting fastcgi.error_header can result in a WARNING).
+    (Jakub Zelenka)
 
 - mysqli:
   . Fixed bug GH-9841 (mysqli_query throws warning despite using

sapi/fpm/fpm/fpm_main.c

@@ -1924,7 +1924,7 @@ consult the installation file that came with this distribution, or visit \n\
 			request_body_fd = -2;
 
 			if (UNEXPECTED(EG(exit_status) == 255)) {
-				if (CGIG(error_header) && *CGIG(error_header)) {
+				if (CGIG(error_header) && *CGIG(error_header) && !SG(headers_sent)) {
 					sapi_header_line ctr = {0};
 
 					ctr.line = CGIG(error_header);

sapi/fpm/tests/bug68207-fastcgi-error-header-sent.phpt

@@ -0,0 +1,45 @@
+--TEST--
+FPM: bug68207 - fastcgi.error_header setting headers after sent
+--SKIPIF--
+<?php
+include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = static
+pm.max_children = 1
+catch_workers_output = yes
+EOT;
+
+$code = <<<EOT
+<?php
+echo 1;
+fastcgi_finish_request();
+d();
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start(iniEntries: ['fastcgi.error_header' => '"HTTP/1.1 550 PHP Error"']);
+$tester->expectLogStartNotices();
+$tester->request()->expectBody('1');
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->expectNoLogPattern('/Cannot modify header information/');
+$tester->close();
+
+?>
+Done
+--EXPECT--
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>

sapi/fpm/tests/logreader.inc

@@ -137,7 +137,7 @@ class LogReader
      *
      * @return false
      */
-    private function printError(?string $errorMessage): bool
+    public function printError(?string $errorMessage): bool
     {
         if (is_null($errorMessage)) {
             return false;
@@ -155,8 +155,8 @@ class LogReader
      * @param callable    $matcher             Callback to identify a match
      * @param string|null $notFoundMessage     Error message if matcher does not succeed.
      * @param bool        $checkAllLogs        Whether to also check past logs.
-     * @param int         $timeoutSeconds      Timeout in seconds for reading of all messages.
-     * @param int         $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
+     * @param int|null    $timeoutSeconds      Timeout in seconds for reading of all messages.
+     * @param int|null    $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
      *
      * @return bool
      * @throws \Exception
@@ -165,9 +165,18 @@ class LogReader
         callable $matcher,
         string $notFoundMessage = null,
         bool $checkAllLogs = false,
-        int $timeoutSeconds = 3,
-        int $timeoutMicroseconds = 0
+        int $timeoutSeconds = null,
+        int $timeoutMicroseconds = null
     ): bool {
+        if (is_null($timeoutSeconds) && is_null($timeoutMicroseconds)) {
+            $timeoutSeconds      = 3;
+            $timeoutMicroseconds = 0;
+        } elseif (is_null($timeoutSeconds)) {
+            $timeoutSeconds = 0;
+        } elseif (is_null($timeoutMicroseconds)) {
+            $timeoutMicroseconds = 0;
+        }
+
         $startTime = microtime(true);
         $endTime   = $startTime + $timeoutSeconds + ($timeoutMicroseconds / 1_000_000);
         if ($checkAllLogs) {

sapi/fpm/tests/logtool.inc

@@ -127,19 +127,33 @@ class LogTool
     /**
      * Match the matcher checking the log lines using the callback.
      *
-     * @param callable $matcher         Callback checking whether the log line matches the expected message.
-     * @param string   $notFoundMessage Error message to show if the message is not found.
+     * @param callable    $matcher             Callback checking whether the log line matches the expected message.
+     * @param string|null $notFoundMessage     Error message to show if the message is not found.
+     * @param bool        $checkAllLogs        Whether to also check past logs.
+     * @param int|null    $timeoutSeconds      Timeout in seconds for reading of all messages.
+     * @param int|null    $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
      *
      * @return bool
      * @throws \Exception
      */
-    private function match(callable $matcher, string $notFoundMessage): bool
-    {
+    private function match(
+        callable $matcher,
+        string $notFoundMessage = null,
+        bool $checkAllLogs = false,
+        int $timeoutSeconds = null,
+        int $timeoutMicroseconds = null
+    ): bool {
         if ($this->getError()) {
             return false;
         }
 
-        if ($this->logReader->readUntil($matcher, $notFoundMessage)) {
+        if ($this->logReader->readUntil(
+            $matcher,
+            $notFoundMessage,
+            $checkAllLogs,
+            $timeoutSeconds,
+            $timeoutMicroseconds
+        )) {
             $this->popError();
 
             return true;
@@ -434,7 +448,8 @@ class LogTool
      * @return bool
      * @throws \Exception
      */
-    public function expectReloadingLogsLines(): bool {
+    public function expectReloadingLogsLines(): bool
+    {
         return (
             $this->expectNotice('error log file re-opened') &&
             $this->expectNotice('access log file re-opened')
@@ -570,10 +585,14 @@ class LogTool
     /**
      * Expect log entry.
      *
-     * @param string      $type            Entry type like NOTICE, WARNING, DEBUG and so on.
-     * @param string      $expectedMessage Message to search for
-     * @param string|null $pool            Pool that is used and prefixes the message.
-     * @param string      $ignoreErrorFor  Ignore error for supplied string in the message.
+     * @param string      $type                Entry type like NOTICE, WARNING, DEBUG and so on.
+     * @param string      $expectedMessage     Message to search for
+     * @param string|null $pool                Pool that is used and prefixes the message.
+     * @param string      $ignoreErrorFor      Ignore error for supplied string in the message.
+     * @param bool        $checkAllLogs        Whether to also check past logs.
+     * @param bool        $invert              Whether the log entry is not expected rather than expected.
+     * @param int|null    $timeoutSeconds      Timeout in seconds for reading of all messages.
+     * @param int|null    $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
      *
      * @return bool
      * @throws \Exception
@@ -582,16 +601,29 @@ class LogTool
         string $type,
         string $expectedMessage,
         string $pool = null,
-        string $ignoreErrorFor = self::DEBUG
+        string $ignoreErrorFor = self::DEBUG,
+        bool $checkAllLogs = false,
+        bool $invert = false,
+        int $timeoutSeconds = null,
+        int $timeoutMicroseconds = null
     ): bool {
         if ($this->getError()) {
             return false;
         }
 
-        return $this->match(
+        $matchResult = $this->match(
             $this->getEntryMatcher($type, $expectedMessage, $pool, $ignoreErrorFor),
-            "The $type does not match expected message"
+            $invert ? null : "The $type does not match expected message",
+            $checkAllLogs,
+            $timeoutSeconds,
+            $timeoutMicroseconds
         );
+
+        if ($matchResult && $invert) {
+            return $this->error("The $type matches unexpected message");
+        }
+
+        return $matchResult;
     }
 
     /**
@@ -667,14 +699,23 @@ class LogTool
     /**
      * Expect pattern in the log line.
      *
-     * @param string $pattern
+     * @param string   $pattern             Pattern to use.
+     * @param bool     $invert              Whether to expect pattern not to match.
+     * @param bool     $checkAllLogs        Whether to also check past logs.
+     * @param int|null $timeoutSeconds      Timeout in seconds for reading of all messages.
+     * @param int|null $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
      *
      * @return bool
      * @throws \Exception
      */
-    public function expectPattern(string $pattern): bool
-    {
-        return $this->match(
+    public function expectPattern(
+        string $pattern,
+        bool $invert = false,
+        bool $checkAllLogs = false,
+        int $timeoutSeconds = null,
+        int $timeoutMicroseconds = null,
+    ): bool {
+        $matchResult = $this->match(
             function ($line) use ($pattern) {
                 if (preg_match($pattern, $line) === 1) {
                     $this->traceMatch("Pattern expectation", $pattern, $line);
@@ -684,8 +725,17 @@ class LogTool
 
                 return false;
             },
-            'The search pattern not found'
+            $invert ? null : 'The search pattern not found',
+            $checkAllLogs,
+            $timeoutSeconds,
+            $timeoutMicroseconds
         );
+
+        if ($invert && $matchResult) {
+            return $this->logReader->printError('The search pattern found - PATTERN: ' . $pattern);
+        }
+
+        return $matchResult;
     }
 
     /**