Skip to content

Commit 3083bec

Browse files
oshadmioshadmi
committed
Allow get_long to fail
1 parent ecbb30e commit 3083bec

File tree

7 files changed

+126
-45
lines changed

7 files changed

+126
-45
lines changed

‎src/c_api.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,13 @@ pub fn json_api_get_json_from_iter<M: Manager>(
157157
pub fn json_api_get_int<M: Manager>(_: M, json: *const c_void, val: *mut c_longlong) -> c_int {
158158
let json = unsafe { &*(json.cast::<M::V>()) };
159159
match json.get_type() {
160-
SelectValueType::Long => {
161-
unsafe { *val = json.get_long() };
162-
Status::Ok as c_int
163-
}
160+
SelectValueType::Long => json.get_long().map_or_else(
161+
|_| Status::Err as c_int,
162+
|v| {
163+
unsafe { *val = v };
164+
Status::Ok as c_int
165+
},
166+
),
164167
_ => Status::Err as c_int,
165168
}
166169
}
@@ -173,10 +176,13 @@ pub fn json_api_get_double<M: Manager>(_: M, json: *const c_void, val: *mut c_do
173176
unsafe { *val = json.get_double() };
174177
Status::Ok as c_int
175178
}
176-
SelectValueType::Long => {
177-
unsafe { *val = json.get_long() as f64 };
178-
Status::Ok as c_int
179-
}
179+
SelectValueType::Long => json.get_long().map_or_else(
180+
|_| Status::Err as c_int,
181+
|v| {
182+
unsafe { *val = v as f64 };
183+
Status::Ok as c_int
184+
},
185+
),
180186
_ => Status::Err as c_int,
181187
}
182188
}

‎src/commands.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,9 @@ impl<'a, V: SelectValue + 'a> KeyValue<'a, V> {
128128
}
129129
}
130130

131-
SelectValueType::Long => RedisValue::Integer(v.get_long()),
131+
SelectValueType::Long => {
132+
RedisValue::Integer(v.get_long().map_or_else(|_| i64::MAX, |v| v))
133+
}
132134

133135
SelectValueType::Double => RedisValue::Float(v.get_double()),
134136

