From c483b59124bda89f86d8a54bcbdb8e5b89fa40ea Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sun, 28 Feb 2021 21:08:17 +0000 Subject: Fix bug #80024: Duplication of info about inherited socket after pool removing --- NEWS | 2 + sapi/fpm/fpm/fpm_sockets.c | 28 +++-- sapi/fpm/tests/bug68391-conf-include-order.phpt | 7 +- .../fpm/tests/bug80024-socket-reduced-inherit.phpt | 49 ++++++++ sapi/fpm/tests/logtool.inc | 39 ++++++- sapi/fpm/tests/tester.inc | 124 ++++++++++++++++++--- 6 files changed, 211 insertions(+), 38 deletions(-) create mode 100644 sapi/fpm/tests/bug80024-socket-reduced-inherit.phpt diff --git a/NEWS b/NEWS index bfb249e46e..42eeddacdc 100644 --- a/NEWS +++ b/NEWS @@ -142,6 +142,8 @@ PHP NEWS - FPM: . Fixed bug #69625 (FPM returns 200 status on request without SCRIPT_FILENAME env). (Jakub Zelenka) + . Fixed bug #80024 (Duplication of info about inherited socket after pool + removing). (Jakub Zelenka) - Intl: . Fixed bug #80425 (MessageFormatAdapter::getArgTypeList redefined). (Nikita) diff --git a/sapi/fpm/fpm/fpm_sockets.c b/sapi/fpm/fpm/fpm_sockets.c index aaa16cc89e..c9c0acc7b7 100644 --- a/sapi/fpm/fpm/fpm_sockets.c +++ b/sapi/fpm/fpm/fpm_sockets.c @@ -39,6 +39,16 @@ static struct fpm_array_s sockets_list; enum { FPM_GET_USE_SOCKET = 1, FPM_STORE_SOCKET = 2, FPM_STORE_USE_SOCKET = 3 }; +static inline void fpm_sockets_get_env_name(char *envname, unsigned idx) /* {{{ */ +{ + if (!idx) { + strcpy(envname, "FPM_SOCKETS"); + } else { + sprintf(envname, "FPM_SOCKETS_%d", idx); + } +} +/* }}} */ + static void fpm_sockets_cleanup(int which, void *arg) /* {{{ */ { unsigned i; @@ -82,13 +92,11 @@ static void fpm_sockets_cleanup(int which, void *arg) /* {{{ */ if (env_value) { for (i = 0; i < socket_set_count; i++) { - if (!i) { - strcpy(envname, "FPM_SOCKETS"); - } else { - sprintf(envname, "FPM_SOCKETS_%d", i); - } + fpm_sockets_get_env_name(envname, i); setenv(envname, env_value + socket_set[i], 1); } + fpm_sockets_get_env_name(envname, socket_set_count); + unsetenv(envname); free(env_value); } @@ -373,7 +381,7 @@ int fpm_sockets_init_main() /* {{{ */ { unsigned i, lq_len; struct fpm_worker_pool_s *wp; - char sockname[32]; + char envname[32]; char sockpath[256]; char *inherited; struct listening_socket_s *ls; @@ -384,12 +392,8 @@ int fpm_sockets_init_main() /* {{{ */ /* import inherited sockets */ for (i = 0; i < FPM_ENV_SOCKET_SET_MAX; i++) { - if (!i) { - strcpy(sockname, "FPM_SOCKETS"); - } else { - sprintf(sockname, "FPM_SOCKETS_%d", i); - } - inherited = getenv(sockname); + fpm_sockets_get_env_name(envname, i); + inherited = getenv(envname); if (!inherited) { break; } diff --git a/sapi/fpm/tests/bug68391-conf-include-order.phpt b/sapi/fpm/tests/bug68391-conf-include-order.phpt index a357cf8bd3..dc22d5fea7 100644 --- a/sapi/fpm/tests/bug68391-conf-include-order.phpt +++ b/sapi/fpm/tests/bug68391-conf-include-order.phpt @@ -17,7 +17,7 @@ log_level = notice include = {{INCLUDE:CONF}} EOT; -$cfgPoolTemplate = <<start(); diff --git a/sapi/fpm/tests/bug80024-socket-reduced-inherit.phpt b/sapi/fpm/tests/bug80024-socket-reduced-inherit.phpt new file mode 100644 index 0000000000..9c05471b67 --- /dev/null +++ b/sapi/fpm/tests/bug80024-socket-reduced-inherit.phpt @@ -0,0 +1,49 @@ +--TEST-- +FPM: bug80024 - Duplication of info about inherited socket after pool removing +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +$cfg['count'] = 128; +$tester->reload($cfg); +$tester->expectLogReloadingNotices(129); +$tester->reload(); +$tester->expectLogReloadingNotices(128); +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/logtool.inc b/sapi/fpm/tests/logtool.inc index 78523635d0..caf88ce662 100644 --- a/sapi/fpm/tests/logtool.inc +++ b/sapi/fpm/tests/logtool.inc @@ -9,6 +9,12 @@ class LogTool const P_PREFIX_STDOUT = '\[pool unconfined\] child \d+ said into stdout: '; const FINAL_SUFFIX = ', pipe is closed'; + const DEBUG = 'DEBUG'; + const NOTICE = 'NOTICE'; + const WARNING = 'WARNING'; + const ERROR = 'ERROR'; + const ALERT = 'ALERT'; + /** * @var string */ @@ -278,6 +284,28 @@ class LogTool return true; } + /** + * @param array $lines + * @return bool + */ + public function expectReloadingLines(array $lines) + { + if ( + !$this->expectNotice($lines[0], 'Reloading in progress ...') || + !$this->expectNotice($lines[1], 'reloading: .*') + ) { + return false; + } + + for ($i = 2; $i < count($lines) - 2; $i++) { + if (!$this->expectNotice($lines[$i], 'using inherited socket fd=\d+, "[^"]+"')) { + return false; + } + } + + return $this->expectStartingLines(array_splice($lines, $i)); + } + /** * @param array $lines * @return bool @@ -359,7 +387,7 @@ class LogTool */ public function expectDebug(string $line, string $expectedMessage, $pool = null) { - return $this->expectEntry('DEBUG', $line, $expectedMessage, $pool); + return $this->expectEntry(self::DEBUG, $line, $expectedMessage, $pool); } /** @@ -370,7 +398,7 @@ class LogTool */ public function expectNotice(string $line, string $expectedMessage, $pool = null) { - return $this->expectEntry('NOTICE', $line, $expectedMessage, $pool); + return $this->expectEntry(self::NOTICE, $line, $expectedMessage, $pool); } /** @@ -381,7 +409,7 @@ class LogTool */ public function expectWarning(string $line, string $expectedMessage, $pool = null) { - return $this->expectEntry('WARNING', $line, $expectedMessage, $pool); + return $this->expectEntry(self::WARNING, $line, $expectedMessage, $pool); } /** @@ -392,7 +420,7 @@ class LogTool */ public function expectError(string $line, string $expectedMessage, $pool = null) { - return $this->expectEntry('ERROR', $line, $expectedMessage, $pool); + return $this->expectEntry(self::ERROR, $line, $expectedMessage, $pool); } /** @@ -403,10 +431,9 @@ class LogTool */ public function expectAlert(string $line, string $expectedMessage, $pool = null) { - return $this->expectEntry('ALERT', $line, $expectedMessage, $pool); + return $this->expectEntry(self::ALERT, $line, $expectedMessage, $pool); } - /** * @param string $msg * @return bool diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc index 5c57652d88..283ca2162c 100644 --- a/sapi/fpm/tests/tester.inc +++ b/sapi/fpm/tests/tester.inc @@ -72,7 +72,7 @@ class Tester /** * Configuration template * - * @var string + * @var string|array */ private $configTemplate; @@ -152,7 +152,14 @@ class Tester unlink($filePath); } } - // clean config files + + self::cleanConfigFiles(); + } + + /** + * Clean config files + */ + static public function cleanConfigFiles() { if (is_dir(self::CONF_DIR)) { foreach(glob(self::CONF_DIR . '/*.conf') as $name) { unlink($name); @@ -686,6 +693,12 @@ class Tester } } + if ($this->debug) { + foreach ($lines as $line) { + echo "LOG LINE: " . $line; + } + } + return $lines; } @@ -731,6 +744,24 @@ class Tester return getmygid(); } + /** + * Reload FPM by sending USR2 signal and optionally change config before that. + * + * @param string|array $configTemplate + * @return string + * @throws \Exception + */ + public function reload($configTemplate = null) + { + if (!is_null($configTemplate)) { + self::cleanConfigFiles(); + $this->configTemplate = $configTemplate; + $this->createConfig(); + } + + return $this->signal('USR2'); + } + /** * Send signal to the supplied PID or the server PID. * @@ -784,14 +815,13 @@ class Tester throw new \Exception('The config template array has to have main config'); } $mainTemplate = $configTemplates['main']; - unset($configTemplates['main']); if (!is_dir(self::CONF_DIR)) { mkdir(self::CONF_DIR); } - foreach ($configTemplates as $name => $configTemplate) { + foreach ($this->createPoolConfigs($configTemplates) as $name => $poolConfig) { $this->makeFile( 'conf', - $this->processTemplate($configTemplate), + $this->processTemplate($poolConfig), self::CONF_DIR, $name ); @@ -803,6 +833,36 @@ class Tester return $this->makeFile($extension, $this->processTemplate($mainTemplate)); } + /** + * Create pool config templates. + * + * @param array $configTemplates + * @return array + * @throws \Exception + */ + private function createPoolConfigs(array $configTemplates) + { + if (!isset($configTemplates['poolTemplate'])) { + unset($configTemplates['main']); + return $configTemplates; + } + $poolTemplate = $configTemplates['poolTemplate']; + $configs = []; + if (isset($configTemplates['count'])) { + $start = $configTemplates['start'] ?? 1; + for ($i = $start; $i < $start + $configTemplates['count']; $i++) { + $configs[$i] = str_replace('%index%', $i, $poolTemplate); + } + } elseif (isset($configTemplates['names'])) { + foreach($configTemplates['names'] as $name) { + $configs[$name] = str_replace('%name%', $name, $poolTemplate); + } + } else { + throw new \Exception('The config template requires count or names if poolTemplate set'); + } + return $configs; + } + /** * Process template string. * @@ -1110,6 +1170,16 @@ class Tester $this->logTool->checkTruncatedMessage($this->response->getErrorData()); } + /** + * Expect reloading lines to be logged. + * + * @param int $socketCount + */ + public function expectLogReloadingNotices($socketCount = 1) + { + $this->logTool->expectReloadingLines($this->getLogLines($socketCount + 4)); + } + /** * Expect starting lines to be logged. */ @@ -1176,16 +1246,36 @@ class Tester return $this->logTool->checkWrappedMessage($logLines, false, true, $is_stderr); } + /** + * Expect log entry. + * + * @param string $type The log type + * @param string $message The expected message + * @param string|null $pool The pool for pool prefixed log entry + * @param int $count The number of items + * @return bool + */ + private function expectLogEntry(string $type, string $message, $pool = null, $count = 1) + { + for ($i = 0; $i < $count; $i++) { + if (!$this->logTool->expectEntry($type, $this->getLastLogLine(), $message, $pool)) { + return false; + } + } + return true; + } + /** * Expect a log debug message. * * @param string $message * @param string|null $pool + * @param int $count * @return bool */ - public function expectLogDebug(string $message, $pool = null) + public function expectLogDebug(string $message, $pool = null, $count = 1) { - return $this->logTool->expectDebug($this->getLastLogLine(), $message, $pool); + return $this->expectLogEntry(LogTool::DEBUG, $message, $pool, $count); } /** @@ -1193,11 +1283,12 @@ class Tester * * @param string $message * @param string|null $pool + * @param int $count * @return bool */ - public function expectLogNotice(string $message, $pool = null) + public function expectLogNotice(string $message, $pool = null, $count = 1) { - return $this->logTool->expectNotice($this->getLastLogLine(), $message, $pool); + return $this->expectLogEntry(LogTool::NOTICE, $message, $pool, $count); } /** @@ -1205,11 +1296,12 @@ class Tester * * @param string $message * @param string|null $pool + * @param int $count * @return bool */ - public function expectLogWarning(string $message, $pool = null) + public function expectLogWarning(string $message, $pool = null, $count = 1) { - return $this->logTool->expectWarning($this->getLastLogLine(), $message, $pool); + return $this->expectLogEntry(LogTool::WARNING, $message, $pool, $count); } /** @@ -1217,11 +1309,12 @@ class Tester * * @param string $message * @param string|null $pool + * @param int $count * @return bool */ - public function expectLogError(string $message, $pool = null) + public function expectLogError(string $message, $pool = null, $count = 1) { - return $this->logTool->expectError($this->getLastLogLine(), $message, $pool); + return $this->expectLogEntry(LogTool::ERROR, $message, $pool, $count); } /** @@ -1229,11 +1322,12 @@ class Tester * * @param string $message * @param string|null $pool + * @param int $count * @return bool */ - public function expectLogAlert(string $message, $pool = null) + public function expectLogAlert(string $message, $pool = null, $count = 1) { - return $this->logTool->expectAlert($this->getLastLogLine(), $message, $pool); + return $this->expectLogEntry(LogTool::ALERT, $message, $pool, $count); } /** -- cgit v1.2.1 From 538f95b1b7c88949afea9fc690397309115d8d40 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sun, 21 Mar 2021 18:58:57 +0000 Subject: Fix NEWS entry position for the latest FPM fix --- NEWS | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 42eeddacdc..eef3b0302c 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,10 @@ PHP NEWS - DOM: . Fixed bug #66783 (UAF when appending DOMDocument to element). (cmb) +- FPM: + . Fixed bug #80024 (Duplication of info about inherited socket after pool + removing). (Jakub Zelenka) + - PDO_ODBC: . Fixed bug #80783 (PDO ODBC truncates BLOB records at every 256th byte). (cmb) @@ -142,8 +146,6 @@ PHP NEWS - FPM: . Fixed bug #69625 (FPM returns 200 status on request without SCRIPT_FILENAME env). (Jakub Zelenka) - . Fixed bug #80024 (Duplication of info about inherited socket after pool - removing). (Jakub Zelenka) - Intl: . Fixed bug #80425 (MessageFormatAdapter::getArgTypeList redefined). (Nikita) -- cgit v1.2.1