Skip to content

Commit 08b5777

Browse files
JoshuaBehrensbukka
authored andcommitted
Warn when fpm socket was not registered on the expected path
This might happen if the UDS length limit is exceeded. Co-authored-by: Jakub Zelenka <bukka@php.net> Closes GH-11066
1 parent 72e2e25 commit 08b5777

8 files changed

+216
-9
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ PHP NEWS
2222
. Added DOMElement::className and DOMElement::id. (nielsdos)
2323
. Added DOMParentNode::replaceChildren(). (nielsdos)
2424

25+
- FPM:
26+
. Added warning to log when fpm socket was not registered on the expected
27+
path. (Joshua Behrens, Jakub Zelenka)
28+
2529
- Intl:
2630
. Fix memory leak in MessageFormatter::format() on failure. (Girgias)
2731

UPGRADING

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,9 @@ PHP 8.3 UPGRADE NOTES
429429
current system user. Previously, calling FFI::load() was not possible during
430430
preloading if the opcache.preload_user directive was set.
431431

432+
- FPM:
433+
. FPM CLI test now fails if the socket path is longer than supported by OS.
434+
432435
- Opcache:
433436
. In the cli and phpdbg SAPIs, preloading does not require the
434437
opcache.preload_user directive to be set anymore when running as root. In

sapi/fpm/fpm/fpm_sockets.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,9 +396,25 @@ static int fpm_socket_af_inet_listening_socket(struct fpm_worker_pool_s *wp) /*
396396
static int fpm_socket_af_unix_listening_socket(struct fpm_worker_pool_s *wp) /* {{{ */
397397
{
398398
struct sockaddr_un sa_un;
399+
size_t socket_length = sizeof(sa_un.sun_path);
400+
size_t address_length = strlen(wp->config->listen_address);
399401

400402
memset(&sa_un, 0, sizeof(sa_un));
401-
strlcpy(sa_un.sun_path, wp->config->listen_address, sizeof(sa_un.sun_path));
403+
strlcpy(sa_un.sun_path, wp->config->listen_address, socket_length);
404+
405+
if (address_length >= socket_length) {
406+
zlog(
407+
ZLOG_WARNING,
408+
"[pool %s] cannot bind to UNIX socket '%s' as path is too long (found length: %zu, "
409+
"maximal length: %zu), trying cut socket path instead '%s'",
410+
wp->config->name,
411+
wp->config->listen_address,
412+
address_length,
413+
socket_length,
414+
sa_un.sun_path
415+
);
416+
}
417+
402418
sa_un.sun_family = AF_UNIX;
403419
return fpm_sockets_get_listening_socket(wp, (struct sockaddr *) &sa_un, sizeof(struct sockaddr_un));
404420
}

sapi/fpm/fpm/fpm_unix.c

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <stdlib.h>
99
#include <unistd.h>
1010
#include <sys/types.h>
11+
#include <sys/un.h>
1112
#include <pwd.h>
1213
#include <grp.h>
1314

@@ -63,6 +64,33 @@ static struct passwd *fpm_unix_get_passwd(struct fpm_worker_pool_s *wp, const ch
6364
return pwd;
6465
}
6566

67+
static inline bool fpm_unix_check_listen_address(struct fpm_worker_pool_s *wp, const char *address, int flags)
68+
{
69+
if (wp->listen_address_domain != FPM_AF_UNIX) {
70+
return true;
71+
}
72+
73+
struct sockaddr_un test_socket;
74+
size_t address_length = strlen(address);
75+
size_t socket_length = sizeof(test_socket.sun_path);
76+
77+
if (address_length < socket_length) {
78+
return true;
79+
}
80+
81+
zlog(
82+
flags,
83+
"[pool %s] cannot bind to UNIX socket '%s' as path is too long (found length: %zu, "
84+
"maximal length: %zu)",
85+
wp->config->name,
86+
address,
87+
address_length,
88+
socket_length
89+
);
90+
91+
return false;
92+
}
93+
6694
static inline bool fpm_unix_check_passwd(struct fpm_worker_pool_s *wp, const char *name, int flags)
6795
{
6896
return !name || fpm_unix_is_id(name) || fpm_unix_get_passwd(wp, name, flags);
@@ -90,6 +118,7 @@ bool fpm_unix_test_config(struct fpm_worker_pool_s *wp)
90118
return (
91119
fpm_unix_check_passwd(wp, config->user, ZLOG_ERROR) &&
92120
fpm_unix_check_group(wp, config->group, ZLOG_ERROR) &&
121+
fpm_unix_check_listen_address(wp, config->listen_address, ZLOG_SYSERROR) &&
93122
fpm_unix_check_passwd(wp, config->listen_owner, ZLOG_SYSERROR) &&
94123
fpm_unix_check_group(wp, config->listen_group, ZLOG_SYSERROR)
95124
);
@@ -273,7 +302,7 @@ int fpm_unix_set_socket_permissions(struct fpm_worker_pool_s *wp, const char *pa
273302
/* Copy the new ACL entry from config */
274303
for (i=ACL_FIRST_ENTRY ; acl_get_entry(aclconf, i, &entryconf) ; i=ACL_NEXT_ENTRY) {
275304
if (0 > acl_create_entry (&aclfile, &entryfile) ||
276-
0 > acl_copy_entry(entryfile, entryconf)) {
305+
0 > acl_copy_entry(entryfile, entryconf)) {
277306
zlog(ZLOG_SYSERROR, "[pool %s] failed to add entry to the ACL of the socket '%s'", wp->config->name, path);
278307
acl_free(aclfile);
279308
return -1;

sapi/fpm/tests/logreader.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class LogReader
1616
*
1717
* @var string|null
1818
*/
19-
private ?string $currentSourceName;
19+
private ?string $currentSourceName = null;
2020

2121
/**
2222
* Log descriptors.
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
--TEST--
2+
FPM: UNIX socket filename is too for start
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc"; ?>
6+
--FILE--
7+
<?php
8+
9+
require_once "tester.inc";
10+
$socketFilePrefix = __DIR__ . '/socket-file';
11+
$socketFile = sprintf(
12+
"%s-fpm-unix-socket-too-long-filename-but-starts-anyway%s.sock",
13+
$socketFilePrefix,
14+
str_repeat('-0000', 11)
15+
);
16+
17+
$cfg = <<<EOT
18+
[global]
19+
error_log = {{FILE:LOG}}
20+
21+
[fpm_pool]
22+
listen = $socketFile
23+
pm = static
24+
pm.max_children = 1
25+
catch_workers_output = yes
26+
EOT;
27+
28+
$tester = new FPM\Tester($cfg);
29+
$tester->start();
30+
$tester->expectLogStartNotices();
31+
$tester->expectLogPattern(
32+
sprintf(
33+
'/\[pool fpm_pool\] cannot bind to UNIX socket \'%s\' as path is too long '
34+
. '\(found length: %d, maximal length: \d+\), trying cut socket path instead \'.+\'/',
35+
preg_quote($socketFile, '/'),
36+
strlen($socketFile)
37+
),
38+
true
39+
);
40+
41+
$files = glob($socketFilePrefix . '*');
42+
43+
if ($files === []) {
44+
echo 'Socket files were not found.' . PHP_EOL;
45+
}
46+
47+
if ($socketFile === $files[0]) {
48+
// this means the socket file path length is not an issue (anymore). Might be not long enough
49+
echo 'Socket file is the same as configured.' . PHP_EOL;
50+
}
51+
52+
$tester->terminate();
53+
$tester->expectLogTerminatingNotices();
54+
$tester->close();
55+
?>
56+
Done
57+
--EXPECT--
58+
Done
59+
--CLEAN--
60+
<?php
61+
require_once "tester.inc";
62+
FPM\Tester::clean();
63+
64+
// cleanup socket file if php-fpm was not killed
65+
$socketFile = sprintf(
66+
"/socket-file-fpm-unix-socket-too-long-filename-but-starts-anyway%s.sock",
67+
__DIR__,
68+
str_repeat('-0000', 11)
69+
);
70+
71+
if (is_file($socketFile)) {
72+
unlink($socketFile);
73+
}
74+
?>
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
FPM: UNIX socket filename is too for test
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc"; ?>
6+
--FILE--
7+
<?php
8+
9+
require_once "tester.inc";
10+
11+
$socketFilePrefix = __DIR__ . '/socket-file';
12+
$socketFile = sprintf(
13+
"/socket-file-fpm-unix-socket-too-long-filename-but-starts-anyway%s.sock",
14+
__DIR__,
15+
str_repeat('-0000', 11)
16+
);
17+
18+
$cfg = <<<EOT
19+
[global]
20+
error_log = {{FILE:LOG}}
21+
22+
[fpm_pool]
23+
listen = $socketFile
24+
pm = static
25+
pm.max_children = 1
26+
catch_workers_output = yes
27+
EOT;
28+
29+
$tester = new FPM\Tester($cfg);
30+
$tester->testConfig(true, [
31+
sprintf(
32+
'/cannot bind to UNIX socket \'%s\' as path is too long '
33+
. '\(found length: %d, maximal length: \d+\)/',
34+
preg_quote($socketFile, '/'),
35+
strlen($socketFile)
36+
),
37+
'/FPM initialization failed/',
38+
]);
39+
?>
40+
Done
41+
--EXPECT--
42+
Done
43+
--CLEAN--
44+
<?php
45+
require_once "tester.inc";
46+
FPM\Tester::clean();
47+
?>

sapi/fpm/tests/tester.inc

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,26 +382,50 @@ class Tester
382382
* @return null|array
383383
* @throws \Exception
384384
*/
385-
public function testConfig($silent = false)
385+
public function testConfig($silent = false, array|string|null $expectedPattern = null): ?array
386386
{
387387
$configFile = $this->createConfig();
388388
$cmd = self::findExecutable() . ' -n -tt -y ' . $configFile . ' 2>&1';
389389
$this->trace('Testing config using command', $cmd, true);
390390
exec($cmd, $output, $code);
391+
$found = 0;
392+
if ($expectedPattern !== null) {
393+
$expectedPatterns = is_array($expectedPattern) ? $expectedPattern : [$expectedPattern];
394+
}
391395
if ($code) {
392396
$messages = [];
393397
foreach ($output as $outputLine) {
394398
$message = preg_replace("/\[.+?\]/", "", $outputLine, 1);
399+
if ($expectedPattern !== null) {
400+
for ($i = 0; $i < count($expectedPatterns); $i++) {
401+
$pattern = $expectedPatterns[$i];
402+
if ($pattern !== null && preg_match($pattern, $message)) {
403+
$found++;
404+
$expectedPatterns[$i] = null;
405+
}
406+
}
407+
}
395408
$messages[] = $message;
396409
if ( ! $silent) {
397410
$this->error($message, null, false);
398411
}
399412
}
413+
} else {
414+
$messages = null;
415+
}
400416

401-
return $messages;
417+
if ($expectedPattern !== null && $found < count($expectedPatterns)) {
418+
$missingPatterns = array_filter($expectedPatterns);
419+
$errorMessage = sprintf(
420+
"The expected config %s %s %s not been found",
421+
count($missingPatterns) > 1 ? 'patterns' : 'pattern',
422+
implode(', ', $missingPatterns),
423+
count($missingPatterns) > 1 ? 'have' : 'has',
424+
);
425+
$this->error($errorMessage);
402426
}
403427

404-
return null;
428+
return $messages;
405429
}
406430

407431
/**
@@ -1155,9 +1179,19 @@ class Tester
11551179
return $address;
11561180
}
11571181

1158-
return sys_get_temp_dir() . '/' .
1159-
hash('crc32', dirname($address)) . '-' .
1160-
basename($address);
1182+
$addressPart = hash('crc32', dirname($address)) . '-' . basename($address);
1183+
1184+
// is longer on Mac, than on Linux
1185+
$tmpDirAddress = sys_get_temp_dir() . '/' . $addressPart;
1186+
;
1187+
1188+
if (strlen($tmpDirAddress) <= 104) {
1189+
return $tmpDirAddress;
1190+
}
1191+
1192+
$srcRootAddress = dirname(__DIR__, 3) . '/' . $addressPart;
1193+
1194+
return $srcRootAddress;
11611195
}
11621196

11631197
return $this->getHost($type) . ':' . $port;

0 commit comments

Comments
 (0)