Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Redis Server URI correct parsing #6495

Closed
wants to merge 2 commits into from

Conversation

franckstauffer
Copy link

Including password support.

Some PaaS give you full-blown URIs to configure Redis (and other servers) - up until now the URIs were not parsed due to a bad condition in normalizeServer and there was no support for URIs containing a password anyway. This should fix both.

Including password support.
@Ocramius Ocramius added this to the 2.3.2 milestone Jul 29, 2014
@Ocramius Ocramius self-assigned this Jul 29, 2014
@@ -216,6 +219,7 @@ protected function normalizeServer(&$server)
'port' => $port,
'timeout' => $timeout,
);
return $password;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the password being returned?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is returned because normalizeServer doesn't have access to the resource id / array to store the password. It is not an optimum solution but doing it a different way requires more code to be moved around as both the way the class manages the config internally and its methods don't take into account the fact the password could actually come from the URI of the server and not from a separate config entry.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@franckstauffer I'm working on it right now. I'll see if I can move it somewhere else, as I don't like this return value.

Ocramius added a commit that referenced this pull request Aug 6, 2014
…orage\Adapter\RedisResourceManager#setServer` and `#setResource`
Ocramius added a commit that referenced this pull request Aug 6, 2014
…etServer` and `#setResource` implementation
@Ocramius Ocramius closed this in c5dfcda Aug 6, 2014
Ocramius added a commit that referenced this pull request Aug 6, 2014
@Ocramius
Copy link
Member

Ocramius commented Aug 6, 2014

@franckstauffer manually merged, thanks!

gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
… features of `Zend\Cache\Storage\Adapter\RedisResourceManager#setServer` and `#setResource`
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
…pter\RedisResourceManager#setServer` and `#setResource` implementation
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants