Skip to content

Commit 5a4520b

Browse files
bukkabukka
committed
Fix bug #68207: Setting fastcgi.error_header can result in a WARNING
1 parent 31b20f1 commit 5a4520b

File tree

6 files changed

+314
-66
lines changed

6 files changed

+314
-66
lines changed

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ PHP NEWS
4747
php-fpm 8.1.11). (Jakub Zelenka)
4848
. Fixed bug GH-9959 (Solaris port event mechanism is still broken after bug
4949
#66694). (Petr Sumbera)
50+
. Fixed bug #68207 (Setting fastcgi.error_header can result in a WARNING).
51+
(Jakub Zelenka)
5052

5153
- mysqli:
5254
. Fixed bug GH-9841 (mysqli_query throws warning despite using

‎sapi/fpm/fpm/fpm_main.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1924,7 +1924,7 @@ consult the installation file that came with this distribution, or visit \n\
19241924
request_body_fd = -2;
19251925

19261926
if (UNEXPECTED(EG(exit_status) == 255)) {
1927-
if (CGIG(error_header) && *CGIG(error_header)) {
1927+
if (CGIG(error_header) && *CGIG(error_header) && !SG(headers_sent)) {
19281928
sapi_header_line ctr = {0};
19291929

19301930
ctr.line = CGIG(error_header);

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
FPM: bug68207 - fastcgi.error_header setting headers after sent
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc"; ?>
6+
--FILE--
7+
<?php
8+
9+
require_once "tester.inc";
10+
11+
$cfg = <<<EOT
12+
[global]
13+
error_log = {{FILE:LOG}}
14+
[unconfined]
15+
listen = {{ADDR}}
16+
pm = static
17+
pm.max_children = 1
18+
catch_workers_output = yes
19+
EOT;
20+
21+
$code = <<<EOT
22+
<?php
23+
echo 1;
24+
fastcgi_finish_request();
25+
d();
26+
EOT;
27+
28+
$tester = new FPM\Tester($cfg, $code);
29+
$tester->start(iniEntries: ['fastcgi.error_header' => '"HTTP/1.1 550 PHP Error"']);
30+
$tester->expectLogStartNotices();
31+
$tester->request()->expectBody('1');
32+
$tester->terminate();
33+
$tester->expectLogTerminatingNotices();
34+
$tester->expectNoLogPattern('/Cannot modify header information/');
35+
$tester->close();
36+
37+
?>
38+
Done
39+
--EXPECT--
40+
Done
41+
--CLEAN--
42+
<?php
43+
require_once "tester.inc";
44+
FPM\Tester::clean();
45+
?>

‎sapi/fpm/tests/logreader.inc‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class LogReader
137137
*
138138
* @return false
139139
*/
140-
private function printError(?string $errorMessage): bool
140+
public function printError(?string $errorMessage): bool
141141
{
142142
if (is_null($errorMessage)) {
143143
return false;
@@ -155,8 +155,8 @@ class LogReader
155155
* @param callable $matcher Callback to identify a match
156156
* @param string|null $notFoundMessage Error message if matcher does not succeed.
157157
* @param bool $checkAllLogs Whether to also check past logs.
158-
* @param int $timeoutSeconds Timeout in seconds for reading of all messages.
159-
* @param int $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
158+
* @param int|null $timeoutSeconds Timeout in seconds for reading of all messages.
159+
* @param int|null $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
160160
*
161161
* @return bool
162162
* @throws \Exception
@@ -165,9 +165,18 @@ class LogReader
165165
callable $matcher,
166166
string $notFoundMessage = null,
167167
bool $checkAllLogs = false,
168-
int $timeoutSeconds = 3,
169-
int $timeoutMicroseconds = 0
168+
int $timeoutSeconds = null,
169+
int $timeoutMicroseconds = null
170170
): bool {
171+
if (is_null($timeoutSeconds) && is_null($timeoutMicroseconds)) {
172+
$timeoutSeconds = 3;
173+
$timeoutMicroseconds = 0;
174+
} elseif (is_null($timeoutSeconds)) {
175+
$timeoutSeconds = 0;
176+
} elseif (is_null($timeoutMicroseconds)) {
177+
$timeoutMicroseconds = 0;
178+
}
179+
171180
$startTime = microtime(true);
172181
$endTime = $startTime + $timeoutSeconds + ($timeoutMicroseconds / 1_000_000);
173182
if ($checkAllLogs) {

‎sapi/fpm/tests/logtool.inc‎

Lines changed: 68 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -127,19 +127,33 @@ class LogTool
127127
/**
128128
* Match the matcher checking the log lines using the callback.
129129
*
130-
* @param callable $matcher Callback checking whether the log line matches the expected message.
131-
* @param string $notFoundMessage Error message to show if the message is not found.
130+
* @param callable $matcher Callback checking whether the log line matches the expected message.
131+
* @param string|null $notFoundMessage Error message to show if the message is not found.
132+
* @param bool $checkAllLogs Whether to also check past logs.
133+
* @param int|null $timeoutSeconds Timeout in seconds for reading of all messages.
134+
* @param int|null $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
132135
*
133136
* @return bool
134137
* @throws \Exception
135138
*/
136-
private function match(callable $matcher, string $notFoundMessage): bool
137-
{
139+
private function match(
140+
callable $matcher,
141+
string $notFoundMessage = null,
142+
bool $checkAllLogs = false,
143+
int $timeoutSeconds = null,
144+
int $timeoutMicroseconds = null
145+
): bool {
138146
if ($this->getError()) {
139147
return false;
140148
}
141149

142-
if ($this->logReader->readUntil($matcher, $notFoundMessage)) {
150+
if ($this->logReader->readUntil(
151+
$matcher,
152+
$notFoundMessage,
153+
$checkAllLogs,
154+
$timeoutSeconds,
155+
$timeoutMicroseconds
156+
)) {
143157
$this->popError();
144158

145159
return true;
@@ -434,7 +448,8 @@ class LogTool
434448
* @return bool
435449
* @throws \Exception
436450
*/
437-
public function expectReloadingLogsLines(): bool {
451+
public function expectReloadingLogsLines(): bool
452+
{
438453
return (
439454
$this->expectNotice('error log file re-opened') &&
440455
$this->expectNotice('access log file re-opened')
@@ -570,10 +585,14 @@ class LogTool
570585
/**
571586
* Expect log entry.
572587
*
573-
* @param string $type Entry type like NOTICE, WARNING, DEBUG and so on.
574-
* @param string $expectedMessage Message to search for
575-
* @param string|null $pool Pool that is used and prefixes the message.
576-
* @param string $ignoreErrorFor Ignore error for supplied string in the message.
588+
* @param string $type Entry type like NOTICE, WARNING, DEBUG and so on.
589+
* @param string $expectedMessage Message to search for
590+
* @param string|null $pool Pool that is used and prefixes the message.
591+
* @param string $ignoreErrorFor Ignore error for supplied string in the message.
592+
* @param bool $checkAllLogs Whether to also check past logs.
593+
* @param bool $invert Whether the log entry is not expected rather than expected.
594+
* @param int|null $timeoutSeconds Timeout in seconds for reading of all messages.
595+
* @param int|null $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
577596
*
578597
* @return bool
579598
* @throws \Exception
@@ -582,16 +601,29 @@ class LogTool
582601
string $type,
583602
string $expectedMessage,
584603
string $pool = null,
585-
string $ignoreErrorFor = self::DEBUG
604+
string $ignoreErrorFor = self::DEBUG,
605+
bool $checkAllLogs = false,
606+
bool $invert = false,
607+
int $timeoutSeconds = null,
608+
int $timeoutMicroseconds = null
586609
): bool {
587610
if ($this->getError()) {
588611
return false;
589612
}
590613

591-
return $this->match(
614+
$matchResult = $this->match(
592615
$this->getEntryMatcher($type, $expectedMessage, $pool, $ignoreErrorFor),
593-
"The $type does not match expected message"
616+
$invert ? null : "The $type does not match expected message",
617+
$checkAllLogs,
618+
$timeoutSeconds,
619+
$timeoutMicroseconds
594620
);
621+
622+
if ($matchResult && $invert) {
623+
return $this->error("The $type matches unexpected message");
624+
}
625+
626+
return $matchResult;
595627
}
596628

597629
/**
@@ -667,14 +699,23 @@ class LogTool
667699
/**
668700
* Expect pattern in the log line.
669701
*
670-
* @param string $pattern
702+
* @param string $pattern Pattern to use.
703+
* @param bool $invert Whether to expect pattern not to match.
704+
* @param bool $checkAllLogs Whether to also check past logs.
705+
* @param int|null $timeoutSeconds Timeout in seconds for reading of all messages.
706+
* @param int|null $timeoutMicroseconds Additional timeout in microseconds for reading of all messages.
671707
*
672708
* @return bool
673709
* @throws \Exception
674710
*/
675-
public function expectPattern(string $pattern): bool
676-
{
677-
return $this->match(
711+
public function expectPattern(
712+
string $pattern,
713+
bool $invert = false,
714+
bool $checkAllLogs = false,
715+
int $timeoutSeconds = null,
716+
int $timeoutMicroseconds = null,
717+
): bool {
718+
$matchResult = $this->match(
678719
function ($line) use ($pattern) {
679720
if (preg_match($pattern, $line) === 1) {
680721
$this->traceMatch("Pattern expectation", $pattern, $line);
@@ -684,8 +725,17 @@ class LogTool
684725

685726
return false;
686727
},
687-
'The search pattern not found'
728+
$invert ? null : 'The search pattern not found',
729+
$checkAllLogs,
730+
$timeoutSeconds,
731+
$timeoutMicroseconds
688732
);
733+
734+
if ($invert && $matchResult) {
735+
return $this->logReader->printError('The search pattern found - PATTERN: ' . $pattern);
736+
}
737+
738+
return $matchResult;
689739
}
690740

691741
/**

0 commit comments

Comments
 (0)