From 94435fbc73145fb85a4ec647e74b193c57a86045 Mon Sep 17 00:00:00 2001
From: Jeffrey Wilcke <geffobscura@gmail.com>
Date: Fri, 20 Nov 2015 19:23:55 +0100
Subject: [PATCH] whisper: fixed broadcast race

Whisper's expire and broadcast loops happen in two separate go routines.
Whenever an envelope is being expired it's removed from the set of
envelopes and it looses all information about the envelope, including
the "known hash". After the envelope has been removed it can be
re-accepted by a broadcasting peer putting back the envelope in the set
of envelopes. Since the envelope broadcast loop is separate of the
expire loop expired messages may be broadcast to other peer, resulting
in messages **never** being dropped.

This PR includes an expire check before adding new messages to the set
of envelopes.
---
 peer_test.go    | 7 +++++--
 whisper.go      | 5 +++++
 whisper_test.go | 9 +++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/peer_test.go b/peer_test.go
index 594671e..b3d2031 100644
--- a/peer_test.go
+++ b/peer_test.go
@@ -239,14 +239,17 @@ func TestPeerMessageExpiration(t *testing.T) {
 	}
 	payload := []interface{}{envelope}
 	if err := p2p.ExpectMsg(tester.stream, messagesCode, payload); err != nil {
-		t.Fatalf("message mismatch: %v", err)
+		// A premature empty message may have been broadcast, check the next too
+		if err := p2p.ExpectMsg(tester.stream, messagesCode, payload); err != nil {
+			t.Fatalf("message mismatch: %v", err)
+		}
 	}
 	// Check that the message is inside the cache
 	if !peer.known.Has(envelope.Hash()) {
 		t.Fatalf("message not found in cache")
 	}
 	// Discard messages until expiration and check cache again
-	exp := time.Now().Add(time.Second + expirationCycle)
+	exp := time.Now().Add(time.Second + 2*expirationCycle + 100*time.Millisecond)
 	for time.Now().Before(exp) {
 		if err := p2p.ExpectMsg(tester.stream, messagesCode, []interface{}{}); err != nil {
 			t.Fatalf("message mismatch: %v", err)
diff --git a/whisper.go b/whisper.go
index 676d8ae..a341f23 100644
--- a/whisper.go
+++ b/whisper.go
@@ -234,6 +234,11 @@ func (self *Whisper) add(envelope *Envelope) error {
 	self.poolMu.Lock()
 	defer self.poolMu.Unlock()
 
+	// short circuit when a received envelope has already expired
+	if envelope.Expiry <= uint32(time.Now().Unix()) {
+		return nil
+	}
+
 	// Insert the message into the tracked pool
 	hash := envelope.Hash()
 	if _, ok := self.messages[hash]; ok {
diff --git a/whisper_test.go b/whisper_test.go
index 1a9a866..b83ce0f 100644
--- a/whisper_test.go
+++ b/whisper_test.go
@@ -207,4 +207,13 @@ func TestMessageExpiration(t *testing.T) {
 	if found {
 		t.Fatalf("message not expired from cache")
 	}
+
+	node.add(envelope)
+	node.poolMu.RLock()
+	_, found = node.messages[envelope.Hash()]
+	node.poolMu.RUnlock()
+	if found {
+		t.Fatalf("message was added to cache")
+	}
+
 }