-
Notifications
You must be signed in to change notification settings - Fork 37
fix substr crash #491
fix substr crash #491
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1313,32 +1313,26 @@ FunctionManager::FunctionManager() { | |
attr.isPure_ = true; | ||
attr.body_ = [](const auto &args) -> Value { | ||
auto argSize = args.size(); | ||
if (argSize < 2 || argSize >3) { | ||
if (argSize < 2 || argSize > 3) { | ||
LOG(ERROR) << "Unexpected arguments count " << args.size(); | ||
return Value::kNullBadData; | ||
} | ||
if (args[0].isNull()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. type Signature only use in deduce expr's type , When it encounters null, it returns directly in validator user cannot enter EMPTY .so we not process EMPTY There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the query input can be empty, such as pipe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. EMPTY will do later ,all function have this problem There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay |
||
return Value::kNullValue; | ||
} | ||
if (!args[0].isStr() || !args[1].isInt() || (argSize == 3 && !args[2].isInt())) { | ||
return Value::kNullBadType; | ||
} | ||
auto value = args[0].getStr(); | ||
auto start = args[1].getInt(); | ||
auto length = (args.size() == 2) ? value.size() - start : args[2].getInt(); | ||
if (args[0].isStr() && args[1].isInt()) { | ||
if (argSize == 3) { | ||
if (!args[2].isInt()) { | ||
return Value::kNullBadType; | ||
} | ||
} | ||
if (static_cast<size_t>(std::abs(start)) > value.size() || length == 0) { | ||
return std::string(""); | ||
} | ||
if (start < 0) { | ||
LOG(ERROR) << "Invalid Start index " << start; | ||
return Value::kNullBadData; | ||
} | ||
if (start == 0) { | ||
return value; | ||
} | ||
return value.substr(start, length); | ||
if (start < 0 || length < 0) { | ||
return Value::kNullBadData; | ||
} | ||
return Value::kNullBadType; | ||
if (static_cast<size_t>(start) > value.size() || length == 0) { | ||
return std::string(""); | ||
} | ||
return value.substr(start, length); | ||
}; | ||
functions_["substring"] = attr; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,6 +150,11 @@ TEST_F(FunctionManagerTest, testNull) { | |
TEST_FUNCTION(toUpper, args_["nullvalue"], Value::kNullValue); | ||
TEST_FUNCTION(split, std::vector<Value>({Value::kNullValue, ","}), Value::kNullValue); | ||
TEST_FUNCTION(split, std::vector<Value>({"123,22", Value::kNullValue}), Value::kNullValue); | ||
TEST_FUNCTION(substr, std::vector<Value>({Value::kNullValue, 1, 2}), Value::kNullValue); | ||
TEST_FUNCTION(substr, std::vector<Value>({"hello", Value::kNullValue, 2}), Value::kNullBadType); | ||
TEST_FUNCTION(substr, std::vector<Value>({"hello", 2, Value::kNullValue}), Value::kNullBadType); | ||
TEST_FUNCTION(substr, std::vector<Value>({"hello", -1, 10}), Value::kNullBadData); | ||
TEST_FUNCTION(substr, std::vector<Value>({"hello", 1, -2}), Value::kNullBadData); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add test when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alread exist in line 244 |
||
} | ||
|
||
TEST_F(FunctionManagerTest, functionCall) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The find method will check the count of arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix