[#284] *: Return error from all ReadFromV2 methods

Return `error` from all `ReadFromV2` methods in order to support
backward compatibility if message will be extended with some formatted
field.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>

accounting/decimal.go

@@ -13,11 +13,13 @@ import "github.com/nspcc-dev/neofs-api-go/v2/accounting"
 // 	_ = Decimal(accounting.Decimal{}) // not recommended
 type Decimal accounting.Decimal
 
-// ReadFromV2 reads Decimal from the accounting.Decimal message.
+// ReadFromV2 reads Decimal from the accounting.Decimal message. Checks if the
+// message conforms to NeoFS API V2 protocol.
 //
 // See also WriteToV2.
-func (d *Decimal) ReadFromV2(m accounting.Decimal) {
+func (d *Decimal) ReadFromV2(m accounting.Decimal) error {
 	*d = Decimal(m)
+	return nil
 }
 
 // WriteToV2 writes Decimal to the accounting.Decimal message.

accounting/decimal_test.go

@@ -32,7 +32,7 @@ func TestDecimalMessageV2(t *testing.T) {
 	m.SetValue(7)
 	m.SetPrecision(8)
 
-	d.ReadFromV2(m)
+	require.NoError(t, d.ReadFromV2(m))
 
 	require.EqualValues(t, m.GetValue(), d.Value())
 	require.EqualValues(t, m.GetPrecision(), d.Precision())

bearer/bearer.go

@@ -279,10 +279,9 @@ func (b Token) VerifySignature() bool {
 	}
 
 	var sig neofscrypto.Signature
-	sig.ReadFromV2(b.sig)
 
 	// TODO: (#233) check owner<->key relation
-	return sig.Verify(b.signedData())
+	return sig.ReadFromV2(b.sig) == nil && sig.Verify(b.signedData())
 }
 
 // Marshal encodes Token into a binary format of the NeoFS API protocol

checksum/checksum.go

@@ -3,6 +3,7 @@ package checksum
 import (
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"fmt"
 
 	"github.com/nspcc-dev/neofs-api-go/v2/refs"
@@ -35,11 +36,24 @@ const (
 	TZ
 )
 
-// ReadFromV2 reads Checksum from the refs.Checksum message.
+// ReadFromV2 reads Checksum from the refs.Checksum message. Checks if the
+// message conforms to NeoFS API V2 protocol.
 //
 // See also WriteToV2.
-func (c *Checksum) ReadFromV2(m refs.Checksum) {
+func (c *Checksum) ReadFromV2(m refs.Checksum) error {
+	if len(m.GetSum()) == 0 {
+		return errors.New("missing value")
+	}
+
+	switch m.GetType() {
+	default:
+		return fmt.Errorf("unsupported type %v", m.GetType())
+	case refs.SHA256, refs.TillichZemor:
+	}
+
 	*c = Checksum(m)
+
+	return nil
 }
 
 // WriteToV2 writes Checksum to the refs.Checksum message.

client/accounting.go

@@ -2,6 +2,8 @@ package client
 
 import (
 	"context"
+	"errors"
+	"fmt"
 
 	v2accounting "github.com/nspcc-dev/neofs-api-go/v2/accounting"
 	"github.com/nspcc-dev/neofs-api-go/v2/refs"
@@ -94,13 +96,22 @@ func (c *Client) BalanceGet(ctx context.Context, prm PrmBalanceGet) (*ResBalance
 	cc.result = func(r responseV2) {
 		resp := r.(*v2accounting.BalanceResponse)
 
-		if bal := resp.GetBody().GetBalance(); bal != nil {
+		bal := resp.GetBody().GetBalance()
+		if bal == nil {
+			cc.err = errors.New("missing balance field")
+			return
+		}
+
 		var d accounting.Decimal
-			d.ReadFromV2(*bal)
+
+		cc.err = d.ReadFromV2(*bal)
+		if cc.err != nil {
+			cc.err = fmt.Errorf("invalid balance: %w", cc.err)
+			return
+		}
 
 		res.setAmount(&d)
 	}
-	}
 
 	// process call
 	if !cc.processCall() {

client/netmap.go

@@ -95,7 +95,11 @@ func (c *Client) EndpointInfo(ctx context.Context, prm PrmEndpointInfo) (*ResEnd
 
 		var ver version.Version
 		if v2ver := body.GetVersion(); v2ver != nil {
-			ver.ReadFromV2(*v2ver)
+			cc.err = ver.ReadFromV2(*v2ver)
+			if cc.err != nil {
+				cc.err = fmt.Errorf("invalid version: %w", cc.err)
+				return
+			}
 		}
 		res.setLatestVersion(&ver)
 

container/container_test.go

@@ -63,7 +63,7 @@ func TestContainer_Init(t *testing.T) {
 	require.NotNil(t, verV2)
 
 	var ver version.Version
-	ver.ReadFromV2(*verV2)
+	require.NoError(t, ver.ReadFromV2(*verV2))
 
 	require.Equal(t, version.Current(), ver)
 
@@ -345,7 +345,7 @@ func TestCalculateSignature(t *testing.T) {
 	sig.WriteToV2(&msg)
 
 	var sig2 neofscrypto.Signature
-	sig2.ReadFromV2(msg)
+	require.NoError(t, sig2.ReadFromV2(msg))
 
 	require.True(t, container.VerifySignature(sig2, val))
 }

crypto/crypto_test.go

@@ -39,7 +39,7 @@ func TestSignature(t *testing.T) {
 
 		s.WriteToV2(&m)
 
-		s.ReadFromV2(m)
+		require.NoError(t, s.ReadFromV2(m))
 
 		valid := s.Verify(data)
 		require.True(t, valid, "type %T", signer)

crypto/signature.go

@@ -1,6 +1,7 @@
 package neofscrypto
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/nspcc-dev/neofs-api-go/v2/refs"
@@ -16,11 +17,29 @@ import (
 // 	_ = Signature(refs.Signature{}) // not recommended
 type Signature refs.Signature
 
-// ReadFromV2 reads Signature from the refs.Signature message.
+// ReadFromV2 reads Signature from the refs.Signature message. Checks if the
+// message conforms to NeoFS API V2 protocol.
 //
 // See also WriteToV2.
-func (x *Signature) ReadFromV2(m refs.Signature) {
+func (x *Signature) ReadFromV2(m refs.Signature) error {
+	if len(m.GetKey()) == 0 {
+		return errors.New("missing public key")
+	} else if len(m.GetSign()) == 0 {
+		return errors.New("missing signature")
+	}
+
+	switch m.GetScheme() {
+	default:
+		return fmt.Errorf("unsupported scheme %v", m.GetSign())
+	case
+		refs.ECDSA_SHA512,
+		refs.ECDSA_RFC6979_SHA256,
+		refs.ECDSA_RFC6979_SHA256_WALLET_CONNECT:
+	}
+
 	*x = Signature(m)
+
+	return nil
 }
 
 // WriteToV2 writes Signature to the refs.Signature message.

object/fmt.go

@@ -127,9 +127,8 @@ func (o *Object) VerifyIDSignature() bool {
 	}
 
 	var sig neofscrypto.Signature
-	sig.ReadFromV2(*sigV2)
 
-	return sig.Verify(idV2.StableMarshal(nil))
+	return sig.ReadFromV2(*sigV2) == nil && sig.Verify(idV2.StableMarshal(nil))
 }
 
 // SetIDWithSignature sets object identifier and signature.

object/object.go

@@ -117,7 +117,7 @@ func (o *Object) Signature() *neofscrypto.Signature {
 	}
 
 	var sig neofscrypto.Signature
-	sig.ReadFromV2(*sigv2)
+	sig.ReadFromV2(*sigv2) // FIXME(@cthulhu-rider): #226 handle error
 
 	return &sig
 }
@@ -149,7 +149,7 @@ func (o *Object) SetPayload(v []byte) {
 func (o *Object) Version() *version.Version {
 	var ver version.Version
 	if verV2 := (*object.Object)(o).GetHeader().GetVersion(); verV2 != nil {
-		ver.ReadFromV2(*verV2)
+		ver.ReadFromV2(*verV2) // FIXME(@cthulhu-rider): #226 handle error
 	}
 	return &ver
 }
@@ -248,7 +248,7 @@ func (o *Object) PayloadChecksum() (checksum.Checksum, bool) {
 	v2 := (*object.Object)(o)
 
 	if hash := v2.GetHeader().GetPayloadHash(); hash != nil {
-		v.ReadFromV2(*hash)
+		v.ReadFromV2(*hash) // FIXME(@cthulhu-rider): #226 handle error
 		return v, true
 	}
 
@@ -278,7 +278,7 @@ func (o *Object) PayloadHomomorphicHash() (checksum.Checksum, bool) {
 	v2 := (*object.Object)(o)
 
 	if hash := v2.GetHeader().GetHomomorphicHash(); hash != nil {
-		v.ReadFromV2(*hash)
+		v.ReadFromV2(*hash) // FIXME(@cthulhu-rider): #226 handle error
 		return v, true
 	}
 

reputation/trust.go

@@ -394,9 +394,8 @@ func (x GlobalTrust) VerifySignature() bool {
 	}
 
 	var sig neofscrypto.Signature
-	sig.ReadFromV2(*sigV2)
 
-	return sig.Verify(x.m.GetBody().StableMarshal(nil))
+	return sig.ReadFromV2(*sigV2) == nil && sig.Verify(x.m.GetBody().StableMarshal(nil))
 }
 
 // Marshal encodes GlobalTrust into a binary format of the NeoFS API protocol

session/common.go

@@ -178,10 +178,9 @@ func (x commonData) verifySignature(w contextWriter) bool {
 	}
 
 	var sig neofscrypto.Signature
-	sig.ReadFromV2(x.sig)
 
 	// TODO: (#233) check owner<->key relation
-	return sig.Verify(x.signedData(w))
+	return sig.ReadFromV2(x.sig) == nil && sig.Verify(x.signedData(w))
 }
 
 func (x commonData) marshal(w contextWriter) []byte {

session/container.go

@@ -210,7 +210,6 @@ func (x Container) VerifySessionDataSignature(data, signature []byte) bool {
 	sigV2.SetSign(signature)
 
 	var sig neofscrypto.Signature
-	sig.ReadFromV2(sigV2)
 
-	return sig.Verify(data)
+	return sig.ReadFromV2(sigV2) == nil && sig.Verify(data)
 }

storagegroup/storagegroup.go

@@ -1,6 +1,7 @@
 package storagegroup
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 
@@ -23,11 +24,55 @@ import (
 // 	_ = StorageGroup(storagegroup.StorageGroup) // not recommended
 type StorageGroup storagegroup.StorageGroup
 
+// reads StorageGroup from the storagegroup.StorageGroup message. If checkFieldPresence is set,
+// returns an error on absence of any protocol-required field.
+func (sg *StorageGroup) readFromV2(m storagegroup.StorageGroup, checkFieldPresence bool) error {
+	var err error
+
+	h := m.GetValidationHash()
+	if h != nil {
+		err = new(checksum.Checksum).ReadFromV2(*h)
+		if err != nil {
+			return fmt.Errorf("invalid hash: %w", err)
+		}
+	} else if checkFieldPresence {
+		return errors.New("missing hash")
+	}
+
+	members := m.GetMembers()
+	if len(members) > 0 {
+		var member oid.ID
+		mMembers := make(map[oid.ID]struct{}, len(members))
+		var exits bool
+
+		for i := range members {
+			err = member.ReadFromV2(members[i])
+			if err != nil {
+				return fmt.Errorf("invalid member: %w", err)
+			}
+
+			_, exits = mMembers[member]
+			if exits {
+				return fmt.Errorf("duplicated member %s", member)
+			}
+
+			mMembers[member] = struct{}{}
+		}
+	} else if checkFieldPresence {
+		return errors.New("missing members")
+	}
+
+	*sg = StorageGroup(m)
+
+	return nil
+}
+
 // ReadFromV2 reads StorageGroup from the storagegroup.StorageGroup message.
+// Checks if the message conforms to NeoFS API V2 protocol.
 //
 // See also WriteToV2.
-func (sg *StorageGroup) ReadFromV2(m storagegroup.StorageGroup) {
+func (sg *StorageGroup) ReadFromV2(m storagegroup.StorageGroup) error {
-	*sg = StorageGroup(m)
+	return sg.readFromV2(m, true)
 }
 
 // WriteToV2 writes StorageGroup to the storagegroup.StorageGroup message.
@@ -68,7 +113,7 @@ func (sg *StorageGroup) SetValidationDataSize(epoch uint64) {
 func (sg StorageGroup) ValidationDataHash() (v checksum.Checksum, isSet bool) {
 	v2 := (storagegroup.StorageGroup)(sg)
 	if checksumV2 := v2.GetValidationHash(); checksumV2 != nil {
-		v.ReadFromV2(*checksumV2)
+		v.ReadFromV2(*checksumV2) // FIXME(@cthulhu-rider): #226 handle error
 		isSet = true
 	}
 
@@ -174,7 +219,7 @@ func (sg *StorageGroup) Unmarshal(data []byte) error {
 		return err
 	}
 
-	return formatCheck(v2)
+	return sg.readFromV2(*v2, false)
 }
 
 // MarshalJSON encodes StorageGroup to protobuf JSON format.
@@ -195,20 +240,7 @@ func (sg *StorageGroup) UnmarshalJSON(data []byte) error {
 		return err
 	}
 
-	return formatCheck(v2)
+	return sg.readFromV2(*v2, false)
-}
-
-func formatCheck(v2 *storagegroup.StorageGroup) error {
-	var oID oid.ID
-
-	for _, m := range v2.GetMembers() {
-		err := oID.ReadFromV2(m)
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
 }
 
 // ReadFromObject assemble StorageGroup from a regular

storagegroup/storagegroup_test.go

@@ -49,13 +49,7 @@ func TestStorageGroup_ReadFromV2(t *testing.T) {
 			v2 storagegroupV2.StorageGroup
 		)
 
-		x.ReadFromV2(v2)
+		require.Error(t, x.ReadFromV2(v2))
-
-		require.Zero(t, x.ExpirationEpoch())
-		require.Zero(t, x.ValidationDataSize())
-		_, set := x.ValidationDataHash()
-		require.False(t, set)
-		require.Zero(t, x.Members())
 	})
 
 	t.Run("from non-zero", func(t *testing.T) {
@@ -72,13 +66,13 @@ func TestStorageGroup_ReadFromV2(t *testing.T) {
 		mm := v2.GetMembers()
 		hashV2 := v2.GetValidationHash()
 
-		x.ReadFromV2(*v2)
+		require.NoError(t, x.ReadFromV2(*v2))
 
 		require.Equal(t, epoch, x.ExpirationEpoch())
 		require.Equal(t, size, x.ValidationDataSize())
 
 		var hash checksum.Checksum
-		hash.ReadFromV2(*hashV2)
+		require.NoError(t, hash.ReadFromV2(*hashV2))
 		h, set := x.ValidationDataHash()
 		require.True(t, set)
 		require.Equal(t, hash, h)
@@ -143,7 +137,7 @@ func TestStorageGroup_WriteToV2(t *testing.T) {
 		require.Equal(t, x.ValidationDataSize(), v2.GetValidationDataSize())
 
 		var hash checksum.Checksum
-		hash.ReadFromV2(*v2.GetValidationHash())
+		require.NoError(t, hash.ReadFromV2(*v2.GetValidationHash()))
 
 		h, set := x.ValidationDataHash()
 		require.True(t, set)

version/version.go

@@ -55,11 +55,13 @@ func (v Version) WriteToV2(m *refs.Version) {
 	*m = (refs.Version)(v)
 }
 
-// ReadFromV2 reads Version from the refs.Version message.
+// ReadFromV2 reads Version from the refs.Version message. Checks if the message
+// conforms to NeoFS API V2 protocol.
 //
 // See also WriteToV2.
-func (v *Version) ReadFromV2(m refs.Version) {
+func (v *Version) ReadFromV2(m refs.Version) error {
 	*v = Version(m)
+	return nil
 }
 
 // String implements fmt.Stringer.