From 3c75f684132cba00cbf92fdffb99fd10488509a3 Mon Sep 17 00:00:00 2001
From: Matt Joiner <anacrolix@gmail.com>
Date: Sun, 24 Jul 2016 00:34:40 +1000
Subject: [PATCH] Kick connections that contribute the final piece of missing
 metadata that fails to validate

Failing to get metainfo was resetting Torrent.metadataBytes, which caused arriving metadata data to appear to belong to nonexistent pieces, despite passing the check that we'd in fact requested them from the sending connection. This was unnecessarily noisy. Instead return an error if we got all the data, but couldn't set the Torrent's info bytes, that should propagate out and cause the connection that contributed the final missing piece to be dropped. This will at least provide some improved resistance to unfriendly behaviour on the network.
---
 client.go  | 25 +++++++++++--------------
 torrent.go | 19 ++++++++++---------
 2 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/client.go b/client.go
index 71760476..d1654f8f 100644
--- a/client.go
+++ b/client.go
@@ -1028,47 +1028,44 @@ func (cl *Client) connDeleteRequest(t *Torrent, cn *connection, r request) bool
 }
 
 // Process incoming ut_metadata message.
-func (cl *Client) gotMetadataExtensionMsg(payload []byte, t *Torrent, c *connection) (err error) {
+func (cl *Client) gotMetadataExtensionMsg(payload []byte, t *Torrent, c *connection) error {
 	var d map[string]int
-	err = bencode.Unmarshal(payload, &d)
+	err := bencode.Unmarshal(payload, &d)
 	if err != nil {
-		err = fmt.Errorf("error unmarshalling payload: %s: %q", err, payload)
-		return
+		return fmt.Errorf("error unmarshalling payload: %s: %q", err, payload)
 	}
 	msgType, ok := d["msg_type"]
 	if !ok {
-		err = errors.New("missing msg_type field")
-		return
+		return errors.New("missing msg_type field")
 	}
 	piece := d["piece"]
 	switch msgType {
 	case pp.DataMetadataExtensionMsgType:
 		if !c.requestedMetadataPiece(piece) {
-			err = fmt.Errorf("got unexpected piece %d", piece)
-			return
+			return fmt.Errorf("got unexpected piece %d", piece)
 		}
 		c.metadataRequests[piece] = false
 		begin := len(payload) - metadataPieceSize(d["total_size"], piece)
 		if begin < 0 || begin >= len(payload) {
-			err = fmt.Errorf("data has bad offset in payload: %d", begin)
-			return
+			return fmt.Errorf("data has bad offset in payload: %d", begin)
 		}
 		t.saveMetadataPiece(piece, payload[begin:])
 		c.UsefulChunksReceived++
 		c.lastUsefulChunkReceived = time.Now()
-		t.maybeMetadataCompleted()
+		return t.maybeCompleteMetadata()
 	case pp.RequestMetadataExtensionMsgType:
 		if !t.haveMetadataPiece(piece) {
 			c.Post(t.newMetadataExtensionMessage(c, pp.RejectMetadataExtensionMsgType, d["piece"], nil))
-			break
+			return nil
 		}
 		start := (1 << 14) * piece
 		c.Post(t.newMetadataExtensionMessage(c, pp.DataMetadataExtensionMsgType, piece, t.metadataBytes[start:start+t.metadataPieceSize(piece)]))
+		return nil
 	case pp.RejectMetadataExtensionMsgType:
+		return nil
 	default:
-		err = errors.New("unknown msg_type value")
+		return errors.New("unknown msg_type value")
 	}
-	return
 }
 
 func (cl *Client) upload(t *Torrent, c *connection) {
diff --git a/torrent.go b/torrent.go
index 039e572d..147c3a98 100644
--- a/torrent.go
+++ b/torrent.go
@@ -163,8 +163,9 @@ func (t *Torrent) addPeer(p Peer) {
 }
 
 func (t *Torrent) invalidateMetadata() {
-	t.metadataBytes = nil
-	t.metadataCompletedChunks = nil
+	for i := range t.metadataCompletedChunks {
+		t.metadataCompletedChunks[i] = false
+	}
 	t.info = nil
 }
 
@@ -1027,26 +1028,26 @@ func (t *Torrent) updateAllPieceCompletions() {
 	}
 }
 
-func (t *Torrent) maybeMetadataCompleted() {
+// Returns an error if the metadata was completed, but couldn't be set for
+// some reason. Blame it on the last peer to contribute.
+func (t *Torrent) maybeCompleteMetadata() error {
 	if t.haveInfo() {
 		// Nothing to do.
-		return
+		return nil
 	}
 	if !t.haveAllMetadataPieces() {
 		// Don't have enough metadata pieces.
-		return
+		return nil
 	}
-	// TODO(anacrolix): If this fails, I think something harsher should be
-	// done.
 	err := t.setInfoBytes(t.metadataBytes)
 	if err != nil {
-		log.Printf("error setting metadata: %s", err)
 		t.invalidateMetadata()
-		return
+		return fmt.Errorf("error setting info bytes: %s", err)
 	}
 	if t.cl.config.Debug {
 		log.Printf("%s: got metadata from peers", t)
 	}
+	return nil
 }
 
 func (t *Torrent) readerPieces() (ret bitmap.Bitmap) {
-- 
2.48.1