From fe36f673adfe03ae24a71bc747a7a45b1cf5b45f Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sun, 8 Oct 2023 22:21:31 +0100 Subject: [PATCH] Fix bug #76922: FastCGI terminates conn after FCGI_GET_VALUES Closes GH-12387 --- main/fastcgi.c | 7 +- .../tests/bug76922-fcgi-get-value-conn.phpt | 43 ++++++++++ sapi/fpm/tests/fcgi.inc | 10 +-- sapi/fpm/tests/response.inc | 86 +++++++++++++++++++ sapi/fpm/tests/tester.inc | 32 +++++++ 5 files changed, 170 insertions(+), 8 deletions(-) create mode 100644 sapi/fpm/tests/bug76922-fcgi-get-value-conn.phpt diff --git a/main/fastcgi.c b/main/fastcgi.c index a77491f1bf23..df309df9fdc7 100644 --- a/main/fastcgi.c +++ b/main/fastcgi.c @@ -1202,7 +1202,7 @@ static int fcgi_read_request(fcgi_request *req) req->keep = 0; return 0; } - return 0; + return 2; } else { return 0; } @@ -1470,7 +1470,8 @@ int fcgi_accept_request(fcgi_request *req) return -1; } req->hook.on_read(); - if (fcgi_read_request(req)) { + int read_result = fcgi_read_request(req); + if (read_result == 1) { #ifdef _WIN32 if (is_impersonate && !req->tcp) { pipe = (HANDLE)_get_osfhandle(req->fd); @@ -1481,7 +1482,7 @@ int fcgi_accept_request(fcgi_request *req) } #endif return req->fd; - } else { + } else if (read_result == 0) { fcgi_close(req, 1, 1); } } diff --git a/sapi/fpm/tests/bug76922-fcgi-get-value-conn.phpt b/sapi/fpm/tests/bug76922-fcgi-get-value-conn.phpt new file mode 100644 index 000000000000..20d3dd7b815c --- /dev/null +++ b/sapi/fpm/tests/bug76922-fcgi-get-value-conn.phpt @@ -0,0 +1,43 @@ +--TEST-- +FPM: bug76922 - FCGI conn termination after FCGI_GET_VALUES +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +$tester->requestValues(connKeepAlive: true)->expectValue('FCGI_MPXS_CONNS', '0'); +$tester->request(connKeepAlive: true)->expectBody('1'); +$tester->requestValues(connKeepAlive: true)->expectValue('FCGI_MPXS_CONNS', '0'); +$tester->terminate(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- + +_sock, $this->buildPacket(self::GET_VALUES, $request, 0)); $resp = $this->readPacket(); - if ($resp['type'] == self::GET_VALUES_RESULT) { - return $this->readNvpair($resp['content'], $resp['length']); + if (isset($resp['type']) && $resp['type'] == self::GET_VALUES_RESULT) { + return $this->readNvpair($resp['content'], $resp['contentLength']); } else { throw new \Exception('Unexpected response type, expecting GET_VALUES_RESULT'); } diff --git a/sapi/fpm/tests/response.inc b/sapi/fpm/tests/response.inc index cd5e19d708c5..29105caaffd9 100644 --- a/sapi/fpm/tests/response.inc +++ b/sapi/fpm/tests/response.inc @@ -434,3 +434,89 @@ class Response return false; } } + +class ValuesResponse +{ + /** + * @var array + */ + private array $values; + + /** + * @param string|array|null $values + */ + public function __construct($values = null) + { + if ( ! is_array($values)) { + if ( ! is_null($values) ) { + $this->error('Invalid values supplied', true); + } + $this->values = []; + } else { + $this->values = $values; + } + } + + /** + * Expect value. + * + * @param string $name + * @param mixed $value + * @return ValuesResponse + */ + public function expectValue(string $name, $value = null) + { + if ( ! isset($this->values[$name])) { + return $this->error("Value $name not found in values"); + } + if ( ! is_null($value) && $value !== $this->values[$name]) { + return $this->error("Value $name is {$this->values[$name]} but expected $value"); + } + return $this; + } + + /** + * Get values. + * + * @return array + */ + public function getValues() + { + return $this->values; + } + + /** + * Debug output data. + * + * @return ValuesResponse + */ + public function debugOutput() + { + echo ">>> ValuesResponse\n"; + echo "----------------- Values -----------------\n"; + var_dump($this->values); + echo "---------------------------------------\n\n"; + + return $this; + } + + /** + * Emit error message + * + * @param string $message + * @param bool $throw + * + * @return ValuesResponse + */ + private function error(string $message, $throw = false): bool + { + $errorMessage = "ERROR: $message\n"; + if ($throw) { + throw new \Exception($errorMessage); + } + $this->debugOutput(); + echo $errorMessage; + + return $this; + } +} diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc index 9c9f6b58653d..e97a44bdbd7b 100644 --- a/sapi/fpm/tests/tester.inc +++ b/sapi/fpm/tests/tester.inc @@ -868,6 +868,38 @@ class Tester } } + /** + * Execute request for getting FastCGI values. + * + * @param string|null $address + * @param bool $connKeepAlive + * + * @return ValuesResponse + * @throws \Exception + */ + public function requestValues( + string $address = null, + bool $connKeepAlive = false + ): ValuesResponse { + if ($this->hasError()) { + return new Response(null, true); + } + + try { + $valueResponse = new ValuesResponse( + $this->getClient($address, $connKeepAlive)->getValues(['FCGI_MPXS_CONNS']) + ); + if ($this->debug) { + $this->response->debugOutput(); + } + } catch (\Exception $exception) { + $this->error("Request for getting values failed", $exception); + $valueResponse = new ValuesResponse(); + } + + return $valueResponse; + } + /** * Get client. *