Fix Wrongly parsed the RESP empty/null array #652
Conversation
|
Cool, can you add some test cases in protocol.tcl to make sure it has already fixed? |
There was a problem hiding this comment.
Agree with @git-hulk, the rest looks good. Thanks for contribution.
| } catch (std::exception &e) { | ||
| free(line); | ||
| return Status(Status::NotOK, "Protocol error: invalid multibulk length"); | ||
| } | ||
| if (multi_bulk_len_ <= 0) { |
There was a problem hiding this comment.
Is an assignment multi_bulk_len_ = 0 needed while the condition multi_bulk_len_ < 0 holds?
There was a problem hiding this comment.
There is a condition if (multi_bulk_len_ == 0) in src/redis_request.cc:111
There was a problem hiding this comment.
state_ is still ArrayLen, so it will read again and assign to multi_bulk_len_. it's a good habit that to add the assignment multi_bulk_len_ = 0. Thank you
sure, that's what i should do. |
Cool, thanks @willshS |
|
Shall this patch fix #633? |
yes |
tisonkun
changed the title
This closes #633.
Now : I only fix like:
echo -e '*-1\r\n*2\r\n$3\r\nget\r\n$3\r\nkey\r\n' | nc 127.0.0.1 6666echo -e '*2\r\n$3\r\nget\r\n$3\r\nkey\r\n*0\r\n*2\r\n$3\r\nget\r\n$3\r\nkey\r\n' | nc 127.0.0.1 6666Redis can have "nil" value. like:
kvrocks also behaves the same way.(I didn't modify anything about "nil" value)