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 crash when logging NULL chars. #1093

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

MeirShpilraien
Copy link
Collaborator

When log NULL char, the server crashed on panic on redismodule-rs when trying to convert the &str to CString:
https://github.com/RedisLabsModules/redismodule-rs/blob/bf2b3a6a04b795ad1d58d5252ae22d25430c3349/src/logging.rs#L55

When we create our logs we can be sure we do not have embedded NULLs inside the log message, but when the log message comes from user script we can not be sure. To solve it we escape the string using escape_default which will escape all chars according to the following rules:

  • Tab is escaped as \t.
  • Carriage return is escaped as \r.
  • Line feed is escaped as \n.
  • Single quote is escaped as \'.
  • Double quote is escaped as \".
  • Backslash is escaped as \\.
  • Any character in the 'printable ASCII' range 0x20 .. 0x7e inclusive is not escaped.
  • All other characters are given hexadecimal Unicode escapes; see

@MeirShpilraien MeirShpilraien requested a review from iddm March 7, 2024 10:12
@MeirShpilraien MeirShpilraien added the cherrypick_2.0 PR should be cherrypick to 2.0 branch label Mar 7, 2024
@MeirShpilraien MeirShpilraien merged commit 5d6f4dd into master Mar 8, 2024
20 checks passed
@MeirShpilraien MeirShpilraien deleted the fix_crash_when_logging_null_char branch March 8, 2024 13:26
MeirShpilraien added a commit that referenced this pull request Mar 18, 2024
* Fix crash when logging NULL chars.

* Increase MSRV to 1.74

(cherry picked from commit 5d6f4dd)
MeirShpilraien added a commit that referenced this pull request Mar 19, 2024
* Fix crash when logging NULL chars.

* Increase MSRV to 1.74

(cherry picked from commit 5d6f4dd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick_2.0 PR should be cherrypick to 2.0 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants