Fix Redis unix socket connections in SymfonyAdapterProvider (silent filesystem fallback)#41001
Fix Redis unix socket connections in SymfonyAdapterProvider (silent filesystem fallback)#41001rhoerr wants to merge 1 commit into
Conversation
The Symfony cache adapter builds Redis DSNs as redis://host:port/db. When env.php configures a unix socket (server => /path/redis.sock, port => 0), this produces redis:///path/redis.sock:0/1 - Symfony's RedisTrait strips the trailing /1 as the dbindex and resolves the socket file as 'redis.sock:0', which does not exist. The connection failure is swallowed by createAdapter()'s catch-all, so cache and FPC silently fall back to the filesystem adapter with no logging. Treat a host beginning with '/' as a unix socket path - the same convention phpredis and Credis (the previous Zend-based Redis backend) use - and emit Symfony's socket DSN form: redis://[auth@]/path/to/redis.sock/<dbindex> Apply the same handling to the Predis fallback connection parameters (scheme unix + path instead of tcp + host/port). Passwords are now rawurlencode()d instead of urlencode()d, matching the rawurldecode() Symfony applies when parsing DSN userinfo. DSN construction and Predis parameter building are extracted into private builder methods with unit test coverage for TCP and socket forms, with and without auth. Signed-off-by: Ryan Hoerr <rhoerr@gmail.com>
|
Hi @rhoerr. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
1 similar comment
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
Description (*)
Configuring the Redis cache backend with a unix socket in
env.php('backend' => 'redis','server' => '/path/to/redis.sock','port' => '0') silently falls back to the filesystem cache adapter — cache and FPC quietly stop using Redis with no error and no log entry.Root cause:
SymfonyAdapterProvider::createPhpRedisConnection()builds the DSN asredis://host:port/dbunconditionally:With a socket config this yields
redis:///path/to/redis.sock:0/1. Symfony'sRedisTrait::createConnection()strips the trailing/1as the dbindex and resolves the unix socket file as/path/to/redis.sock:0, which does not exist. The connection failure is swallowed bycreateAdapter()'s catch-all\Exceptionhandler, which falls back tocreateFilesystemAdapter()without logging. The Predis fallback path (createOptimizedPredisConnection()) has the same gap — it always buildsscheme => tcpparameters.Fix: treat a host beginning with
/as a unix socket path — the same convention used by phpredis'sconnect()and by Credis behind the previousMagento\Framework\Cache\Backend\Redis, so socket configurations that worked before the Symfony cache migration work again unchanged — and emit Symfony's documented socket DSN formredis://[auth@]/path/to/redis.sock/<dbindex>. The Predis parameter builder likewise emitsscheme => unix, path => ...for socket hosts. A hostname can never begin with/(DNS labels are letters/digits/hyphens), so the check cannot misroute TCP configurations.Minor related correction: the password is now
rawurlencode()d rather thanurlencode()d, matching therawurldecode()Symfony applies when parsing DSN userinfo (differs only for passwords containing spaces).DSN construction and Predis connection parameters are extracted into small private builder methods (
buildRedisBaseDsn(),buildPredisConnectionParameters()) so they are unit-testable; behavior is otherwise unchanged.Related Pull Requests
Same fix submitted to Mage-OS: mage-os/mageos-magento2#294
Fixed Issues (if relevant)
N/A (root cause analysis included above)
Manual testing scenarios (*)
app/etc/env.phpfordefaultandpage_cache:redis-cli -s /var/run/redis/redis.sock -n 1 dbsizestays at 0 andvar/cache/fills with filesystem cache entries.var/cache/is no longer populated by the cache frontend.'server' => '127.0.0.1', 'port' => '6379'), with and without password, behave as before.New unit test
Magento\Framework\Cache\Test\Unit\Frontend\Adapter\SymfonyAdapterProviderTestcovers TCP and socket DSN/parameter building, with/without auth, password encoding of reserved characters, and port being ignored for sockets (11 tests, 11 assertions, PHPUnit 12).Questions or comments
The catch-all fallback in
createAdapter()that swallows connection errors without logging turns any Redis misconfiguration into an invisible performance regression; a follow-up adding at least a log line may be worthwhile.Contribution checklist (*)
Also submitted to Mage-OS 3: mage-os/mageos-magento2#294