storage: read the super block extra before unmarshaling it (#11932)

* storage: read the super block extra before unmarshaling it

ReadSuperBlock allocated a buffer of ExtraSize bytes but never filled it,
so any volume with a super block extra failed to load with an invalid
wire-format error.

Co-authored-by: chuanying chen <18412672+ccy00808@users.noreply.github.com>

* storage: test super block extra read error paths

---------

Co-authored-by: chuanying chen <18412672+ccy00808@users.noreply.github.com>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>

weed/storage/super_block/super_block_read.go.go

@@ -34,10 +34,16 @@ func ReadSuperBlock(datBackend backend.BackendStorageFile) (superBlock SuperBloc
 	if superBlock.ExtraSize > 0 {
 		// read more
 		extraData := make([]byte, int(superBlock.ExtraSize))
+		if n, e := datBackend.ReadAt(extraData, SuperBlockSize); e != nil {
+			if n != len(extraData) {
+				err = fmt.Errorf("cannot read volume %s super block extra: %v", datBackend.Name(), e)
+				return
+			}
+		}
 		superBlock.Extra = &master_pb.SuperBlockExtra{}
 		err = proto.Unmarshal(extraData, superBlock.Extra)
 		if err != nil {
-			err = fmt.Errorf("cannot read volume %s super block extra: %v", datBackend.Name(), err)
+			err = fmt.Errorf("cannot unmarshal volume %s super block extra: %v", datBackend.Name(), err)
 			return
 		}
 	}

weed/storage/super_block/super_block_test.go

@@ -1,8 +1,16 @@
 package super_block
 
 import (
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
+	"google.golang.org/protobuf/proto"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb/master_pb"
+	"github.com/seaweedfs/seaweedfs/weed/storage/backend"
 	"github.com/seaweedfs/seaweedfs/weed/storage/needle"
 )
 
@@ -23,3 +31,114 @@ func TestSuperBlockReadWrite(t *testing.T) {
 	}
 
 }
+
+func TestReadSuperBlockWithExtra(t *testing.T) {
+	rp, _ := NewReplicaPlacementFromByte(byte(001))
+	ttl, _ := needle.ReadTTL("15d")
+	s := &SuperBlock{
+		Version:          needle.GetCurrentVersion(),
+		ReplicaPlacement: rp,
+		Ttl:              ttl,
+		Extra: &master_pb.SuperBlockExtra{
+			ErasureCoding: &master_pb.SuperBlockExtra_ErasureCoding{
+				Data:      10,
+				Parity:    4,
+				VolumeIds: []uint32{1, 2, 3},
+			},
+		},
+	}
+
+	datPath := filepath.Join(t.TempDir(), "1.dat")
+	if err := os.WriteFile(datPath, s.Bytes(), 0644); err != nil {
+		t.Fatal(err)
+	}
+	f, err := os.Open(datPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	datBackend := backend.NewDiskFile(f)
+	defer datBackend.Close()
+
+	got, err := ReadSuperBlock(datBackend)
+	if err != nil {
+		t.Fatalf("ReadSuperBlock: %v", err)
+	}
+	if got.ExtraSize != s.ExtraSize {
+		t.Errorf("ExtraSize = %d, want %d", got.ExtraSize, s.ExtraSize)
+	}
+	if !proto.Equal(got.Extra, s.Extra) {
+		t.Errorf("Extra = %v, want %v", got.Extra, s.Extra)
+	}
+}
+
+// io.ReaderAt may return io.EOF on a full read; a remote backend can pass that through.
+type eofBackendStorageFile struct {
+	backend.DiskFile
+	data []byte
+}
+
+func (f *eofBackendStorageFile) ReadAt(p []byte, off int64) (n int, err error) {
+	if off >= int64(len(f.data)) {
+		return 0, io.EOF
+	}
+	return copy(p, f.data[off:]), io.EOF
+}
+
+func TestReadSuperBlockExtraFullReadWithEOF(t *testing.T) {
+	rp, _ := NewReplicaPlacementFromByte(byte(001))
+	ttl, _ := needle.ReadTTL("15d")
+	s := &SuperBlock{
+		Version:          needle.GetCurrentVersion(),
+		ReplicaPlacement: rp,
+		Ttl:              ttl,
+		Extra: &master_pb.SuperBlockExtra{
+			ErasureCoding: &master_pb.SuperBlockExtra_ErasureCoding{
+				Data:      10,
+				Parity:    4,
+				VolumeIds: []uint32{1, 2, 3},
+			},
+		},
+	}
+
+	got, err := ReadSuperBlock(&eofBackendStorageFile{data: s.Bytes()})
+	if err != nil {
+		t.Fatalf("ReadSuperBlock: %v", err)
+	}
+	if !proto.Equal(got.Extra, s.Extra) {
+		t.Errorf("Extra = %v, want %v", got.Extra, s.Extra)
+	}
+}
+
+func TestReadSuperBlockTruncatedExtra(t *testing.T) {
+	rp, _ := NewReplicaPlacementFromByte(byte(001))
+	ttl, _ := needle.ReadTTL("15d")
+	s := &SuperBlock{
+		Version:          needle.GetCurrentVersion(),
+		ReplicaPlacement: rp,
+		Ttl:              ttl,
+		Extra: &master_pb.SuperBlockExtra{
+			ErasureCoding: &master_pb.SuperBlockExtra_ErasureCoding{
+				Data:      10,
+				Parity:    4,
+				VolumeIds: []uint32{1, 2, 3},
+			},
+		},
+	}
+
+	data := s.Bytes()
+	datPath := filepath.Join(t.TempDir(), "1.dat")
+	if err := os.WriteFile(datPath, data[:len(data)-1], 0644); err != nil {
+		t.Fatal(err)
+	}
+	f, err := os.Open(datPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	datBackend := backend.NewDiskFile(f)
+	defer datBackend.Close()
+
+	_, err = ReadSuperBlock(datBackend)
+	if err == nil || !strings.Contains(err.Error(), "cannot read") {
+		t.Fatalf("expected read error for truncated extra, got %v", err)
+	}
+}