Fix phpGH-11086: FPM: config test runs twice in daemonised mode

The previous check for STDERR did not work so this fixes it. Closes phpGH-13357
stixpjr · Mar 9, 2024 · a19267d · a19267d
1 parent 72197e3
commit a19267d
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 10 deletions.
diff --git a/NEWS b/NEWS
@@ -8,6 +8,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-13612 (Corrupted memory in destructor with weak references).
     (nielsdos)
 
+- FPM:
+  . Fixed GH-11086 (FPM: config test runs twice in daemonised mode).
+    (Jakub Zelenka)
+
 - Gettext:
   . Fixed sigabrt raised with dcgettext/dcngettext calls with gettext 0.22.5
     with category set to LC_ALL. (David Carlier)

diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,34 @@
+--TEST--
+FPM: gh68591 - daemonized mode duplicated logs
+--SKIPIF--
+<?php
+include "skipif.inc";
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = /dev/stderr
+daemonize = true
+[unconfined]
+listen = {{ADDR}}
+pm = static
+pm.max_children = 1
+EOT;
+
+$tester = new FPM\Tester($cfg);
+$tester->testConfig(dumpConfig: false, printOutput: true);
+
+?>
+Done
+--EXPECTF--
+%sNOTICE: configuration file %s test is successful
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git 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];