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 67208e6e Fix incorrent bytes to calculate length of ziplist/listpack
32bit string (#2124)
67208e6e is described below
commit 67208e6e94a7a98477ee214cda3dd7e890da5a55
Author: hulk <[email protected]>
AuthorDate: Thu Feb 29 00:37:05 2024 +0800
Fix incorrent bytes to calculate length of ziplist/listpack 32bit string
(#2124)
Currently, it calculates the length of 32-bit string from position 0 to 3,
but the right position should be 1 to 4 in ziplist/listpack.
So it will retrieve a wrong length and throw an error.
---
src/storage/rdb_listpack.cc | 4 +-
src/storage/rdb_ziplist.cc | 4 +-
tests/gocase/unit/rdb/rdb_test.go | 61 +++++++++++++++++++++++++++++++
tests/gocase/unit/restore/restore_test.go | 7 ++++
4 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/src/storage/rdb_listpack.cc b/src/storage/rdb_listpack.cc
index 4eb3597d..cd4bba4d 100644
--- a/src/storage/rdb_listpack.cc
+++ b/src/storage/rdb_listpack.cc
@@ -207,8 +207,8 @@ StatusOr<std::string> ListPack::Next() {
pos_ += value_len + encodeBackLen(value_len + 2);
} else if ((c & ListPack32BitStringMask) == ListPack32BitString) { // 32bit
string
GET_OR_RET(peekOK(5));
- value_len = (static_cast<uint32_t>(data[pos_])) |
(static_cast<uint32_t>(data[pos_ + 1]) << 8) |
- (static_cast<uint32_t>(data[pos_ + 2]) << 16) |
(static_cast<uint32_t>(data[pos_ + 3]) << 24);
+ value_len = (static_cast<uint32_t>(data[pos_ + 1])) |
(static_cast<uint32_t>(data[pos_ + 2]) << 8) |
+ (static_cast<uint32_t>(data[pos_ + 3]) << 16) |
(static_cast<uint32_t>(data[pos_ + 4]) << 24);
// skip 5byte encoding type
pos_ += 5;
GET_OR_RET(peekOK(value_len));
diff --git a/src/storage/rdb_ziplist.cc b/src/storage/rdb_ziplist.cc
index 3d7cfd84..772226ea 100644
--- a/src/storage/rdb_ziplist.cc
+++ b/src/storage/rdb_ziplist.cc
@@ -63,8 +63,8 @@ StatusOr<std::string> ZipList::Next() {
} else if ((encoding) == ZIP_STR_32B) {
GET_OR_RET(peekOK(5));
len_bytes = 5;
- len = (static_cast<uint32_t>(data[pos_]) << 24) |
(static_cast<uint32_t>(data[pos_ + 1]) << 16) |
- (static_cast<uint32_t>(data[pos_ + 2]) << 8) |
static_cast<uint32_t>(data[pos_ + 3]);
+ len = (static_cast<uint32_t>(data[pos_ + 1]) << 24) |
(static_cast<uint32_t>(data[pos_ + 2]) << 16) |
+ (static_cast<uint32_t>(data[pos_ + 3]) << 8) |
static_cast<uint32_t>(data[pos_ + 4]);
} else {
return {Status::NotOK, "invalid ziplist encoding"};
}
diff --git a/tests/gocase/unit/rdb/rdb_test.go
b/tests/gocase/unit/rdb/rdb_test.go
new file mode 100644
index 00000000..619bd4ea
--- /dev/null
+++ b/tests/gocase/unit/rdb/rdb_test.go
@@ -0,0 +1,61 @@
+/*
+ * 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 rdb
+
+import (
+ "context"
+ "encoding/base64"
+ "io"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/apache/kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func TestLoadRDB(t *testing.T) {
+
+ srv := util.StartServer(t, map[string]string{})
+ defer srv.Close()
+
+ ctx := context.Background()
+ client := srv.NewClient()
+ defer func() { require.NoError(t, client.Close()) }()
+
+ t.Run("key with ziplist 32bit string", func(t *testing.T) {
+ encodedData :=
"UkVESVMwMDA5+glyZWRpcy12ZXIGNi4yLjEw+gpyZWRpcy1iaXRzwED6BWN0aW1lwtbG3GX6CHVzZWQtbWVtwsA9FAD6DGFvZi1wcmVhbWJsZcAA/gD7AQAOBEFCQ0QNwxdB6wTrAQAA6CADBPAAAAAC4P8B4MsBAQD/w0HNgAAAi5EOkYsAAAoAAAABAACAAACLIAMBwH/g/wPhDQfg/wDg/wDg/wDg/wDg/wDg/wDg/wDg/wDg/wDg/wDg/wDg/wDg/wDg/wDg/wDgYwBP++D/AOD/AOD/AOD/AEAA5P8n4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P8A4LcAAcB/4LfB4P8A4P8A4P8A4P8A4P8A4P8A4P8A4P
[...]
+ decoder := base64.NewDecoder(base64.StdEncoding,
strings.NewReader(encodedData))
+ decodedData, err := io.ReadAll(decoder)
+ require.NoError(t, err)
+
+ rdbFileName := "dump-with-32bit-string.rdb"
+ require.NoError(t, os.WriteFile(rdbFileName, decodedData, 0644))
+ defer func() {
+ _ = os.Remove(rdbFileName)
+ }()
+
+ absFilePath, err := filepath.Abs(rdbFileName)
+ require.NoError(t, err)
+ require.NoError(t, client.Do(ctx, "RDB", "LOAD",
absFilePath).Err())
+ require.EqualValues(t, 601, client.LLen(ctx, "ABCD").Val())
+ })
+}
diff --git a/tests/gocase/unit/restore/restore_test.go
b/tests/gocase/unit/restore/restore_test.go
index 81d4a076..e4c8bc8c 100644
--- a/tests/gocase/unit/restore/restore_test.go
+++ b/tests/gocase/unit/restore/restore_test.go
@@ -193,6 +193,13 @@ func TestRestore_List(t *testing.T) {
}, rdb.LRange(ctx, key, 0, -1).Val())
require.EqualValues(t, -1, rdb.TTL(ctx, key).Val())
})
+
+ t.Run("List pack with 32bit string", func(t *testing.T) {
+ key := util.RandString(32, 64, util.Alpha)
+ value :=
"\x12\r\x02\xc3\x12A\xe7\a\xe7\x01\x00\x00\xf0\x00\x80\x01\xe0\xff\x01\xe0\xcc\x01\x01\x01\xff\x02\xc3A\xca\x80\x00\x00\x8b\x8f\a\x8f\x8b\x00\x00\x01\x00\xf0\x80
\x06\x03\x00\x00\xc0\x7f\xe0\xff\x03\xe1\r\a\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0c\x00O\xfb\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00\xe0\xff\x00@\x00\xe4\xff'\xe0\xff\x00\xe0\xf
[...]
+ require.NoError(t, rdb.Restore(ctx, key, 0, value).Err())
+ require.EqualValues(t, 601, rdb.LLen(ctx, key).Val())
+ })
}
func TestRestore_Set(t *testing.T) {
