Skip to content

Commit e3d1beb

Browse files
bukkabukka
committed
Fix bug #76922: FastCGI terminates conn after FCGI_GET_VALUES
Closes GH-12387
1 parent 7e5fb56 commit e3d1beb

File tree

6 files changed

+172
-8
lines changed

6 files changed

+172
-8
lines changed

‎NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ PHP NEWS
2424
- FPM:
2525
. Fixed bug GH-12232 (FPM: segfault dynamically loading extension without
2626
opcache). (Jakub Zelenka)
27+
. Fixed bug #76922 (FastCGI terminates conn after FCGI_GET_VALUES).
28+
(Jakub Zelenka)
2729

2830
- Intl:
2931
. Removed the BC break on IntlDateFormatter::construct which threw an

‎main/fastcgi.c

+4-3
Original file line numberDiff line numberDiff line change
@@ -1202,7 +1202,7 @@ static int fcgi_read_request(fcgi_request *req)
12021202
req->keep = 0;
12031203
return 0;
12041204
}
1205-
return 0;
1205+
return 2;
12061206
} else {
12071207
return 0;
12081208
}
@@ -1470,7 +1470,8 @@ int fcgi_accept_request(fcgi_request *req)
14701470
return -1;
14711471
}
14721472
req->hook.on_read();
1473-
if (fcgi_read_request(req)) {
1473+
int read_result = fcgi_read_request(req);
1474+
if (read_result == 1) {
14741475
#ifdef _WIN32
14751476
if (is_impersonate && !req->tcp) {
14761477
pipe = (HANDLE)_get_osfhandle(req->fd);
@@ -1481,7 +1482,7 @@ int fcgi_accept_request(fcgi_request *req)
14811482
}
14821483
#endif
14831484
return req->fd;
1484-
} else {
1485+
} else if (read_result == 0) {
14851486
fcgi_close(req, 1, 1);
14861487
}
14871488
}

‎sapi/fpm/tests/bug76922-fcgi-get-value-conn.phpt

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
FPM: bug76922 - FCGI conn termination after FCGI_GET_VALUES
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
[unconfined]
14+
listen = {{ADDR}}
15+
pm = static
16+
pm.max_children = 1
17+
catch_workers_output = yes
18+
EOT;
19+
20+
$code = <<<EOT
21+
<?php
22+
echo 1;
23+
EOT;
24+
25+
$tester = new FPM\Tester($cfg, $code);
26+
$tester->start();
27+
$tester->expectLogStartNotices();
28+
$tester->requestValues(connKeepAlive: true)->expectValue('FCGI_MPXS_CONNS', '0');
29+
$tester->request(connKeepAlive: true)->expectBody('1');
30+
$tester->requestValues(connKeepAlive: true)->expectValue('FCGI_MPXS_CONNS', '0');
31+
$tester->terminate();
32+
$tester->close();
33+
34+
?>
35+
Done
36+
--EXPECT--
37+
Done
38+
--CLEAN--
39+
<?php
40+
require_once "tester.inc";
41+
FPM\Tester::clean();
42+
?>
43+
<?php

‎sapi/fpm/tests/fcgi.inc

+5-5
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ class Client
6060
const OVERLOADED = 2;
6161
const UNKNOWN_ROLE = 3;
6262

63-
const MAX_CONNS = 'MAX_CONNS';
64-
const MAX_REQS = 'MAX_REQS';
65-
const MPXS_CONNS = 'MPXS_CONNS';
63+
const MAX_CONNS = 'FCGI_MAX_CONNS';
64+
const MAX_REQS = 'FCGI_MAX_REQS';
65+
const MPXS_CONNS = 'FCGI_MPXS_CONNS';
6666

6767
const HEADER_LEN = 8;
6868

@@ -454,8 +454,8 @@ class Client
454454
fwrite($this->_sock, $this->buildPacket(self::GET_VALUES, $request, 0));
455455

