Skip to content
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

Secure connection via phpredis doesn't work #562

Closed
benburleson opened this issue Mar 11, 2020 · 5 comments · Fixed by #563
Closed

Secure connection via phpredis doesn't work #562

benburleson opened this issue Mar 11, 2020 · 5 comments · Fixed by #563
Labels
enhancement Improves existing functionality

Comments

@benburleson
Copy link
Contributor

I use PHPRedis (not Predis) and recently started upgrading our Redis instances to enable encryption in transit, but found that connections via snc wouldn't work. After some debugging, I found that snc's phpredis factory didn't care if the secure protocol rediss was used in the dsn config.

I found that changing this line https://github.com/snc/SncRedisBundle/blob/master/Factory/PhpredisClientFactory.php#L138 to the following allows secure connections via PHPRedis:

$connectParameters[] = ($parsedDsn->getTls() ? 'tls://' : '') . $parsedDsn->getHost();

After this change, I can switch between the old (not encrypted) and new (encrypted) instances just by changing the dsn config for snc_redis.

@B-Galati
Copy link
Collaborator

Thanks, could you send a PR please? With tests and doc updated, etc.

@benburleson
Copy link
Contributor Author

Hi @B-Galati , do you have any notes for contributing, specifically for running the tests? It's not obvious what I would need set up locally, if anything.

@B-Galati
Copy link
Collaborator

@benburleson You would need to run a redis instance like so: docker run --rm -it -p 6379:6379 redis:4

@curry684 curry684 added the enhancement Improves existing functionality label Mar 12, 2020
@curry684
Copy link
Collaborator

We fixed this in 2.x with #445, was it not properly forward ported to 3.x at the time?

@B-Galati
Copy link
Collaborator

We fixed this in 2.x with #445, was it not properly forward ported to 3.x at the time?

I think it was only for Predis not PHPRedis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants