From 55a0a1c9c2a6df4f05d539f0fc67a8c7052a338f Mon Sep 17 00:00:00 2001 From: Rasmus Dahlberg Date: Thu, 18 Feb 2021 17:51:05 +0100 Subject: fixed unexpected reflect behavior After tls.Unmarshal() an empty slice is not assigned the nil value, but rather a slice of zero length. It is in contrast to NewTreeHeadV1(), which assigns a nil value. Therefore, reflect.DeepEqual() considers them to be different. Fixed by assigning an empty tree head extension as `make([]byte, 0)`, and not looking for nil values but rather zero-length values with len(). Further read: "Note that a non-nil empty slice and a nil slice [...] are not deeply equal." (https://golang.org/pkg/reflect/#DeepEqual). --- crypto_test.go | 4 ++-- sth.go | 14 +------------- type.go | 2 +- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/crypto_test.go b/crypto_test.go index cfbb0a8..75e530e 100644 --- a/crypto_test.go +++ b/crypto_test.go @@ -118,8 +118,8 @@ func TestGenV1Sth(t *testing.T) { if got, want := sth.TreeHead.RootHash.Data, th.RootHash.Data; !bytes.Equal(got, want) { t.Errorf("got root hash %X, wanted %X in test %q", got, want, table.description) } - if sth.TreeHead.Extension != nil { - t.Errorf("got extensions %X, wanted nil in test %q", sth.TreeHead.Extension, table.description) + if len(sth.TreeHead.Extension) != 0 { + t.Errorf("got extensions %X, wanted none in test %q", sth.TreeHead.Extension, table.description) } } } diff --git a/sth.go b/sth.go index e251b98..5cf3413 100644 --- a/sth.go +++ b/sth.go @@ -1,7 +1,6 @@ package stfe import ( - "bytes" "context" "fmt" "reflect" @@ -91,21 +90,10 @@ func (s *ActiveSthSource) Cosigned(_ context.Context) (*StItem, error) { return s.currSth, nil } -func (a *SignedTreeHeadV1) Equals(b *SignedTreeHeadV1) bool { - return bytes.Equal(a.LogId, b.LogId) && - bytes.Equal(a.Signature, b.Signature) && - a.TreeHead.Timestamp == b.TreeHead.Timestamp && - a.TreeHead.TreeSize == b.TreeHead.TreeSize && - bytes.Equal(a.TreeHead.RootHash.Data, b.TreeHead.RootHash.Data) && - bytes.Equal(a.TreeHead.Extension, b.TreeHead.Extension) - // TODO: why reflect.DeepEqual(a, b) gives a different result? Fixme. -} - func (s *ActiveSthSource) AddCosignature(_ context.Context, costh *StItem) error { s.mutex.Lock() defer s.mutex.Unlock() - //if !reflect.DeepEqual(s.nextSth.CosignedTreeHeadV1.SignedTreeHeadV1, costh.CosignedTreeHeadV1.SignedTreeHeadV1) { - if !(&s.nextSth.CosignedTreeHeadV1.SignedTreeHeadV1).Equals(&costh.CosignedTreeHeadV1.SignedTreeHeadV1) { + if !reflect.DeepEqual(s.nextSth.CosignedTreeHeadV1.SignedTreeHeadV1, costh.CosignedTreeHeadV1.SignedTreeHeadV1) { return fmt.Errorf("cosignature covers a different tree head") } witness := costh.CosignedTreeHeadV1.SignatureV1[0].Namespace.String() diff --git a/type.go b/type.go index 72aeecc..db9ddc6 100644 --- a/type.go +++ b/type.go @@ -282,7 +282,7 @@ func NewTreeHeadV1(lr *types.LogRootV1) *TreeHeadV1 { uint64(lr.TimestampNanos / 1000 / 1000), uint64(lr.TreeSize), NodeHash{lr.RootHash}, - nil, + make([]byte, 0), } } -- cgit v1.2.3