Skip to content

Commit f611a96

Browse files
ephraimfeldblumephraimfeldblum
authored
fix get_memory to prevent crash on CRDT (#1313)
1 parent c15c0f0 commit f611a96

File tree

4 files changed

+44
-62
lines changed

4 files changed

+44
-62
lines changed

‎redis_json/src/commands.rs

+4-6
Original file line numberDiff line numberDiff line change
@@ -1800,19 +1800,17 @@ pub fn json_debug<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>)
18001800
let key = manager.open_key_read(ctx, &key)?;
18011801
if path.is_legacy() {
18021802
Ok(match key.get_value()? {
1803-
Some(doc) => {
1804-
manager.get_memory(KeyValue::new(doc).get_first(path.get_path())?)?
1805-
}
1803+
Some(doc) => M::get_memory(KeyValue::new(doc).get_first(path.get_path())?)?,
18061804
None => 0,
18071805
}
18081806
.into())
18091807
} else {
18101808
Ok(match key.get_value()? {
18111809
Some(doc) => KeyValue::new(doc)
18121810
.get_values(path.get_path())?
1813-
.iter()
1814-
.map(|v| manager.get_memory(v).unwrap())
1815-
.collect(),
1811+
.into_iter()
1812+
.map(M::get_memory)
1813+
.try_collect()?,
18161814
None => vec![],
18171815
}
18181816
.into())

‎redis_json/src/ivalue_manager.rs

+38-51
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::redisjson::normalize_arr_start_index;
1111
use crate::Format;
1212
use crate::REDIS_JSON_TYPE;
1313
use bson::{from_document, Document};
14-
use ijson::{DestructuredMut, INumber, IObject, IString, IValue, ValueType};
14+
use ijson::{DestructuredMut, DestructuredRef, INumber, IObject, IString, IValue};
1515
use json_path::select_value::{SelectValue, SelectValueType};
1616
use redis_module::key::{verify_type, KeyFlags, RedisKey, RedisKeyWritable};
1717
use redis_module::raw::{RedisModuleKey, Status};
@@ -451,55 +451,45 @@ impl<'a> Manager for RedisIValueJsonKeyManager<'a> {
451451
///
452452
/// following https://github.com/Diggsey/ijson/issues/23#issuecomment-1377270111
453453
///
454-
fn get_memory(&self, v: &Self::V) -> Result<usize, RedisError> {
455-
let res = size_of::<IValue>()
456-
+ match v.type_() {
457-
ValueType::Null | ValueType::Bool => 0,
458-
ValueType::Number => {
459-
let num = v.as_number().unwrap();
460-
if num.has_decimal_point() {
461-
// 64bit float
462-
16
463-
} else if num >= &INumber::from(-128) && num <= &INumber::from(383) {
464-
// 8bit
465-
0
466-
} else if num > &INumber::from(-8_388_608) && num <= &INumber::from(8_388_607) {
467-
// 24bit
468-
4
469-
} else {
470-
// 64bit
471-
16
472-
}
454+
fn get_memory(v: &Self::V) -> Result<usize, RedisError> {
455+
Ok(match v.destructure_ref() {
456+
DestructuredRef::Null | DestructuredRef::Bool(_) => 0,
457+
DestructuredRef::Number(num) => {
458+
const STATIC_LO: i32 = -1 << 7; // INumber::STATIC_LOWER
459+
const STATIC_HI: i32 = 0b11 << 7; // INumber::STATIC_UPPER
460+
const SHORT_LO: i32 = -1 << 23; // INumber::SHORT_LOWER
461+
const SHORT_HI: i32 = 1 << 23; // INumber::SHORT_UPPER
462+
463+
if num.has_decimal_point() {
464+
16 // 64bit float
465+
} else if &INumber::from(STATIC_LO) <= num && num < &INumber::from(STATIC_HI) {
466+
0 // 8bit
467+
} else if &INumber::from(SHORT_LO) <= num && num < &INumber::from(SHORT_HI) {
468+
4 // 24bit
469+
} else {
470+
16 // 64bit
473471
}
474-
ValueType::String => v.as_string().unwrap().len(),
475-
ValueType::Array => {
476-
let arr = v.as_array().unwrap();
477-
let capacity = arr.capacity();
478-
if capacity == 0 {
479-
0
480-
} else {
481-
size_of::<usize>() * (capacity + 2)
482-
+ arr
483-
.into_iter()
484-
.map(|v| self.get_memory(v).unwrap())
485-
.sum::<usize>()
486-
}
472+
}
473+
DestructuredRef::String(s) => s.len(),
474+
DestructuredRef::Array(arr) => match arr.capacity() {
475+
0 => 0,
476+
capacity => {
477+
arr.into_iter() // IValueManager::get_memory() always returns OK, safe to unwrap here
478+
.map(|val| Self::get_memory(val).unwrap())
479+
.sum::<usize>()
480+
+ (capacity + 2) * size_of::<usize>()
487481
}
488-
ValueType::Object => {
489-
let val = v.as_object().unwrap();
490-
let capacity = val.capacity();
491-
if capacity == 0 {
492-
0
493-
} else {
494-
size_of::<usize>() * (capacity * 3 + 2)
495-
+ val
496-
.into_iter()
497-
.map(|(s, v)| s.len() + self.get_memory(v).unwrap())
498-
.sum::<usize>()
499-
}
482+
},
483+
DestructuredRef::Object(obj) => match obj.capacity() {
484+
0 => 0,
485+
capacity => {
486+
obj.into_iter() // IValueManager::get_memory() always returns OK, safe to unwrap here
487+
.map(|(s, val)| s.len() + Self::get_memory(val).unwrap())
488+
.sum::<usize>()
489+
+ (capacity * 3 + 2) * size_of::<usize>()
500490
}
501-
};
502-
Ok(res)
491+
},
492+
} + size_of::<IValue>())
503493
}
504494

505495
fn is_json(&self, key: *mut RedisModuleKey) -> Result<bool, RedisError> {
@@ -521,9 +511,6 @@ mod tests {
521511
fn test_get_memory() {
522512
let _guard = SINGLE_THREAD_TEST_MUTEX.lock();
523513

524-
let manager = RedisIValueJsonKeyManager {
525-
phantom: PhantomData,
526-
};
527514
let json = r#"{
528515
"a": 100.12,
529516
"b": "foo",
@@ -540,7 +527,7 @@ mod tests {
540527
"m": {"t": "f"}
541528
}"#;
542529
let value = serde_json::from_str(json).unwrap();
543-
let res = manager.get_memory(&value).unwrap();
530+
let res = RedisIValueJsonKeyManager::get_memory(&value).unwrap();
544531
assert_eq!(res, 903);
545532
}
546533

‎redis_json/src/manager.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ pub trait Manager {
9595
fn apply_changes(&self, ctx: &Context);
9696
#[allow(clippy::wrong_self_convention)]
9797
fn from_str(&self, val: &str, format: Format, limit_depth: bool) -> Result<Self::O, Error>;
98-
fn get_memory(&self, v: &Self::V) -> Result<usize, RedisError>;
98+
fn get_memory(v: &Self::V) -> Result<usize, RedisError>;
9999
fn is_json(&self, key: *mut RedisModuleKey) -> Result<bool, RedisError>;
100100
}
101101

‎redis_json/src/redisjson.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,6 @@ pub mod type_methods {
251251
#[allow(non_snake_case, unused)]
252252
pub unsafe extern "C" fn mem_usage(value: *const c_void) -> usize {
253253
let json = unsafe { &*(value as *mut RedisJSON<ijson::IValue>) };
254-
let manager = RedisIValueJsonKeyManager {
255-
phantom: PhantomData,
256-
};
257-
manager.get_memory(&json.data).unwrap_or(0)
254+
RedisIValueJsonKeyManager::get_memory(&json.data).unwrap_or(0)
258255
}
259256
}

0 commit comments

Comments
 (0)