From 246cc5a16c7ad381a05be2df28164a61858aefa9 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 8 Feb 2024 16:34:19 +0000 Subject: [PATCH] Fix GH-11086: FPM: config test runs twice in daemonised mode The previous check for STDERR did not work so this fixes it --- sapi/fpm/fpm/fpm_stdio.c | 11 ++++-- sapi/fpm/fpm/zlog.c | 7 ++-- sapi/fpm/fpm/zlog.h | 2 +- .../gh-11086-daemonized-logs-duplicated.phpt | 34 +++++++++++++++++++ sapi/fpm/tests/tester.inc | 18 +++++++--- 5 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 sapi/fpm/tests/gh-11086-daemonized-logs-duplicated.phpt diff --git a/sapi/fpm/fpm/fpm_stdio.c b/sapi/fpm/fpm/fpm_stdio.c index a225d3357dd99..55139a0e02ec1 100644 --- a/sapi/fpm/fpm/fpm_stdio.c +++ b/sapi/fpm/fpm/fpm_stdio.c @@ -153,7 +153,7 @@ int fpm_stdio_init_child(struct fpm_worker_pool_s *wp) /* {{{ */ close(fpm_globals.error_log_fd); } fpm_globals.error_log_fd = -1; - zlog_set_fd(-1); + zlog_set_fd(-1, 0); return 0; } @@ -374,13 +374,14 @@ int fpm_stdio_open_error_log(int reopen) /* {{{ */ php_openlog(fpm_global_config.syslog_ident, LOG_PID | LOG_CONS, fpm_global_config.syslog_facility); fpm_globals.error_log_fd = ZLOG_SYSLOG; if (fpm_use_error_log()) { - zlog_set_fd(fpm_globals.error_log_fd); + zlog_set_fd(fpm_globals.error_log_fd, 0); } return 0; } #endif fd = open(fpm_global_config.error_log, O_WRONLY | O_APPEND | O_CREAT, S_IRUSR | S_IWUSR); + if (0 > fd) { zlog(ZLOG_SYSERROR, "failed to open error_log (%s)", fpm_global_config.error_log); return -1; @@ -393,7 +394,11 @@ int fpm_stdio_open_error_log(int reopen) /* {{{ */ } else { fpm_globals.error_log_fd = fd; if (fpm_use_error_log()) { - zlog_set_fd(fpm_globals.error_log_fd); + bool is_stderr = ( + strcmp(fpm_global_config.error_log, "/dev/stderr") == 0 || + strcmp(fpm_global_config.error_log, "/proc/self/fd/2") == 0 + ); + zlog_set_fd(fpm_globals.error_log_fd, is_stderr); } } if (0 > fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC)) { diff --git a/sapi/fpm/fpm/zlog.c b/sapi/fpm/fpm/zlog.c index e5e49f8385d21..3903c6eb24178 100644 --- a/sapi/fpm/fpm/zlog.c +++ b/sapi/fpm/fpm/zlog.c @@ -25,6 +25,7 @@ #define EXTRA_SPACE_FOR_PREFIX 128 static int zlog_fd = -1; +static bool zlog_fd_is_stderr = false; static int zlog_level = ZLOG_NOTICE; static int zlog_limit = ZLOG_DEFAULT_LIMIT; static zlog_bool zlog_buffering = ZLOG_DEFAULT_BUFFERING; @@ -88,11 +89,13 @@ size_t zlog_print_time(struct timeval *tv, char *timebuf, size_t timebuf_len) /* } /* }}} */ -int zlog_set_fd(int new_fd) /* {{{ */ +int zlog_set_fd(int new_fd, zlog_bool is_stderr) /* {{{ */ { int old_fd = zlog_fd; zlog_fd = new_fd; + zlog_fd_is_stderr = is_stderr; + return old_fd; } /* }}} */ @@ -244,7 +247,7 @@ void vzlog(const char *function, int line, int flags, const char *fmt, va_list a zend_quiet_write(zlog_fd > -1 ? zlog_fd : STDERR_FILENO, buf, len); } - if (zlog_fd != STDERR_FILENO && zlog_fd != -1 && + if (!zlog_fd_is_stderr && zlog_fd != -1 && !launched && (flags & ZLOG_LEVEL_MASK) >= ZLOG_NOTICE) { zend_quiet_write(STDERR_FILENO, buf, len); } diff --git a/sapi/fpm/fpm/zlog.h b/sapi/fpm/fpm/zlog.h index 679996041b32e..be22acc32f3ca 100644 --- a/sapi/fpm/fpm/zlog.h +++ b/sapi/fpm/fpm/zlog.h @@ -17,7 +17,7 @@ typedef unsigned char zlog_bool; #define ZLOG_FALSE 0 void zlog_set_external_logger(void (*logger)(int, char *, size_t)); -int zlog_set_fd(int new_fd); +int zlog_set_fd(int new_fd, zlog_bool is_stderr); int zlog_set_level(int new_value); int zlog_set_limit(int new_value); int zlog_set_buffering(zlog_bool buffering); diff --git a/sapi/fpm/tests/gh-11086-daemonized-logs-duplicated.phpt b/sapi/fpm/tests/gh-11086-daemonized-logs-duplicated.phpt new file mode 100644 index 0000000000000..5c435aafc2598 --- /dev/null +++ b/sapi/fpm/tests/gh-11086-daemonized-logs-duplicated.phpt @@ -0,0 +1,34 @@ +--TEST-- +FPM: gh68591 - daemonized mode duplicated logs +--SKIPIF-- + +--FILE-- +testConfig(dumpConfig: false, printOutput: true); + +?> +Done +--EXPECTF-- +%sNOTICE: configuration file %s test is successful +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc index a196320353b0e..84759c4a2c85a 100644 --- a/sapi/fpm/tests/tester.inc +++ b/sapi/fpm/tests/tester.inc @@ -440,12 +440,22 @@ class Tester * @return null|array * @throws \Exception */ - public function testConfig($silent = false, array|string|null $expectedPattern = null): ?array - { - $configFile = $this->createConfig(); - $cmd = self::findExecutable() . ' -n -tt -y ' . $configFile . ' 2>&1'; + public function testConfig( + $silent = false, + array|string|null $expectedPattern = null, + $dumpConfig = true, + $printOutput = false + ): ?array { + $configFile = $this->createConfig(); + $configTestArg = $dumpConfig ? '-tt' : '-t'; + $cmd = self::findExecutable() . " -n $configTestArg -y $configFile 2>&1"; $this->trace('Testing config using command', $cmd, true); exec($cmd, $output, $code); + if ($printOutput) { + foreach ($output as $outputLine) { + echo $outputLine . "\n"; + } + } $found = 0; if ($expectedPattern !== null) { $expectedPatterns = is_array($expectedPattern) ? $expectedPattern : [$expectedPattern];