Fix bug #77106: Missing separator in FPM FastCGI errors

Author: bukka

NEWS

@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.2.2
 
+- FPM:
+  . Fixed bug #77106 (Missing separator in FPM FastCGI errors). (Jakub Zelenka)
+
 - LDAP:
   . Fixed bug GH-10112 (LDAP\Connection::__construct() refers to ldap_create()).
     (cmb)

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();

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();
+?>

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;