diff --git a/NEWS b/NEWS index fee22ba7eea..2e411e20313 100644 --- a/NEWS +++ b/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 diff --git a/sapi/fpm/fpm/fpm_main.c b/sapi/fpm/fpm/fpm_main.c index a8d7f73406e..eb8fab8e832 100644 --- a/sapi/fpm/fpm/fpm_main.c +++ b/sapi/fpm/fpm/fpm_main.c @@ -1924,7 +1924,7 @@ fastcgi_request_done: 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); diff --git a/sapi/fpm/tests/bug68207-fastcgi-error-header-sent.phpt b/sapi/fpm/tests/bug68207-fastcgi-error-header-sent.phpt new file mode 100644 index 00000000000..3708b51b7a1 --- /dev/null +++ b/sapi/fpm/tests/bug68207-fastcgi-error-header-sent.phpt @@ -0,0 +1,45 @@ +--TEST-- +FPM: bug68207 - fastcgi.error_header setting headers after sent +--SKIPIF-- + +--FILE-- +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-- + diff --git a/sapi/fpm/tests/logreader.inc b/sapi/fpm/tests/logreader.inc index c8ecb3ad2be..96bcc9ec7cf 100644 --- a/sapi/fpm/tests/logreader.inc +++ b/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) { diff --git a/sapi/fpm/tests/logtool.inc b/sapi/fpm/tests/logtool.inc index b1c9009f6f8..64a58ec7600 100644 --- a/sapi/fpm/tests/logtool.inc +++ b/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; } /** diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc index ae60114f78f..c1f090ab145 100644 --- a/sapi/fpm/tests/tester.inc +++ b/sapi/fpm/tests/tester.inc @@ -380,6 +380,7 @@ class Tester * @param bool $forceStderr Whether to output to stderr so error log is used. * @param bool $daemonize Whether to start FPM daemonized * @param array $extensions List of extension to add if shared build used. + * @param array $iniEntries List of ini entries to use. * * @return bool * @throws \Exception @@ -388,7 +389,8 @@ class Tester array $extraArgs = [], bool $forceStderr = true, bool $daemonize = false, - array $extensions = [] + array $extensions = [], + array $iniEntries = [], ) { $configFile = $this->createConfig(); $desc = $this->outDesc ? [] : [1 => array('pipe', 'w'), 2 => array('redirect', 1)]; @@ -412,6 +414,10 @@ class Tester } } + foreach ($iniEntries as $iniEntryName => $iniEntryValue) { + $cmd[] = '-d' . $iniEntryName . '=' . $iniEntryValue; + } + if (getenv('TEST_FPM_RUN_AS_ROOT')) { $cmd[] = '--allow-to-run-as-root'; } @@ -1379,13 +1385,54 @@ class Tester /** * Expect log pattern in logs. * - * @param string $pattern Log pattern + * @param string $pattern Log pattern + * @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. * * @throws \Exception */ - public function expectLogPattern(string $pattern) - { - $this->logTool->expectPattern($pattern); + public function expectLogPattern( + string $pattern, + bool $checkAllLogs = false, + int $timeoutSeconds = null, + int $timeoutMicroseconds = null, + ) { + $this->logTool->expectPattern( + $pattern, + false, + $checkAllLogs, + $timeoutSeconds, + $timeoutMicroseconds + ); + } + + /** + * Expect no such log pattern in logs. + * + * @param string $pattern Log pattern + * @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. + * + * @throws \Exception + */ + public function expectNoLogPattern( + string $pattern, + bool $checkAllLogs = true, + int $timeoutSeconds = null, + int $timeoutMicroseconds = null, + ) { + if (is_null($timeoutSeconds) && is_null($timeoutMicroseconds)) { + $timeoutMicroseconds = 10; + } + $this->logTool->expectPattern( + $pattern, + true, + $checkAllLogs, + $timeoutSeconds, + $timeoutMicroseconds + ); } /** @@ -1439,11 +1486,15 @@ class Tester /** * Expect log entry. * - * @param string $type The log type - * @param string $message The expected message - * @param string|null $pool The pool for pool prefixed log entry - * @param int $count The number of items - * @param bool $checkAllLogs Whether to also check past logs. + * @param string $type The log type. + * @param string $message The expected message. + * @param string|null $pool The pool for pool prefixed log entry. + * @param int $count The number of items. + * @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. + * @param string $ignoreErrorFor Ignore error for supplied string in the message. * * @return bool * @throws \Exception @@ -1453,10 +1504,25 @@ class Tester string $message, string $pool = null, int $count = 1, - bool $checkAllLogs = false + bool $checkAllLogs = false, + bool $invert = false, + int $timeoutSeconds = null, + int $timeoutMicroseconds = null, + string $ignoreErrorFor = LogTool::DEBUG ): bool { for ($i = 0; $i < $count; $i++) { - if ( ! $this->logTool->expectEntry($type, $message, $pool, $checkAllLogs)) { + $result = $this->logTool->expectEntry( + $type, + $message, + $pool, + $ignoreErrorFor, + $checkAllLogs, + $invert, + $timeoutSeconds, + $timeoutMicroseconds, + ); + + if ( ! $result) { return false; } } @@ -1467,10 +1533,13 @@ class Tester /** * Expect a log debug message. * - * @param string $message - * @param string|null $pool - * @param int $count - * @param bool $checkAllLogs Whether to also check past logs. + * @param string $message The expected message. + * @param string|null $pool The pool for pool prefixed log entry. + * @param int $count The number of items. + * @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 @@ -1479,18 +1548,34 @@ class Tester string $message, string $pool = null, int $count = 1, - bool $checkAllLogs = false + bool $checkAllLogs = false, + bool $invert = false, + int $timeoutSeconds = null, + int $timeoutMicroseconds = null ): bool { - return $this->expectLogEntry(LogTool::DEBUG, $message, $pool, $count, $checkAllLogs); + return $this->expectLogEntry( + LogTool::DEBUG, + $message, + $pool, + $count, + $checkAllLogs, + $invert, + $timeoutSeconds, + $timeoutMicroseconds, + LogTool::ERROR + ); } /** * Expect a log notice. * - * @param string $message - * @param string|null $pool - * @param int $count - * @param bool $checkAllLogs Whether to also check past logs. + * @param string $message The expected message. + * @param string|null $pool The pool for pool prefixed log entry. + * @param int $count The number of items. + * @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 @@ -1499,18 +1584,33 @@ class Tester string $message, string $pool = null, int $count = 1, - bool $checkAllLogs = false + bool $checkAllLogs = false, + bool $invert = false, + int $timeoutSeconds = null, + int $timeoutMicroseconds = null ): bool { - return $this->expectLogEntry(LogTool::NOTICE, $message, $pool, $count, $checkAllLogs); + return $this->expectLogEntry( + LogTool::NOTICE, + $message, + $pool, + $count, + $checkAllLogs, + $invert, + $timeoutSeconds, + $timeoutMicroseconds + ); } /** * Expect a log warning. * - * @param string $message - * @param string|null $pool - * @param int $count - * @param bool $checkAllLogs Whether to also check past logs. + * @param string $message The expected message. + * @param string|null $pool The pool for pool prefixed log entry. + * @param int $count The number of items. + * @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 @@ -1519,18 +1619,33 @@ class Tester string $message, string $pool = null, int $count = 1, - bool $checkAllLogs = false + bool $checkAllLogs = false, + bool $invert = false, + int $timeoutSeconds = null, + int $timeoutMicroseconds = null ): bool { - return $this->expectLogEntry(LogTool::WARNING, $message, $pool, $count, $checkAllLogs); + return $this->expectLogEntry( + LogTool::WARNING, + $message, + $pool, + $count, + $checkAllLogs, + $invert, + $timeoutSeconds, + $timeoutMicroseconds + ); } /** * Expect a log error. * - * @param string $message - * @param string|null $pool - * @param int $count - * @param bool $checkAllLogs Whether to also check past logs. + * @param string $message The expected message. + * @param string|null $pool The pool for pool prefixed log entry. + * @param int $count The number of items. + * @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 @@ -1539,18 +1654,33 @@ class Tester string $message, string $pool = null, int $count = 1, - bool $checkAllLogs = false + bool $checkAllLogs = false, + bool $invert = false, + int $timeoutSeconds = null, + int $timeoutMicroseconds = null ): bool { - return $this->expectLogEntry(LogTool::ERROR, $message, $pool, $count, $checkAllLogs); + return $this->expectLogEntry( + LogTool::ERROR, + $message, + $pool, + $count, + $checkAllLogs, + $invert, + $timeoutSeconds, + $timeoutMicroseconds + ); } /** * Expect a log alert. * - * @param string $message - * @param string|null $pool - * @param int $count - * @param bool $checkAllLogs Whether to also check past logs. + * @param string $message The expected message. + * @param string|null $pool The pool for pool prefixed log entry. + * @param int $count The number of items. + * @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 @@ -1559,9 +1689,21 @@ class Tester string $message, string $pool = null, int $count = 1, - bool $checkAllLogs = false + bool $checkAllLogs = false, + bool $invert = false, + int $timeoutSeconds = null, + int $timeoutMicroseconds = null ): bool { - return $this->expectLogEntry(LogTool::ALERT, $message, $pool, $count, $checkAllLogs); + return $this->expectLogEntry( + LogTool::ALERT, + $message, + $pool, + $count, + $checkAllLogs, + $invert, + $timeoutSeconds, + $timeoutMicroseconds + ); } /**