From 23a20304381c930ea0843386a650044846586a0b Mon Sep 17 00:00:00 2001
From: Jakub Zelenka <bukka@php.net>
Date: Sat, 7 May 2022 20:13:50 +0100
Subject: [PATCH] Fix bug #72185: php-fpm writes empty fcgi record causing
 nginx 502

This issue might happen if there is change of the fcgi stream when
the buffer is full. Then the empty record is created which signals
end of stream which is incorrect.

The actual fix without a test was contributed by GitHub user @loveharmful
in GH-3198.
---
 NEWS                                          |  2 +
 main/fastcgi.c                                |  6 +--
 sapi/fpm/tests/bug72185-fcgi-empty-frame.phpt | 52 +++++++++++++++++++
 sapi/fpm/tests/fcgi.inc                       | 26 ++++++++--
 sapi/fpm/tests/logtool.inc                    |  2 +-
 5 files changed, 80 insertions(+), 8 deletions(-)
 create mode 100644 sapi/fpm/tests/bug72185-fcgi-empty-frame.phpt

diff --git a/NEWS b/NEWS
index 0d0392fe81b..8d64db358d8 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,8 @@ PHP                                                                        NEWS
 
 - FPM:
   . Fixed ACL build check on MacOS. (David Carlier)
+  . Fixed bug #72185: php-fpm writes empty fcgi record causing nginx 502.
+    (Jakub Zelenka, loveharmful)
 
 - SPL:
   . Fixed bug GH-8235 (iterator_count() may run indefinitely). (cmb)
diff --git a/main/fastcgi.c b/main/fastcgi.c
index dacb06bcfec..bb9c2b237a6 100644
--- a/main/fastcgi.c
+++ b/main/fastcgi.c
@@ -1591,10 +1591,10 @@ int fcgi_write(fcgi_request *req, fcgi_request_type type, const char *str, int l
 		memcpy(req->out_pos, str, len);
 		req->out_pos += len;
 	} else if (len - limit < (int)(sizeof(req->out_buf) - sizeof(fcgi_header))) {
-		if (!req->out_hdr) {
-			open_packet(req, type);
-		}
 		if (limit > 0) {
+			if (!req->out_hdr) {
+				open_packet(req, type);
+			}
 			memcpy(req->out_pos, str, limit);
 			req->out_pos += limit;
 		}
diff --git a/sapi/fpm/tests/bug72185-fcgi-empty-frame.phpt b/sapi/fpm/tests/bug72185-fcgi-empty-frame.phpt
new file mode 100644
index 00000000000..7c4b4468e35
--- /dev/null
+++ b/sapi/fpm/tests/bug72185-fcgi-empty-frame.phpt
@@ -0,0 +1,52 @@
+--TEST--
+FPM: bug72185 - FastCGI empty frame incorrectly created
+--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 = 2
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 2
+catch_workers_output = yes
+decorate_workers_output = no
+php_value[html_errors] = false
+EOT;
+
+$code = <<<'EOT'
+<?php
+for ($i=0; $i < 167; $i++) {
+    error_log('PHP is the best programming language');
+}
+echo 'end';
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+$tester->request()->expectBody('end');
+for ($i=0; $i < 167; $i++) {
+    $tester->expectLogNotice("PHP message: PHP is the best programming language");
+}
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->close();
+
+?>
+Done
+--EXPECT--
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git a/sapi/fpm/tests/fcgi.inc b/sapi/fpm/tests/fcgi.inc
index 0d16e5ba0ba..d17a659dd9c 100644
--- a/sapi/fpm/tests/fcgi.inc
+++ b/sapi/fpm/tests/fcgi.inc
@@ -555,6 +555,25 @@ class Client
         return $id;
     }
 
+    /**
+     * Append response data.
+     *
+     * @param $resp Response
+     * @param $type Either err or our
+     *
+     * @throws \Exception
+     */
+    private function fcgi_stream_append($resp, $type) {
+        if (isset($this->_requests[$resp['requestId']][$type . '_finished'])) {
+            throw new \Exception('FCGI_STD' . strtoupper($type) . ' stream already finished by empty record');
+        }
+        if ($resp['content'] === '') {
+            $this->_requests[$resp['requestId']][$type . '_finished'] = true;
+        } else {
+            $this->_requests[$resp['requestId']][$type . '_response'] .= $resp['content'];
+        }
+    }
+
     /**
      * Blocking call that waits for response data of the specific request
      *
@@ -593,13 +612,12 @@ class Client
             if ($resp['type'] == self::STDOUT || $resp['type'] == self::STDERR) {
                 if ($resp['type'] == self::STDERR) {
                     $this->_requests[$resp['requestId']]['state'] = self::REQ_STATE_ERR;
-                    $this->_requests[$resp['requestId']]['err_response'] .= $resp['content'];
+                    $this->fcgi_stream_append($resp, 'err');
                 } else {
-                    $this->_requests[$resp['requestId']]['out_response'] .= $resp['content'];
+                    $this->fcgi_stream_append($resp, 'out');
                 }
                 $this->_requests[$resp['requestId']]['response'] .= $resp['content'];
-            }
-            if ($resp['type'] == self::END_REQUEST) {
+            } elseif ($resp['type'] == self::END_REQUEST) {
                 $this->_requests[$resp['requestId']]['state'] = self::REQ_STATE_OK;
                 if ($resp['requestId'] == $requestId) {
                     break;
diff --git a/sapi/fpm/tests/logtool.inc b/sapi/fpm/tests/logtool.inc
index caf88ce662b..e085ae291be 100644
--- a/sapi/fpm/tests/logtool.inc
+++ b/sapi/fpm/tests/logtool.inc
@@ -364,7 +364,7 @@ class LogTool
         }
 
         $line = rtrim($line);
-        $pattern = sprintf('/^%s %s: %s$/', self::P_TIME, $type, $expectedMessage);
+        $pattern = sprintf('/^(%s )?%s: %s$/', self::P_TIME, $type, $expectedMessage);
 
         if (preg_match($pattern, $line, $matches) === 0) {
             return $this->error(