Kick connections that contribute the final piece of missing metadata that fails to...

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.

diff --git a/client.go b/client.go
index 717604766261eab6d8a09152aebfdfb14d7b2f37..d1654f8f186f974fe2a152e5b67f267bf2d42966 100644 (file)
--- 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 039e572dc2b2bb92b1a884ecc8cdacd002c03180..147c3a98b1e6585a32b623b0b1390ce06989a82e 100644 (file)
--- 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) {