Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(string): the server will be crashed when passing a negative offset in SETRANGE command #2783

Merged
merged 3 commits into from
Feb 10, 2025
Merged

fix(string): the server will be crashed when passing a negative offset in SETRANGE command #2783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ccc4f7
fix(string): the server will be crashed when passing a negative offse…
git-hulk Feb 10, 2025
152579a
Fix the offset range
git-hulk Feb 10, 2025
a4a1dd7
Revert the removed lines
git-hulk Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/commands/cmd_string.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class CommandSubStr : public CommandGetRange {
class CommandSetRange : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
auto parse_result = ParseInt<int>(args[2], 10);
auto parse_result = ParseInt<int>(args[2], {0, INT32_MAX}, 10);
if (!parse_result) {
return {Status::RedisParseErr, errValueNotInteger};
}
Expand All @@ -234,6 +234,11 @@ class CommandSetRange : public Commander {
uint64_t ret = 0;
redis::String string_db(srv->storage, conn->GetNamespace());

auto total = offset_ + args_[3].size();
if (total > srv->GetConfig()->proto_max_bulk_len) {
return {Status::RedisExecErr, "string exceeds maximum allowed size"};
}

auto s = string_db.SetRange(ctx, args_[1], offset_, args_[3], &ret);
if (!s.ok()) {
return {Status::RedisExecErr, s.ToString()};
Expand Down
17 changes: 17 additions & 0 deletions tests/gocase/unit/type/strings/strings_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,23 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) {
require.ErrorContains(t, rdb.SetRange(ctx, "mykey", 0, "bar").Err(), "WRONGTYPE")
})

t.Run("SETRANGE with negative offset", func(t *testing.T) {
require.ErrorContains(t, rdb.SetRange(ctx, "setrange_negative_offset", -1, "bar").Err(),
"value is not an integer or out of range")
require.ErrorContains(t, rdb.SetRange(ctx, "setrange_negative_offset", -2147483599, "bar").Err(),
"value is not an integer or out of range")
})

t.Run("SETRANGE with offset + value length too large", func(t *testing.T) {
protoMaxBulkLen := int64(1024 * 1024)
require.NoError(t, rdb.ConfigSet(ctx, "proto-max-bulk-len", strconv.FormatInt(protoMaxBulkLen, 10)).Err())
require.ErrorContains(t, rdb.SetRange(ctx, "setrange_out_of_range", protoMaxBulkLen, "world").Err(),
"string exceeds maximum allowed size")

// it should be able to set the value if the length is protoMaxBulkLen
require.NoError(t, rdb.SetRange(ctx, "setrange_out_of_range", protoMaxBulkLen-5, "world").Err())
})

t.Run("GETRANGE against non-existing key", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "mykey").Err())
require.EqualValues(t, "", rdb.GetRange(ctx, "mykey", 0, -1).Val())
Expand Down
Loading