Skip to content
This repository has been archived by the owner on May 17, 2019. It is now read-only.

Comma in Redis password causes failed connection #10

Closed
tillig opened this issue Aug 8, 2017 · 5 comments
Closed

Comma in Redis password causes failed connection #10

tillig opened this issue Aug 8, 2017 · 5 comments

Comments

@tillig
Copy link

tillig commented Aug 8, 2017

I have a related issue filed on the StackExchange.Redis repo: If you have a comma in your strong password like abcd,efgh then when you build a connection string out of it the password is interpreted incorrectly.

For example, say you have this in your Redis VCAP_SERVICES entry:

{
  "credentials": {
    "host": "myredishost",
    "ip_list": [
      "10.20.30.40"
    ],
    "password": "abcd,efgh",
    "port": 16872
  }
}

The RedisCacheConfigurer basically takes these options and builds a connection string that amounts to:

myredishost:16872,password=abcd,efgh

That is then passed to the StackExchange.Redis ConfigurationOptions class to be parsed.

StackExchange.Redis sees the efgh part of the password after the comma as a second host. Then you get failures connecting because efgh doesn't exist.

It would be better if the credentials weren't serialized to a connection string and instead manually built up a ConfigurationOptions object, mapping parsed properties into values. This would bypass the current issue where StackExchange.Redis doesn't have an escape syntax to escape the comma, plus would probably bypass other potentially unforeseen parsing issues.

@dtillman
Copy link
Contributor

@tillig

Good feedback... I'll have a look at that in the next release..

Dave

@TimHess
Copy link
Member

TimHess commented Feb 12, 2018

While not a perfect solution, this issue should be resolved as of build 2.0.0-dev-213.

While it could have been better at the time, the solution of manually building a ConfigurationOptions object does not seem realistic now that the Redis library dependencies have been removed (to align with the way the rest of the connectors are built)

@TimHess TimHess closed this as completed Feb 12, 2018
@tillig
Copy link
Author

tillig commented Feb 12, 2018

@TimHess I don't see in the unit tests that it also ensures the Parse doesn't modify the hosts - when the StackExchange bits parse that comma, it not only drops the part after the comma but adds it to the list of other Redis hosts. Did I miss that somewhere?

@TimHess
Copy link
Member

TimHess commented Feb 12, 2018

@tillig I did observe that behavior before this fix, but did not add any tests to test it becase the workaround I added sets the password to string.Empty before Parse is called (when a comma is found in the password) and plugs it back in afterwards. As far as Parse is concerned there is no comma.

@tillig
Copy link
Author

tillig commented Feb 12, 2018

OK, cool, just checkin'. That was some insidious behavior. Thanks!

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

No branches or pull requests

3 participants