Skip to content

Commit

Permalink
[FAB-3335] Gossip pull may send zero-length digests
Browse files Browse the repository at this point in the history
The bug was introduced in a recent commit that added filtering capability.

Basically when the gossip puller gets a hello from a peer,
it iterates over the items it has, and applies a filter on each of them.
If the filter doesn't permit, it skips to the next iteration,
but the returned slice then contains an "empty-string"
digest in the response to the remote peer.

While this isn't dangerous, this causes un-necessary messages
to be sent so this should be fixed.

Also added an optimization that if there are no digests/items to send,
we don't return any response to the sender.

Change-Id: Ic455413dc9e040ea98ef3bb0ee3531f2c6cfc32c
Signed-off-by: Yacov Manevich <yacovm@il.ibm.com>
  • Loading branch information
yacovm committed Apr 22, 2017
1 parent d661d11 commit 9b5c180
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions gossip/gossip/algo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,14 +291,17 @@ func (engine *PullEngine) OnHello(nonce uint64, context interface{}) {
})

a := engine.state.ToArray()
digest := make([]string, len(a))
var digest []string
filter := engine.digFilter(context)
for i, item := range a {
for _, item := range a {
dig := item.(string)
if !filter(dig) {
continue
}
digest[i] = dig
digest = append(digest, dig)
}
if len(digest) == 0 {
return
}
engine.SendDigest(digest, nonce, context)
}
Expand All @@ -309,6 +312,7 @@ func (engine *PullEngine) OnReq(items []string, nonce uint64, context interface{
return
}
engine.lock.Lock()
defer engine.lock.Unlock()

filter := engine.digFilter(context)
var items2Send []string
Expand All @@ -318,7 +322,9 @@ func (engine *PullEngine) OnReq(items []string, nonce uint64, context interface{
}
}

engine.lock.Unlock()
if len(items2Send) == 0 {
return
}

go engine.SendRes(items2Send, context, nonce)
}
Expand Down

0 comments on commit 9b5c180

Please sign in to comment.