Skip to content

Fix bug #76922: FastCGI terminates conn after FCGI_GET_VALUES #12387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions main/fastcgi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ static int fcgi_read_request(fcgi_request *req)
req->keep = 0;
return 0;
}
return 0;
return 2;
} else {
return 0;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
Expand Down
43 changes: 43 additions & 0 deletions sapi/fpm/tests/bug76922-fcgi-get-value-conn.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--TEST--
FPM: bug76922 - FCGI conn termination after FCGI_GET_VALUES
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php

require_once "tester.inc";

$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
[unconfined]
listen = {{ADDR}}
pm = static
pm.max_children = 1
catch_workers_output = yes
EOT;

$code = <<<EOT
<?php
echo 1;
EOT;

$tester = new FPM\Tester($cfg, $code);
$tester->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--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
<?php
10 changes: 5 additions & 5 deletions sapi/fpm/tests/fcgi.inc
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ class Client
const OVERLOADED = 2;
const UNKNOWN_ROLE = 3;

const MAX_CONNS = 'MAX_CONNS';
const MAX_REQS = 'MAX_REQS';
const MPXS_CONNS = 'MPXS_CONNS';
const MAX_CONNS = 'FCGI_MAX_CONNS';
const MAX_REQS = 'FCGI_MAX_REQS';
const MPXS_CONNS = 'FCGI_MPXS_CONNS';

const HEADER_LEN = 8;

Expand Down Expand Up @@ -454,8 +454,8 @@ class Client
fwrite($this->_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');
}
Expand Down
86 changes: 86 additions & 0 deletions sapi/fpm/tests/response.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
32 changes: 32 additions & 0 deletions sapi/fpm/tests/tester.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down