mirror of
https://github.com/dragonflydb/dragonfly.git
synced 2024-12-15 17:51:06 +00:00
fix(parser): Fix wrong parsing of nested arrays. (#140)
Relevant only for client-side parsing. Thus it did not affect production code. Signed-off-by: Roman Gershman <roman@dragonflydb.io>
This commit is contained in:
parent
f7d3f4d640
commit
15fd451487
3 changed files with 19 additions and 10 deletions
|
@ -247,10 +247,7 @@ auto RedisParser::ConsumeArrayLen(Buffer str) -> Result {
|
|||
LOG(ERROR) << "Unexpected result " << res;
|
||||
}
|
||||
|
||||
if (parse_stack_.size() > 0 && server_mode_)
|
||||
return BAD_STRING;
|
||||
|
||||
if (parse_stack_.size() == 0 && !cached_expr_->empty())
|
||||
if (server_mode_ && (parse_stack_.size() > 0 || !cached_expr_->empty()))
|
||||
return BAD_STRING;
|
||||
|
||||
if (len <= 0) {
|
||||
|
@ -266,17 +263,21 @@ auto RedisParser::ConsumeArrayLen(Buffer str) -> Result {
|
|||
return OK;
|
||||
}
|
||||
|
||||
DVLOG(1) << "PushStack: (" << len << ", " << cached_expr_ << ")";
|
||||
parse_stack_.emplace_back(len, cached_expr_);
|
||||
if (!cached_expr_->empty()) {
|
||||
|
||||
if (state_ == PARSE_ARG_S) {
|
||||
DCHECK(!server_mode_);
|
||||
|
||||
cached_expr_->emplace_back(RespExpr::ARRAY);
|
||||
stash_.emplace_back(new RespExpr::Vec());
|
||||
RespExpr::Vec* arr = stash_.back().get();
|
||||
arr->reserve(len);
|
||||
cached_expr_->back().u = arr;
|
||||
cached_expr_ = arr;
|
||||
} else {
|
||||
state_ = PARSE_ARG_S;
|
||||
}
|
||||
state_ = PARSE_ARG_S;
|
||||
|
||||
return OK;
|
||||
}
|
||||
|
@ -304,6 +305,7 @@ auto RedisParser::ParseArg(Buffer str) -> Result {
|
|||
state_ = FINISH_ARG_S;
|
||||
cached_expr_->emplace_back(RespExpr::NIL);
|
||||
} else {
|
||||
DVLOG(1) << "String(" << len << ")";
|
||||
cached_expr_->emplace_back(RespExpr::STRING);
|
||||
bulk_len_ = len;
|
||||
state_ = BULK_STR_S;
|
||||
|
@ -405,15 +407,16 @@ auto RedisParser::ConsumeBulk(Buffer str) -> Result {
|
|||
}
|
||||
|
||||
void RedisParser::HandleFinishArg() {
|
||||
state_ = PARSE_ARG_S;
|
||||
DCHECK(!parse_stack_.empty());
|
||||
DCHECK_GT(parse_stack_.back().first, 0u);
|
||||
|
||||
while (true) {
|
||||
--parse_stack_.back().first;
|
||||
state_ = PARSE_ARG_S;
|
||||
if (parse_stack_.back().first != 0)
|
||||
break;
|
||||
|
||||
auto* arr = parse_stack_.back().second;
|
||||
DVLOG(1) << "PopStack (" << arr << ")";
|
||||
parse_stack_.pop_back(); // pop 0.
|
||||
if (parse_stack_.empty()) {
|
||||
state_ = CMD_COMPLETE_S;
|
||||
|
|
|
@ -11,6 +11,9 @@ namespace facade {
|
|||
|
||||
/**
|
||||
* @brief Zero-copy (best-effort) parser.
|
||||
* Note: The client-mode parsing is buggy and should not be used in production.
|
||||
* Currently we only use server-mode parsing in production and client-mode in tests.
|
||||
* It works because tests do not do any incremental parsing.
|
||||
*
|
||||
*/
|
||||
class RedisParser {
|
||||
|
|
|
@ -152,8 +152,11 @@ TEST_F(RedisParserTest, Hierarchy) {
|
|||
const char* kThirdArg = "*2\r\n$3\r\n100\r\n$3\r\n200\r\n";
|
||||
string resp = absl::StrCat("*3\r\n$3\r\n900\r\n$3\r\n800\r\n", kThirdArg);
|
||||
ASSERT_EQ(RedisParser::OK, Parse(resp));
|
||||
EXPECT_THAT(args_, ElementsAre("900", "800", ArrArg(2)));
|
||||
EXPECT_THAT(*get<RespVec*>(args_[2].u), ElementsAre("100", "200"));
|
||||
ASSERT_THAT(args_, ElementsAre("900", "800", ArrArg(2)));
|
||||
EXPECT_THAT(args_[2].GetVec(), ElementsAre("100", "200"));
|
||||
|
||||
ASSERT_EQ(RedisParser::OK, Parse("*2\r\n*1\r\n$3\r\n1-0\r\n*1\r\n$2\r\nf1\r\n"));
|
||||
ASSERT_THAT(args_, ElementsAre(ArrLen(1), ArrLen(1)));
|
||||
}
|
||||
|
||||
TEST_F(RedisParserTest, InvalidMult1) {
|
||||
|
|
Loading…
Reference in a new issue