-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] Fixed password used to make the redis connection. #20026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Redis doesn't have user:password concepts, it only has a single "secret" one, see http://redis.io/commands/AUTH . What if this secret contains a ":"? What are you fixing that doesn't work today? |
I'm using Heroku and there Redis add on. The url provided in the connection settings looks like this : As Heroku provide such url it would be nice to support it or report a tricks in the Symfony documentation. As an exemple, a trick already exist in snc/SncRedisBundle RedisDsn.php for this case. |
@@ -72,7 +72,7 @@ public static function createConnection($dsn, array $options = array()) | |||
} | |||
$params = preg_replace_callback('#^redis://(?:([^@]*)@)?#', function ($m) use (&$auth) { | |||
if (isset($m[1])) { | |||
$auth = $m[1]; | |||
$auth = preg_replace('/^(\w+:)?(\w+)$/', '$2', $m[1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of calling preg_replace in preg_replace_callback, I propose to change the previous regexp for:
'#^redis://(?:(?:[^:@]*+:)?([^@]*+)@)?#'
it should do the same, can you confirm?
OK, understood. There is a small potential for bc break if one uses a secret with a |
Thank you @nicolas-grekas , it works with |
👍 |
👍 |
Thank you @ErikSaunier. |
…(ErikSaunier) This PR was merged into the 3.1 branch. Discussion ---------- [Cache] Fixed password used to make the redis connection. | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20025 | License | MIT | Doc PR | http://symfony.com/blog/new-in-symfony-3-1-cache-component I do not know if it's possible to provide a test as `REDIS_HOST` is provided by Travis in [RedisAdapterTest.php](https://github.com/symfony/symfony/blob/3.1/src/Symfony/Component/Cache/Tests/Adapter/RedisAdapterTest.php). Commits ------- 77eea43 [Cache] Fix password used to make the redis connection.
I do not know if it's possible to provide a test as
REDIS_HOST
is provided by Travis in RedisAdapterTest.php.