Move to RedisString #394
Move to RedisString #394
Conversation
| format = Format::from_str(args.next_string()?.as_str())?; | ||
| } | ||
| _ => break, | ||
| _ => break, // TODO shoudl return an error?? |
There was a problem hiding this comment.
Before commit 89b8a95 there seems to have been an error report Err(RedisError::Str("ERR syntax error"))
| { | ||
| if !value.is_array() { | ||
| return Err(err_json(value, "array")); | ||
| } | ||
|
|
||
| let mut items: Vec<Value> = args | ||
| .map(|json| serde_json::from_str(&json)) | ||
| .map(|json| serde_json::from_str(&json.into_string_lossy())) |
There was a problem hiding this comment.
Is a lossy conversion what we want here? I know the Redis way is not to return errors, but can't this cause silent data corruption in case the input is not valid UTF-8? JSON should always be valid UTF-8.
| format = Format::from_str(args.next_string()?.as_str())?; | ||
| } | ||
| _ => break, | ||
| _ => break, // TODO shoudl return an error?? |
There was a problem hiding this comment.
Before commit 89b8a95 there seems to have been an error report Err(RedisError::Str("ERR syntax error"))
| JSONApiKey::new_from_key(key) | ||
| } | ||
|
|
||
| pub fn new_from_str( | ||
| ctx: *mut rawmod::RedisModuleCtx, | ||
| path: *const c_char, | ||
| ) -> Result<JSONApiKey<'a>, RedisError> { | ||
| let ctx = Context::new(ctx); | ||
| let path = unsafe { CStr::from_ptr(path).to_str().unwrap() }; |
There was a problem hiding this comment.
This line not from this PR, but still would like to raise that since it is also related to UFT-8 and lossy conversion vs. error handling mentioned in this comment
let path = unsafe { CStr::from_ptr(path).to_str().unwrap() };
This would panic if invalid UTF-8 is passed here.
There was a problem hiding this comment.
Since c_api.rs currently has no error reporting capability, we should detect conversion failure and the caller should return null on failure (assuming invalid UFT-8 can arrive from redis/from callers of new_from_str)
No description provided.