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

Possibility to retrieve back all settings set to Redis class #320

Closed
pdeszynski opened this issue Mar 25, 2013 · 7 comments
Closed

Possibility to retrieve back all settings set to Redis class #320

pdeszynski opened this issue Mar 25, 2013 · 7 comments

Comments

@pdeszynski
Copy link

Hello,

It would be great if there would be a possibility to get all connection details to phpredis back, like host, port, database number, password etc.

I would like to integrate this library to ZF2 as new Storage (here's a PR zendframework/zendframework#3709). The problem is that wrapper allows to set already initialized Resource. In that case I'm not able to reestabilish connection if something happens to Redis or there was called close() function on oryginal reference.

Using the occasion, is there any possibility to detect if connect function was already called on phpredis object other than calling ping() or checking if socket attribute exists?

@michael-grunder
Copy link
Member

I don't see any reason not to make this information available to the user. I'm trying to work through in my head if there is any security problem with exposing the password as well. Given that you would have set the password yourself when connecting, it seems like it's ok.

@nicolasff any drawbacks to this that you can see?

@nicolasff
Copy link
Member

@michael-grunder agreed, this should be fine.

@michael-grunder
Copy link
Member

Cool. So I think I'll expose the following as read only properties to the phpredis object:

connected:  boolean
host: string
port: integer
dbnum: integer

I think we make retrieving the password a function. This is so if a user were to var_dump the phpredis object (to a log file or something), it wouldn't embed the password there.

@piteer1 Will this work for you
@nicolasff Same question. :)

I'll get this sorted over the next couple of days

Cheers!
Mike

@pdeszynski
Copy link
Author

@michael-grunder @nicolasff Looks great! Thanks for fast answer!

Would it be possible to add also into this list persistent_id and timeout properties? This should cover all my needs.

michael-grunder added a commit that referenced this issue Mar 27, 2013
This commit adds methods to get information about the state
of our phpredis object, such as what host/port we are connected
to, our timeout, etc...

The following methods have been added:

getHost()
getPort()
getDBNum()
getTimeout()
getReadTimeout()
isConnected()
getPersistentID()
getAuth()

In addition, there is a small memory leak fix when a persistent id
was specifically passed to connect() (it wasn't beeing freed).

Addresses issue #320
@michael-grunder
Copy link
Member

Hey guys,

I've added several methods that can provide the introspection that I think is required. Presently they will all return FALSE if we aren't connected to redis (rather than, for example, holding the last information used in a connect call). What this means, is that if you were to connect to a server and then call $redis->close(), the methods will just return FALSE. Please let me know if this will be an issue.

The getAuth() method will return the password that was used to authenticate the connection, but this doesn't verify that the authentication was correct, so that would be up to the caller to determine.

@nicolasff A couple of things. Given that we now store the last used password, we could in theory re-authenticate in the event a reconnect is required. I'm not sure if we should do that or not. Also, I feel paranoid about keeping the password in memory like this. I feel like it's OK, but perhaps we should make this configurable (maybe compile time option?)

@piteer1 Let me know how they work for you.

Cheers,
Mike

@pdeszynski
Copy link
Author

Hello,

@michael-grunder Yeah, it looks great. Thank you!

As for the password I think it shouldn't be visible by doing var_dump on phpredis object (as it's not available in PDO for example). I think it would be also ok even if it won't be possible to retrieve back password but connect will just use last set password if there are security concerns.

Best regards

@michael-grunder
Copy link
Member

Yeah, you won't be able to see the password with a var_dump(). I think we'll mull it around for a while and see if this introduces any security issues.

Cheers!
Mike

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

No branches or pull requests

3 participants