Skip to content

Commit

Permalink
Fix string verbatim reply to work correctly. (#315)
Browse files Browse the repository at this point in the history
Current behaviour is to append to format to the data. This is wrong because the format need to pass separately to Redis. Also, a wrong RedisModuleAPI was used, we need to use the API that allows to pass the format (otherwise Redis uses the default `txt` format).
  • Loading branch information
MeirShpilraien authored Apr 27, 2023
1 parent 5add38e commit 6c0ad12
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 16 deletions.
43 changes: 39 additions & 4 deletions src/context/call_reply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{
ptr::NonNull,
};

use crate::raw::*;
use crate::{raw::*, RedisError};

pub struct StringCallReply<'root> {
reply: NonNull<RedisModuleCallReply>,
Expand Down Expand Up @@ -531,13 +531,48 @@ pub struct VerbatimStringCallReply<'root> {
_dummy: PhantomData<&'root ()>,
}

/// RESP3 state that the verbatim string format must be of length 3.
const VERBATIM_FORMAT_LENGTH: usize = 3;
/// The string format of a verbatim string ([VerbatimStringCallReply]).
#[repr(transparent)]
#[derive(Debug, Default, Copy, Clone, PartialEq)]
pub struct VerbatimStringFormat(pub [c_char; VERBATIM_FORMAT_LENGTH]);

impl TryFrom<&str> for VerbatimStringFormat {
type Error = RedisError;

fn try_from(value: &str) -> Result<Self, Self::Error> {
if value.len() != 3 {
return Err(RedisError::String(format!(
"Verbatim format length must be {VERBATIM_FORMAT_LENGTH}."
)));
}
let mut res = VerbatimStringFormat::default();
value
.chars()
.into_iter()
.take(3)
.enumerate()
.try_for_each(|(i, c)| {
if c as u32 >= 127 {
return Err(RedisError::String(
"Verbatim format must contains only ASCI values.".to_owned(),
));
}
res.0[i] = c as c_char;
Ok(())
})?;
Ok(res)
}
}

impl<'root> VerbatimStringCallReply<'root> {
/// Return the verbatim string value of the [VerbatimStringCallReply] as a tuple.
/// The first entry represents the format, the second entry represent the data.
/// Return None if the format is not a valid utf8
pub fn to_parts(&self) -> Option<(String, Vec<u8>)> {
self.as_parts()
.map(|(format, data)| (format.to_string(), data.to_vec()))
pub fn to_parts(&self) -> Option<(VerbatimStringFormat, Vec<u8>)> {
let (format, data) = self.as_parts()?;
Some((format.try_into().ok()?, data.to_vec()))
}

/// Borrow the verbatim string value of the [VerbatimStringCallReply] as a tuple.
Expand Down
15 changes: 6 additions & 9 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,15 +425,12 @@ impl Context {
raw::reply_with_big_number(self.ctx, s.as_ptr().cast::<c_char>(), s.len())
}

Ok(RedisValue::VerbatimString((format, mut data))) => {
let mut final_data = format.as_bytes().to_vec();
final_data.append(&mut data);
raw::reply_with_verbatim_string(
self.ctx,
final_data.as_ptr().cast::<c_char>(),
final_data.len(),
)
}
Ok(RedisValue::VerbatimString((format, data))) => raw::reply_with_verbatim_string(
self.ctx,
data.as_ptr().cast(),
data.len(),
format.0.as_ptr().cast(),
),

Ok(RedisValue::BulkRedisString(s)) => raw::reply_with_string(self.ctx, s.inner),

Expand Down
3 changes: 2 additions & 1 deletion src/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,9 @@ pub fn reply_with_verbatim_string(
ctx: *mut RedisModuleCtx,
s: *const c_char,
len: size_t,
format: *const c_char,
) -> Status {
unsafe { RedisModule_ReplyWithVerbatimString.unwrap()(ctx, s, len).into() }
unsafe { RedisModule_ReplyWithVerbatimStringType.unwrap()(ctx, s, len, format).into() }
}

// Sets the expiry on a key.
Expand Down
7 changes: 5 additions & 2 deletions src/redisvalue.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::{context::call_reply::CallResult, CallReply, RedisError, RedisString};
use crate::{
context::call_reply::{CallResult, VerbatimStringFormat},
CallReply, RedisError, RedisString,
};
use std::{
collections::{HashMap, HashSet},
hash::Hash,
Expand All @@ -24,7 +27,7 @@ pub enum RedisValue {
Bool(bool),
Float(f64),
BigNumber(String),
VerbatimString((String, Vec<u8>)),
VerbatimString((VerbatimStringFormat, Vec<u8>)),
Array(Vec<RedisValue>),
StaticError(&'static str),
Map(HashMap<RedisValueKey, RedisValue>),
Expand Down

0 comments on commit 6c0ad12

Please sign in to comment.