@@ -417,7 +419,10 @@ impl<'a, V: SelectValue + 'a> KeyValue<'a, V> {
417419
match (a.get_type(), b.get_type()) {
418420
(SelectValueType::Null, SelectValueType::Null) => true,
419421
(SelectValueType::Bool, SelectValueType::Bool) => a.get_bool() == b.get_bool(),
420-
(SelectValueType::Long, SelectValueType::Long) => a.get_long() == b.get_long(),
422+
(SelectValueType::Long, SelectValueType::Long) => match (a.get_long(), b.get_long()) {
423+
(Ok(a), Ok(b)) => a == b,
424+
_ => false,
425+
},
421426
(SelectValueType::Double, SelectValueType::Double) => a.get_double() == b.get_double(),
422427
(SelectValueType::String, SelectValueType::String) => a.get_str() == b.get_str(),
423428
(SelectValueType::Array, SelectValueType::Array) => {
@@ -1933,7 +1938,7 @@ pub fn json_clear<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>)
19331938

19341939
let paths = find_paths(path, root, |v| match v.get_type() {
19351940
SelectValueType::Array | SelectValueType::Object => v.len().unwrap() > 0,
1936-
SelectValueType::Long => v.get_long() != 0,
1941+
SelectValueType::Long => v.get_long().map_or_else(|_| false, |v| v != 0),
19371942
SelectValueType::Double => v.get_double() != 0.0,
19381943
_ => false,
19391944
})?;

‎src/ivalue_manager.rs

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
/*
2-
* Copyright Redis Ltd. 2016 - present
3-
* Licensed under your choice of the Redis Source Available License 2.0 (RSALv2) or
4-
* the Server Side Public License v1 (SSPLv1).
5-
*/
6-
1+
/*
2+
* Copyright Redis Ltd. 2016 - present
3+
* Licensed under your choice of the Redis Source Available License 2.0 (RSALv2) or
4+
* the Server Side Public License v1 (SSPLv1).
5+
*/
6+
77
use crate::error::Error;
8+
use crate::jsonpath::select_value::SelectValue;
89
use crate::manager::{err_json, err_msg_json_expected, err_msg_json_path_doesnt_exist};
910
use crate::manager::{Manager, ReadHolder, WriteHolder};
1011
use crate::redisjson::normalize_arr_start_index;
@@ -219,7 +220,7 @@ impl<'a> IValueKeyHolderWrite<'a> {
219220
mut op2_fun: F2,
220221
) -> Result<Number, RedisError>
221222
where
222-
F1: FnMut(i64, i64) -> i64,
223+
F1: FnMut(i64, i64) -> (i64, bool),
223224
F2: FnMut(f64, f64) -> f64,
224225
{
225226
let in_value = &serde_json::from_str(num)?;
@@ -230,14 +231,40 @@ impl<'a> IValueKeyHolderWrite<'a> {
230231
v.as_number().unwrap().has_decimal_point(),
231232
in_value.as_i64(),
232233
) {
233-
(false, Some(num2)) => Ok(((op1_fun)(v.to_i64().unwrap(), num2)).into()),
234-
_ => {
235-
let num1 = v.to_f64().unwrap();
236-
let num2 = in_value.as_f64().unwrap();
237-
if let Ok(num) = INumber::try_from((op2_fun)(num1, num2)) {
238-
Ok(num)
234+
(false, Some(num2)) => {
235+
// Both are integers
236+
if let Ok(num1) = v.get_long() {
237+
// v is i64
238+
let (num, _) = (op1_fun)(num1, num2);
239+
Ok(INumber::from(num))
239240
} else {
240-
Err(RedisError::Str("result is not a number"))
241+
Err(RedisError::Str("u64 is not supported"))
242+
}
243+
}
244+
(false, None) => {
245+
// Left is integer, Right is f64 or u64
246+
match (v.get_long(), in_value.as_f64()) {
247+
(Ok(num1), Some(num2)) => {
248+
if let Ok(num) = INumber::try_from((op2_fun)(num1 as f64, num2)) {
249+
Ok(num)
250+
} else {
251+
Err(RedisError::Str("result is not a number"))
252+
}
253+
}
254+
_ => Err(RedisError::Str("u64 is not supported")),
255+
}
256+
}
257+
_ => {
258+
// Both are double
259+
match (v.get_double(), in_value.as_f64()) {
260+
(num1, Some(num2)) => {
261+
if let Ok(num) = INumber::try_from((op2_fun)(num1, num2)) {
262+
Ok(num)
263+
} else {
264+
Err(RedisError::Str("result is not a number"))
265+
}
266+
}
267+
_ => Err(RedisError::Str("bad input number")),
241268
}
242269
}
243270
};
@@ -382,15 +409,15 @@ impl<'a> WriteHolder<IValue, IValue> for IValueKeyHolderWrite<'a> {
382409
}
383410

384411
fn incr_by(&mut self, path: Vec<String>, num: &str) -> Result<Number, RedisError> {
385-
self.do_num_op(path, num, |i1, i2| i1 + i2, |f1, f2| f1 + f2)
412+
self.do_num_op(path, num, |i1, i2| i1.overflowing_add(i2), |f1, f2| f1 + f2)
386413
}
387414

388415
fn mult_by(&mut self, path: Vec<String>, num: &str) -> Result<Number, RedisError> {
389-
self.do_num_op(path, num, |i1, i2| i1 * i2, |f1, f2| f1 * f2)
416+
self.do_num_op(path, num, |i1, i2| i1.overflowing_mul(i2), |f1, f2| f1 * f2)
390417
}
391418

392419
fn pow_by(&mut self, path: Vec<String>, num: &str) -> Result<Number, RedisError> {
393-
self.do_num_op(path, num, |i1, i2| i1.pow(i2 as u32), f64::powf)
420+
self.do_num_op(path, num, |i1, i2| i1.overflowing_pow(i2 as u32), f64::powf)
394421
}
395422

396423
fn bool_toggle(&mut self, path: Vec<String>) -> Result<bool, RedisError> {

‎src/jsonpath/json_node.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* the Server Side Public License v1 (SSPLv1).
55
*/
66

7+
use crate::error::Error;
78
use crate::jsonpath::select_value::{SelectValue, SelectValueType};
89
use ijson::{IValue, ValueType};
910
use serde_json::Value;
@@ -118,13 +119,13 @@ impl SelectValue for Value {
118119
}
119120
}
120121

121-
fn get_long(&self) -> i64 {
122+
fn get_long(&self) -> Result<i64, Error> {
122123
match self {
123124
Self::Number(n) => {
124125
if let Some(n) = n.as_i64() {
125-
n
126+
Ok(n)
126127
} else {
127-
panic!("not a long");
128+
Err(Error::from("u64 is currently unsupported"))
128129
}
129130
}
130131
_ => {
@@ -244,17 +245,17 @@ impl SelectValue for IValue {
244245
}
245246
}
246247

247-
fn get_long(&self) -> i64 {
248+
fn get_long(&self) -> Result<i64, Error> {
248249
match self.as_number() {
249250
Some(n) => {
250251
if n.has_decimal_point() {
251252
panic!("not a long");
252253
} else {
253254
if let Some(n) = n.to_i64() {
254-
n
255+
Ok(n)
255256
} else {
256257
// Workaround until adding an API `get_ulong(&self) -> u64`
257-
i64::MAX
258+
Err(Error::from("u64 is currently unsupported"))
258259
}
259260
}
260261
}

‎src/jsonpath/json_path.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,14 +378,20 @@ impl<'i, 'j, S: SelectValue> TermEvaluationResult<'i, 'j, S> {
378378
CmpResult::Ord(b1.cmp(b2))
379379
}
380380
(TermEvaluationResult::Value(v), _) => match v.get_type() {
381-
SelectValueType::Long => TermEvaluationResult::Integer(v.get_long()).cmp(s),
381+
SelectValueType::Long => v.get_long().map_or_else(
382+
|_| CmpResult::NotCmparable,
383+
|n| TermEvaluationResult::Integer(n).cmp(s),
384+
),
382385
SelectValueType::Double => TermEvaluationResult::Float(v.get_double()).cmp(s),
383386
SelectValueType::String => TermEvaluationResult::Str(v.as_str()).cmp(s),
384387
SelectValueType::Bool => TermEvaluationResult::Bool(v.get_bool()).cmp(s),
385388
_ => CmpResult::NotCmparable,
386389
},
387390
(_, TermEvaluationResult::Value(v)) => match v.get_type() {
388-
SelectValueType::Long => self.cmp(&TermEvaluationResult::Integer(v.get_long())),
391+
SelectValueType::Long => v.get_long().map_or_else(
392+
|_| CmpResult::NotCmparable,
393+
|n| self.cmp(&TermEvaluationResult::Integer(n)),
394+
),
389395
SelectValueType::Double => self.cmp(&TermEvaluationResult::Float(v.get_double())),
390396
SelectValueType::String => self.cmp(&TermEvaluationResult::Str(v.as_str())),
391397
SelectValueType::Bool => self.cmp(&TermEvaluationResult::Bool(v.get_bool())),

‎src/jsonpath/select_value.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
/*
2-
* Copyright Redis Ltd. 2016 - present
3-
* Licensed under your choice of the Redis Source Available License 2.0 (RSALv2) or
4-
* the Server Side Public License v1 (SSPLv1).
5-
*/
6-
1+
/*
2+
* Copyright Redis Ltd. 2016 - present
3+
* Licensed under your choice of the Redis Source Available License 2.0 (RSALv2) or
4+
* the Server Side Public License v1 (SSPLv1).
5+
*/
6+
7+
use crate::error::Error;
78
use serde::Serialize;
89
use std::fmt::Debug;
910

@@ -33,6 +34,6 @@ pub trait SelectValue: Debug + Eq + PartialEq + Default + Clone + Serialize {
3334
fn get_str(&self) -> String;
3435
fn as_str(&self) -> &str;
3536
fn get_bool(&self) -> bool;
36-
fn get_long(&self) -> i64;
37+
fn get_long(&self) -> Result<i64, Error>;
3738
fn get_double(&self) -> f64;
3839
}

‎tests/pytest/test.py

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,6 +1300,18 @@ def testOutOfRangeValues(env):
13001300
"max2_f64": 1.7976931348623158e+308,
13011301
"min_f64" : -1.7976931348623157e+308,
13021302
"min2_f64": -1.7976931348623158e+308,
1303+
#"max_u64": 18446744073709551615.0,
1304+
}
1305+
1306+
doc_float_ok_2 = {
1307+
# i64
1308+
"max_i64": 9223372036854775807.0,
1309+
"beyond_max_i64": 9223372036854775808.0, # as u64
1310+
"min_i64": -9223372036854775808.0,
1311+
"below_min_i64": -9223372036854775809.0, # as f64
1312+
# u64
1313+
"max_u64": 18446744073709551615.0,
1314+
"beyond_max_u64": 18446744073709551616.0, # as f64
13031315
}
13041316

13051317
doc_bad_values = {
@@ -1309,16 +1321,39 @@ def testOutOfRangeValues(env):
13091321

13101322
r.expect('JSON.SET', 'doc_int_ok', '$', json.dumps(doc_int_ok)).ok()
13111323
r.expect('JSON.SET', 'doc_float_ok', '$', json.dumps(doc_float_ok)).ok()
1324+
r.expect('JSON.SET', 'doc_float_ok_2', '$', json.dumps(doc_float_ok_2)).ok()
13121325

13131326
def check_object_values(obj, name, epsilon):
1327+
# Test values from JSON.GET are equal to JSON.SET
1328+
# Check no crash (using get_long internally)
1329+
arr = []
13141330
for k, v in iter(obj.items()):
13151331
r.assertTrue(True, message='GET {}={}'.format(k, v))
13161332
res = r.execute_command('JSON.GET', name, '$.{}'.format(k))
13171333
r.assertAlmostEqual(json.loads(res)[0], v, epsilon, message=res)
1318-
1319-
# Test values from JSON.GET are equal to JSON.SET
1334+
try:
1335+
# Check no crash on overflow
1336+
res = r.execute_command('JSON.NUMINCRBY', name, '$.{}'.format(k), '1')
1337+
#r.assertAlmostEqual(json.loads(res)[0], v+1, epsilon, message=res)
1338+
res = r.execute_command('JSON.NUMMULTBY', name, '$.{}'.format(k), '2')
1339+
#r.assertAlmostEqual(json.loads(res)[0], v*2, epsilon, message=res)
1340+
except Exception as e:
1341+
pass
1342+
arr.append(v)
1343+
# Check no crash using values in a filter
1344+
r.expect('JSON.SET', 'arr', '$', json.dumps(arr)).ok()
1345+
res = r.execute_command('JSON.GET', 'arr', '$[?(@!=0)]')
1346+
for res_i, arr_i in zip(json.loads(res), arr):
1347+
r.assertAlmostEqual(res_i, arr_i, epsilon, message=res)
1348+
# Check no crash with other commands
1349+
r.execute_command('JSON.ARRINDEX', 'arr', '$', '10')
1350+
r.execute_command('JSON.RESP', 'arr')
1351+
for k, v in iter(obj.items()):
1352+
r.execute_command('JSON.CLEAR', name, '$.{}'.format(k))
1353+
13201354
check_object_values(doc_int_ok, 'doc_int_ok', 0)
13211355
check_object_values(doc_float_ok, 'doc_float_ok', sys.float_info.epsilon)
1356+
check_object_values(doc_float_ok_2, 'doc_float_ok_2', sys.float_info.epsilon)
13221357

13231358
# Not using json.dumps with out-of-range values here (would be converted to a string representation such as 'Infinity')
13241359
r.expect('JSON.SET', 'doc_bad_values', '$', '{}').ok()

0 commit comments

Comments
 (0)