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

Fix _readyCheck INFO parser's handling of colon characters #1127



Copy link

@Rua-Yuki Rua-Yuki commented May 8, 2020

This super simple PR fixes #1126, an issue which occurs when encountering INFO response field values containing the colon character.

The purpose of this patch is to have the exposed serverInfo client property fully conformant with potential unexpected values Redis can respond with.

I understand that the serverInfo property is not documented, though it has apparently come into direct use by external libraries. I'm curious what the official stance on this is, and if this should be considered a valid practice or rather a misstep.

This commit fixes an issue which would occur when encountering INFO
response field values containing the colon character.

Because the parser logic splits entire lines upon colon character
delimiters and took only the first component as the field name, and
second component as the field value, any line containing multiple
colons would ultimately lead to an incorrectly truncated field value.

For example:

Would be split into:
  ["config_file", "Y", "\\redis\\redis.conf"]

Leading to a field name of "config_file" and an incorrect field value
of "Y".

The resolution is simply to handle additional field value components as
needed, joining them together appropriately.
@luin luin merged commit 38a09e1 into luin:master May 30, 2020
1 check passed
ioredis-robot pushed a commit that referenced this pull request May 30, 2020
## [4.17.2](v4.17.1...v4.17.2) (2020-05-30)

### Bug Fixes

* _readyCheck INFO parser's handling of colon characters ([#1127](#1127)) ([38a09e1](38a09e1))
Copy link

ioredis-robot commented May 30, 2020

🎉 This PR is included in version 4.17.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants