Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sapi/fpm/fpm/fpm_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
45 changes: 45 additions & 0 deletions sapi/fpm/tests/bug68207-fastcgi-error-header-sent.phpt
Original file line number Diff line number Diff line change
@@ -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();
?>
19 changes: 14 additions & 5 deletions sapi/fpm/tests/logreader.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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) {
Expand Down
86 changes: 68 additions & 18 deletions sapi/fpm/tests/logtool.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

/**
Expand Down
Loading