diff --git a/src/storage/redis_db.cc b/src/storage/redis_db.cc index bdbd9789490..eaa94b431ee 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 23afc765457..458ab4e880e 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 00000000000..3a33c1d56c2 --- /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) + + }) +}