Skip to content

Preserve UTF-8 when sanitizing RESP strings - #243

Open
stevenzengg wants to merge 2 commits into
valkey-io:mainfrom
stevenzengg:fix/preserve-utf8-resp-strings
Open

Preserve UTF-8 when sanitizing RESP strings#243
stevenzengg wants to merge 2 commits into
valkey-io:mainfrom
stevenzengg:fix/preserve-utf8-resp-strings

Conversation

@stevenzengg

@stevenzengg stevenzengg commented Aug 25, 2026

Copy link
Copy Markdown

Summary

Context::str_as_legal_resp_string previously converted every Rust char with char as u8, truncating Unicode scalar values instead of preserving their UTF-8 encoding.

For example:

  • é should be encoded as C3 A9, but the previous code produced only E9, so strict UTF-8 clients reject the response.
  • 😀 was truncated to 00, which could make CString::new(...).unwrap() panic because it creates an interior NUL.

This change replaces only CR, LF, and NUL with ASCII spaces while preserving every other character's UTF-8 encoding.

The shared helper affects:

  • simple-string replies
  • error replies
  • raw error replies
  • ACL category names

Test plan

Passed locally:

  • cargo test legal_resp_string_ — 4 passed
  • cargo fmt -- --check
  • cargo build --all --all-targets --no-default-features
  • cargo build --all --all-targets --no-default-features --release
  • cargo test --doc --workspace --no-default-features --release — 3 passed, 3 ignored
  • cargo deny check licenses
  • cargo deny check bans

Workspace suite:

  • 167 unit tests passed
  • 49 of 50 integration tests passed
  • test_valkey_value_derive crashes the locally installed Valkey 9.1.0 in getKeysUsingLegacyRangeSpec; the identical failure reproduces on untouched main, so it is not introduced by this change.

Signed-off-by: Steven Zeng <szeng2@atlassian.com>
@stevenzengg
stevenzengg force-pushed the fix/preserve-utf8-resp-strings branch from 55c33b6 to 426151c Compare August 25, 2026 06:37
Signed-off-by: Steven Zeng <szeng2@atlassian.com>
@stevenzengg

Copy link
Copy Markdown
Author

@dmitrypol, could you review this small UTF-8 correctness fix when you have a chance? It prevents invalid RESP output (and a possible interior-NUL panic) in the shared string-sanitization helper, with focused Unicode and forbidden-character coverage.

I don't have permission to assign reviewers or labels on this repository. Could a maintainer also add the bug label so the PR appears under Bug Fixes in the generated release notes?

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

Successfully merging this pull request may close these issues.

2 participants