From 978d374820098ed509de4f08ff96afc49da79093 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 16 Sep 2014 09:32:53 +0100 Subject: [PATCH 1/5] Change iteration over connection decoding to abort after the first error, and let the UnknownPeersError know which peer was unknown --- peer_cache.go | 28 +++++++++++++++------------- types.go | 1 + utils.go | 2 +- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/peer_cache.go b/peer_cache.go index 390a164c..69e785bb 100644 --- a/peer_cache.go +++ b/peer_cache.go @@ -177,24 +177,22 @@ func (cache *PeerCache) decodeUpdate(update []byte, router *Router) (newPeers ma } } - unknownPeers := false for _, connsBuf := range decodedConns { - connsIterator(connsBuf, func(remoteNameByte []byte, _ string) { + err = connsIterator(connsBuf, func(remoteNameByte []byte, _ string) error { remoteName := PeerNameFromBin(remoteNameByte) if _, found := newPeers[remoteName]; found { - return + return nil } if _, found := cache.table[remoteName]; found { - return + return nil } // Update refers to a peer which we have no knowledge // of. Thus we can't apply the update. Abort. - unknownPeers = true + return UnknownPeersError{remoteName} }) - } - if unknownPeers { - err = UnknownPeersError{} - return + if err != nil { + break + } } return } @@ -282,7 +280,7 @@ func decodePeerNoConns(dec *gob.Decoder) (nameByte []byte, uid uint64, version u return } -func connsIterator(input []byte, fun func([]byte, string)) { +func connsIterator(input []byte, fun func([]byte, string) error) error { buf := new(bytes.Buffer) buf.Write(input) dec := gob.NewDecoder(buf) @@ -290,22 +288,26 @@ func connsIterator(input []byte, fun func([]byte, string)) { var nameByte []byte err := dec.Decode(&nameByte) if err == io.EOF { - return + return nil } checkFatal(err) var foundAt string checkFatal(dec.Decode(&foundAt)) - fun(nameByte, string(foundAt)) + if err = fun(nameByte, string(foundAt)); err != nil { + return err + } } + return nil } func readConnsMap(peer *Peer, buf []byte, table map[PeerName]*Peer) map[PeerName]Connection { conns := make(map[PeerName]Connection) - connsIterator(buf, func(nameByte []byte, remoteTCPAddr string) { + connsIterator(buf, func(nameByte []byte, remoteTCPAddr string) error { name := PeerNameFromBin(nameByte) remotePeer := table[name] conn := NewRemoteConnection(peer, remotePeer, remoteTCPAddr) conns[name] = conn + return nil }) return conns } diff --git a/types.go b/types.go index 020fba18..83daed99 100644 --- a/types.go +++ b/types.go @@ -123,6 +123,7 @@ type FrameTooBigError struct { } type UnknownPeersError struct { + Name PeerName } type NameCollisionError struct { diff --git a/utils.go b/utils.go index 22d7235b..72b8629d 100644 --- a/utils.go +++ b/utils.go @@ -39,7 +39,7 @@ func (ftbe FrameTooBigError) Error() string { } func (upe UnknownPeersError) Error() string { - return fmt.Sprint("Reference to unknown peers") + return fmt.Sprint("Reference to unknown peer: ", upe.Name) } func (nce NameCollisionError) Error() string { From 752cfdcb6770945ca554d20392d1545cf7a59edd Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 16 Sep 2014 11:11:54 +0100 Subject: [PATCH 2/5] Make message for UnknownPeersError more meaningful --- connection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connection.go b/connection.go index 68c337ee..08168870 100644 --- a/connection.go +++ b/connection.go @@ -481,7 +481,7 @@ func (conn *LocalConnection) receiveTCP(decoder *gob.Decoder, usingPassword bool } else if msg[0] == ProtocolUpdate { newUpdate, err := conn.Router.Peers.ApplyUpdate(msg[1:], conn.Router) if _, ok := err.(UnknownPeersError); err != nil && ok { - checkWarn(err) + log.Println("Update from", conn.RemoteTCPAddr(), "contained unknown peer; requesting full update") conn.SendTCP(ProtocolFetchAllByte) continue } else if conn.CheckFatal(err) != nil { From 4e7aa85de080551a101b256bb8489be3f9610f40 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 17 Sep 2014 13:18:07 +0100 Subject: [PATCH 3/5] Revert "Make message for UnknownPeersError more meaningful" This reverts commit 752cfdcb6770945ca554d20392d1545cf7a59edd. --- connection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connection.go b/connection.go index c08bd5f0..a7dfdfa4 100644 --- a/connection.go +++ b/connection.go @@ -481,7 +481,7 @@ func (conn *LocalConnection) receiveTCP(decoder *gob.Decoder, usingPassword bool } else if msg[0] == ProtocolUpdate { newUpdate, err := conn.Router.Peers.ApplyUpdate(msg[1:], conn.Router) if _, ok := err.(UnknownPeersError); err != nil && ok { - log.Println("Update from", conn.RemoteTCPAddr(), "contained unknown peer; requesting full update") + checkWarn(err) conn.SendTCP(ProtocolFetchAllByte) continue } else if conn.CheckFatal(err) != nil { From c8876e144ffe67347e6b27d6a676ef19ac7f1fe2 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 17 Sep 2014 13:18:33 +0100 Subject: [PATCH 4/5] Revert "Change iteration over connection decoding to abort after the first error, and let the UnknownPeersError know which peer was unknown" This reverts commit 978d374820098ed509de4f08ff96afc49da79093. --- peer_cache.go | 28 +++++++++++++--------------- types.go | 1 - utils.go | 2 +- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/peer_cache.go b/peer_cache.go index d9910c95..a499d5ae 100644 --- a/peer_cache.go +++ b/peer_cache.go @@ -177,22 +177,24 @@ func (cache *PeerCache) decodeUpdate(update []byte, router *Router) (newPeers ma } } + unknownPeers := false for _, connsBuf := range decodedConns { - err = connsIterator(connsBuf, func(remoteNameByte []byte, _ string) error { + connsIterator(connsBuf, func(remoteNameByte []byte, _ string) { remoteName := PeerNameFromBin(remoteNameByte) if _, found := newPeers[remoteName]; found { - return nil + return } if _, found := cache.table[remoteName]; found { - return nil + return } // Update refers to a peer which we have no knowledge // of. Thus we can't apply the update. Abort. - return UnknownPeersError{remoteName} + unknownPeers = true }) - if err != nil { - break - } + } + if unknownPeers { + err = UnknownPeersError{} + return } return } @@ -280,7 +282,7 @@ func decodePeerNoConns(dec *gob.Decoder) (nameByte []byte, uid uint64, version u return } -func connsIterator(input []byte, fun func([]byte, string) error) error { +func connsIterator(input []byte, fun func([]byte, string)) { buf := new(bytes.Buffer) buf.Write(input) dec := gob.NewDecoder(buf) @@ -288,26 +290,22 @@ func connsIterator(input []byte, fun func([]byte, string) error) error { var nameByte []byte err := dec.Decode(&nameByte) if err == io.EOF { - return nil + return } checkFatal(err) var foundAt string checkFatal(dec.Decode(&foundAt)) - if err = fun(nameByte, string(foundAt)); err != nil { - return err - } + fun(nameByte, string(foundAt)) } - return nil } func readConnsMap(peer *Peer, buf []byte, table map[PeerName]*Peer) map[PeerName]Connection { conns := make(map[PeerName]Connection) - connsIterator(buf, func(nameByte []byte, remoteTCPAddr string) error { + connsIterator(buf, func(nameByte []byte, remoteTCPAddr string) { name := PeerNameFromBin(nameByte) remotePeer := table[name] conn := NewRemoteConnection(peer, remotePeer, remoteTCPAddr) conns[name] = conn - return nil }) return conns } diff --git a/types.go b/types.go index c4807582..093fa6d9 100644 --- a/types.go +++ b/types.go @@ -123,7 +123,6 @@ type FrameTooBigError struct { } type UnknownPeersError struct { - Name PeerName } type NameCollisionError struct { diff --git a/utils.go b/utils.go index 6979b158..da57cf2a 100644 --- a/utils.go +++ b/utils.go @@ -39,7 +39,7 @@ func (ftbe FrameTooBigError) Error() string { } func (upe UnknownPeersError) Error() string { - return fmt.Sprint("Reference to unknown peer: ", upe.Name) + return fmt.Sprint("Reference to unknown peers") } func (nce NameCollisionError) Error() string { From 8bac04d080119dcdcf049dfe03f82f84eeafb182 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 17 Sep 2014 13:19:52 +0100 Subject: [PATCH 5/5] Remove message about unknown peer from the log --- connection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connection.go b/connection.go index a7dfdfa4..818e928a 100644 --- a/connection.go +++ b/connection.go @@ -481,7 +481,7 @@ func (conn *LocalConnection) receiveTCP(decoder *gob.Decoder, usingPassword bool } else if msg[0] == ProtocolUpdate { newUpdate, err := conn.Router.Peers.ApplyUpdate(msg[1:], conn.Router) if _, ok := err.(UnknownPeersError); err != nil && ok { - checkWarn(err) + // That update contained a peer we didn't know about; request full update conn.SendTCP(ProtocolFetchAllByte) continue } else if conn.CheckFatal(err) != nil {