Skip to content

Commit 3c239f4

Browse files
committed
Merge branch '5.4' into 6.4
* 5.4: explicitly mark nullable parameters as nullable fix low deps tests [HttpKernel] Fix datacollector caster for reference object property bug #51578 [Cache] always select database for persistent redis connections [Security] Validate that CSRF token in form login is string similar to username/password [validator] validated Dutch translation Improve dutch translations [Translation] Skip state=needs-translation entries only when source == target [HttpKernel] Ensure controllers are not lazy [Validator] Fill in trans-unit id 113: This URL does not contain a TLD. [Validator] added missing Polish translation for unit 113 [Validator] add missing lv translation [HttpClient] Let curl handle transfer encoding [Messenger] Make Doctrine connection ignore unrelated tables on setup [HttpFoundation] Set content-type header in RedirectResponse add translations for the requireTld constraint option message [Serializer] Fix unexpected allowed attributes [FrameworkBundle] Fix registration of the bundle path to translation
2 parents b59bbf9 + 7807aa0 commit 3c239f4

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

Tests/Traits/RedisTraitTest.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,57 @@ public static function provideCreateConnection(): array
7373
],
7474
];
7575
}
76+
77+
/**
78+
* Due to a bug in phpredis, the persistent connection will keep its last selected database. So when re-using
79+
* a persistent connection, the database has to be re-selected, too.
80+
*
81+
* @see https://github.com/phpredis/phpredis/issues/1920
82+
*
83+
* @group integration
84+
*/
85+
public function testPconnectSelectsCorrectDatabase()
86+
{
87+
if (!class_exists(\Redis::class)) {
88+
throw new SkippedTestSuiteError('The "Redis" class is required.');
89+
}
90+
if (!getenv('REDIS_HOST')) {
91+
throw new SkippedTestSuiteError('REDIS_HOST env var is not defined.');
92+
}
93+
if (!\ini_get('redis.pconnect.pooling_enabled')) {
94+
throw new SkippedTestSuiteError('The bug only occurs when pooling is enabled.');
95+
}
96+
97+
// Limit the connection pool size to 1:
98+
if (false === $prevPoolSize = ini_set('redis.pconnect.connection_limit', 1)) {
99+
throw new SkippedTestSuiteError('Unable to set pool size');
100+
}
101+
102+
try {
103+
$mock = self::getObjectForTrait(RedisTrait::class);
104+
105+
$dsn = 'redis://'.getenv('REDIS_HOST');
106+
107+
$cacheKey = 'testPconnectSelectsCorrectDatabase';
108+
$cacheValueOnDb1 = 'I should only be on database 1';
109+
110+
// First connect to database 1 and set a value there so we can identify this database:
111+
$db1 = $mock::createConnection($dsn, ['dbindex' => 1, 'persistent' => 1]);
112+
self::assertInstanceOf(\Redis::class, $db1);
113+
self::assertSame(1, $db1->getDbNum());
114+
$db1->set($cacheKey, $cacheValueOnDb1);
115+
self::assertSame($cacheValueOnDb1, $db1->get($cacheKey));
116+
117+
// Unset the connection - do not use `close()` or we will lose the persistent connection:
118+
unset($db1);
119+
120+
// Now connect to database 0 and see that we do not actually ended up on database 1 by checking the value:
121+
$db0 = $mock::createConnection($dsn, ['dbindex' => 0, 'persistent' => 1]);
122+
self::assertInstanceOf(\Redis::class, $db0);
123+
self::assertSame(0, $db0->getDbNum()); // Redis is lying here! We could actually be on any database!
124+
self::assertNotSame($cacheValueOnDb1, $db0->get($cacheKey)); // This value should not exist if we are actually on db 0
125+
} finally {
126+
ini_set('redis.pconnect.connection_limit', $prevPoolSize);
127+
}
128+
}
76129
}

Traits/RedisTrait.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,10 @@ public static function createConnection(#[\SensitiveParameter] string $dsn, arra
293293
}
294294

295295
if ((null !== $auth && !$redis->auth($auth))
296-
|| ($params['dbindex'] && !$redis->select($params['dbindex']))
296+
// Due to a bug in phpredis we must always select the dbindex if persistent pooling is enabled
297+
// @see https://github.com/phpredis/phpredis/issues/1920
298+
// @see https://github.com/symfony/symfony/issues/51578
299+
|| (($params['dbindex'] || ('pconnect' === $connect && '0' !== \ini_get('redis.pconnect.pooling_enabled'))) && !$redis->select($params['dbindex']))
297300
) {
298301
$e = preg_replace('/^ERR /', '', $redis->getLastError());
299302
throw new InvalidArgumentException('Redis connection failed: '.$e.'.');

0 commit comments

Comments
 (0)