This is an automated email from the ASF dual-hosted git repository.
hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 0723ae10 Check data type before metadata decode to avoid confusing
error message (#2015)
0723ae10 is described below
commit 0723ae10e6c75a555a37237b468645daf358968d
Author: 纪华裕 <[email protected]>
AuthorDate: Wed Jan 17 21:54:21 2024 +0800
Check data type before metadata decode to avoid confusing error message
(#2015)
---
src/storage/redis_db.cc | 5 +++-
src/storage/redis_metadata.cc | 12 ++++----
tests/gocase/unit/type/types_test.go | 54 ++++++++++++++++++++++++++++++++++++
3 files changed, 64 insertions(+), 7 deletions(-)
diff --git a/src/storage/redis_db.cc b/src/storage/redis_db.cc
index bdbd9789..eaa94b43 100644
--- a/src/storage/redis_db.cc
+++ b/src/storage/redis_db.cc
@@ -55,7 +55,8 @@ rocksdb::Status Database::ParseMetadata(RedisTypes types,
Slice *bytes, Metadata
});
auto s = metadata->Decode(bytes);
- if (!s.ok()) return s;
+ // delay InvalidArgument error check after type match check
+ if (!s.ok() && !s.IsInvalidArgument()) return s;
if (metadata->Expired()) {
// error discarded here since it already failed
@@ -69,6 +70,8 @@ rocksdb::Status Database::ParseMetadata(RedisTypes types,
Slice *bytes, Metadata
auto _ [[maybe_unused]] = metadata->Decode(old_metadata);
return rocksdb::Status::InvalidArgument(kErrMsgWrongType);
}
+ if (s.IsInvalidArgument()) return s;
+
if (metadata->size == 0 && !metadata->IsEmptyableType()) {
// error discarded here since it already failed
auto _ [[maybe_unused]] = metadata->Decode(old_metadata);
diff --git a/src/storage/redis_metadata.cc b/src/storage/redis_metadata.cc
index 23afc765..458ab4e8 100644
--- a/src/storage/redis_metadata.cc
+++ b/src/storage/redis_metadata.cc
@@ -335,13 +335,13 @@ rocksdb::Status ListMetadata::Decode(Slice *input) {
if (auto s = Metadata::Decode(input); !s.ok()) {
return s;
}
- if (Type() == kRedisList) {
- if (input->size() < 8 + 8) {
- return rocksdb::Status::InvalidArgument(kErrMetadataTooShort);
- }
- GetFixed64(input, &head);
- GetFixed64(input, &tail);
+
+ if (input->size() < 8 + 8) {
+ return rocksdb::Status::InvalidArgument(kErrMetadataTooShort);
}
+ GetFixed64(input, &head);
+ GetFixed64(input, &tail);
+
return rocksdb::Status::OK();
}
diff --git a/tests/gocase/unit/type/types_test.go
b/tests/gocase/unit/type/types_test.go
new file mode 100644
index 00000000..3a33c1d5
--- /dev/null
+++ b/tests/gocase/unit/type/types_test.go
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package types
+
+import (
+ "context"
+ "testing"
+
+ "github.com/apache/kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func TestTypesError(t *testing.T) {
+ srv := util.StartServer(t, map[string]string{})
+ defer srv.Close()
+ ctx := context.Background()
+ rdb := srv.NewClient()
+ defer func() { require.NoError(t, rdb.Close()) }()
+
+ t.Run("Operate with wrong type", func(t *testing.T) {
+ message := "ERR Invalid argument: WRONGTYPE Operation against a
key holding the wrong kind of value"
+ require.NoError(t, rdb.Set(ctx, "a", "hello", 0).Err())
+ require.EqualError(t, rdb.Do(ctx, "XADD", "a", "*", "a",
"test").Err(), message)
+ require.EqualError(t, rdb.Do(ctx, "LPUSH", "a", 1).Err(),
message)
+ require.EqualError(t, rdb.Do(ctx, "HSET", "a", "1", "2").Err(),
message)
+ require.EqualError(t, rdb.Do(ctx, "SADD", "a", "1", "2").Err(),
message)
+ require.EqualError(t, rdb.Do(ctx, "ZADD", "a", "1", "2").Err(),
message)
+ require.EqualError(t, rdb.Do(ctx, "JSON.SET", "a", "$",
"{}").Err(), message)
+ require.EqualError(t, rdb.Do(ctx, "BF.ADD", "a", "test").Err(),
message)
+ require.EqualError(t, rdb.Do(ctx, "SADD", "a", 100).Err(),
message)
+
+ require.NoError(t, rdb.LPush(ctx, "a1", "hello", 0).Err())
+ require.EqualError(t, rdb.Do(ctx, "SETBIT", "a1", 1, 1).Err(),
message)
+ require.EqualError(t, rdb.Do(ctx, "GET", "a1").Err(), message)
+
+ })
+}