456456
$resp = $this->readPacket();
457-
if ($resp['type'] == self::GET_VALUES_RESULT) {
458-
return $this->readNvpair($resp['content'], $resp['length']);
457+
if (isset($resp['type']) && $resp['type'] == self::GET_VALUES_RESULT) {
458+
return $this->readNvpair($resp['content'], $resp['contentLength']);
459459
} else {
460460
throw new \Exception('Unexpected response type, expecting GET_VALUES_RESULT');
461461
}

‎sapi/fpm/tests/response.inc

+86
Original file line numberDiff line numberDiff line change
@@ -434,3 +434,89 @@ class Response
434434
return false;
435435
}
436436
}
437+
438+
class ValuesResponse
439+
{
440+
/**
441+
* @var array
442+
*/
443+
private array $values;
444+
445+
/**
446+
* @param string|array|null $values
447+
*/
448+
public function __construct($values = null)
449+
{
450+
if ( ! is_array($values)) {
451+
if ( ! is_null($values) ) {
452+
$this->error('Invalid values supplied', true);
453+
}
454+
$this->values = [];
455+
} else {
456+
$this->values = $values;
457+
}
458+
}
459+
460+
/**
461+
* Expect value.
462+
*
463+
* @param string $name
464+
* @param mixed $value
465+
* @return ValuesResponse
466+
*/
467+
public function expectValue(string $name, $value = null)
468+
{
469+
if ( ! isset($this->values[$name])) {
470+
return $this->error("Value $name not found in values");
471+
}
472+
if ( ! is_null($value) && $value !== $this->values[$name]) {
473+
return $this->error("Value $name is {$this->values[$name]} but expected $value");
474+
}
475+
return $this;
476+
}
477+
478+
/**
479+
* Get values.
480+
*
481+
* @return array
482+
*/
483+
public function getValues()
484+
{
485+
return $this->values;
486+
}
487+
488+
/**
489+
* Debug output data.
490+
*
491+
* @return ValuesResponse
492+
*/
493+
public function debugOutput()
494+
{
495+
echo ">>> ValuesResponse\n";
496+
echo "----------------- Values -----------------\n";
497+
var_dump($this->values);
498+
echo "---------------------------------------\n\n";
499+
500+
return $this;
501+
}
502+
503+
/**
504+
* Emit error message
505+
*
506+
* @param string $message
507+
* @param bool $throw
508+
*
509+
* @return ValuesResponse
510+
*/
511+
private function error(string $message, $throw = false): bool
512+
{
513+
$errorMessage = "ERROR: $message\n";
514+
if ($throw) {
515+
throw new \Exception($errorMessage);
516+
}
517+
$this->debugOutput();
518+
echo $errorMessage;
519+
520+
return $this;
521+
}
522+
}

‎sapi/fpm/tests/tester.inc

+32
Original file line numberDiff line numberDiff line change
@@ -868,6 +868,38 @@ class Tester
868868
}
869869
}
870870

871+
/**
872+
* Execute request for getting FastCGI values.
873+
*
874+
* @param string|null $address
875+
* @param bool $connKeepAlive
876+
*
877+
* @return ValuesResponse
878+
* @throws \Exception
879+
*/
880+
public function requestValues(
881+
string $address = null,
882+
bool $connKeepAlive = false
883+
): ValuesResponse {
884+
if ($this->hasError()) {
885+
return new Response(null, true);
886+
}
887+
888+
try {
889+
$valueResponse = new ValuesResponse(
890+
$this->getClient($address, $connKeepAlive)->getValues(['FCGI_MPXS_CONNS'])
891+
);
892+
if ($this->debug) {
893+
$this->response->debugOutput();
894+
}
895+
} catch (\Exception $exception) {
896+
$this->error("Request for getting values failed", $exception);
897+
$valueResponse = new ValuesResponse();
898+
}
899+
900+
return $valueResponse;
901+
}
902+
871903
/**
872904
* Get client.
873905
*

0 commit comments

Comments
 (0)