From b435f7edd478d666bdf3aa23da5fe78f908e30e6 Mon Sep 17 00:00:00 2001
From: Jakub Zelenka <bukka@php.net>
Date: Tue, 29 Nov 2022 21:47:53 +0000
Subject: [PATCH] Fix bug #77106: Missing separator in FPM FastCGI errors
---
sapi/fpm/fpm/fpm_main.c | 16 +++++--
sapi/fpm/tests/bug77106-fcgi-missing-nl.phpt | 46 ++++++++++++++++++++
sapi/fpm/tests/response.inc | 36 +++++++++++++--
3 files changed, 91 insertions(+), 7 deletions(-)
create mode 100644 sapi/fpm/tests/bug77106-fcgi-missing-nl.phpt
diff --git a/sapi/fpm/fpm/fpm_main.c b/sapi/fpm/fpm/fpm_main.c
index a8d77ef3bed2..aa43e0c5da40 100644
--- a/sapi/fpm/fpm/fpm_main.c
+++ b/sapi/fpm/fpm/fpm_main.c
@@ -149,6 +149,7 @@ typedef struct _php_cgi_globals_struct {
bool force_redirect;
bool discard_path;
bool fcgi_logging;
+ bool fcgi_logging_request_started;
char *redirect_status_env;
HashTable user_config_cache;
char *error_header;
@@ -598,11 +599,16 @@ void sapi_cgi_log_fastcgi(int level, char *message, size_t len)
* - logging is enabled (fastcgi.logging in php.ini)
* - we are currently dealing with a request
* - the message is not empty
- * - the fcgi_write did not fail
*/
- if (CGIG(fcgi_logging) && request && message && len > 0
- && fcgi_write(request, FCGI_STDERR, message, len) < 0) {
- php_handle_aborted_connection();
+ if (CGIG(fcgi_logging) && request && message && len > 0) {
+ if (CGIG(fcgi_logging_request_started)) {
+ fcgi_write(request, FCGI_STDERR, "; ", 2);
+ } else {
+ CGIG(fcgi_logging_request_started) = true;
+ }
+ if (fcgi_write(request, FCGI_STDERR, message, len) < 0) {
+ php_handle_aborted_connection();
+ }
}
}
/* }}} */
@@ -1403,6 +1409,7 @@ static void php_cgi_globals_ctor(php_cgi_globals_struct *php_cgi_globals)
php_cgi_globals->fix_pathinfo = 1;
php_cgi_globals->discard_path = 0;
php_cgi_globals->fcgi_logging = 1;
+ php_cgi_globals->fcgi_logging_request_started = false;
zend_hash_init(&php_cgi_globals->user_config_cache, 0, NULL, user_config_cache_entry_dtor, 1);
php_cgi_globals->error_header = NULL;
php_cgi_globals->fpm_config = NULL;
@@ -1820,6 +1827,7 @@ consult the installation file that came with this distribution, or visit \n\
char *primary_script = NULL;
request_body_fd = -1;
SG(server_context) = (void *) request;
+ CGIG(fcgi_logging_request_started) = false;
init_request_info();
fpm_request_info();
diff --git a/sapi/fpm/tests/bug77106-fcgi-missing-nl.phpt b/sapi/fpm/tests/bug77106-fcgi-missing-nl.phpt
new file mode 100644
index 000000000000..c4c7adf38254
--- /dev/null
+++ b/sapi/fpm/tests/bug77106-fcgi-missing-nl.phpt
@@ -0,0 +1,46 @@
+--TEST--
+FPM: bug77106 - Missing new lines in FCGI error stream
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+php_admin_flag[log_errors] = 1
+EOT;
+
+$code = <<<EOT
+<?php
+echo \$a;
+echo \$a;
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+$tester->request()->expectErrorPattern(
+ '/Undefined variable \$a in .+ on line \d+; PHP message: PHP Warning:/'
+);
+$tester->terminate();
+$tester->close();
+
+?>
+Done
+--EXPECT--
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git a/sapi/fpm/tests/response.inc b/sapi/fpm/tests/response.inc
index dd74e42990b0..c91c33f1cbe8 100644
--- a/sapi/fpm/tests/response.inc
+++ b/sapi/fpm/tests/response.inc
@@ -41,6 +41,11 @@ class Response
*/
private $expectInvalid;
+ /**
+ * @var bool
+ */
+ private bool $debugOutputted = false;
+
/**
* @param string|array|null $data
* @param bool $expectInvalid
@@ -71,9 +76,9 @@ class Response
$body = implode("\n", $body);
}
- if (!$this->checkIfValid()) {
+ if ( ! $this->checkIfValid()) {
$this->error('Response is invalid');
- } elseif (!$this->checkDefaultHeaders($contentType)) {
+ } elseif ( ! $this->checkDefaultHeaders($contentType)) {
$this->error('Response default headers not found');
} elseif ($body !== $this->rawBody) {
if ($multiLine) {
@@ -134,6 +139,26 @@ class Response
return $this;
}
+ /**
+ * Expect error pattern in the response.
+ *
+ * @param string $errorMessagePattern Expected error message RegExp patter.
+ *
+ * @return Response
+ */
+ public function expectErrorPattern(string $errorMessagePattern): Response
+ {
+ $errorData = $this->getErrorData();
+ if (preg_match($errorMessagePattern, $errorData) === 0) {
+ $this->error(
+ "The expected error pattern $errorMessagePattern does not match the returned error '$errorData'"
+ );
+ $this->debugOutput();
+ }
+
+ return $this;
+ }
+
/**
* Expect no error in the response.
*
@@ -187,6 +212,8 @@ class Response
echo "----------------- ERR -----------------\n";
echo $this->data['err_response'] . "\n";
echo "---------------------------------------\n\n";
+
+ $this->debugOutputted = true;
}
/**
@@ -331,8 +358,11 @@ class Response
*
* @return bool
*/
- private function error($message): bool
+ private function error(string $message): bool
{
+ if ( ! $this->debugOutputted) {
+ $this->debugOutput();
+ }
echo "ERROR: $message\n";
return false;