From d429943d05726569758a884fa67238958972970f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 6 Jan 2011 22:43:34 +0900 Subject: [PATCH] Fixed the bug that peer is marked as seeder whenever it sends extension handshake with metadata size. Peer must be marked as seeder only when client has not got metadata yet. This bug causes aria2 shutdowns connection early when it gets have message from a peer because it wrongly recognizes the peer as seeder. --- src/HandshakeExtensionMessage.cc | 58 +++++++++++++-------------- src/Peer.cc | 1 + test/HandshakeExtensionMessageTest.cc | 8 ++++ 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/src/HandshakeExtensionMessage.cc b/src/HandshakeExtensionMessage.cc index 87605255..1fc85bf9 100644 --- a/src/HandshakeExtensionMessage.cc +++ b/src/HandshakeExtensionMessage.cc @@ -111,42 +111,38 @@ void HandshakeExtensionMessage::doReceivedAction() const std::map::value_type& vt = *itr; peer_->setExtension(vt.first, vt.second); } - SharedHandle attrs = - bittorrent::getTorrentAttrs(dctx_); - if(attrs->metadata.empty() && !peer_->getExtensionMessageID("ut_metadata")) { - // TODO In metadataGetMode, if peer don't support metadata - // transfer, should we drop connection? There is a possibility - // that peer can still tell us peers using PEX. - throw DL_ABORT_EX("Peer doesn't support ut_metadata extension. Goodbye."); - } - if(metadataSize_ > 0) { - if(attrs->metadataSize) { - if(metadataSize_ != attrs->metadataSize) { - throw DL_ABORT_EX("Wrong metadata_size. Which one is correct!?"); + SharedHandle attrs = bittorrent::getTorrentAttrs(dctx_); + if(attrs->metadata.empty()) { + if(!peer_->getExtensionMessageID("ut_metadata")) { + // TODO In metadataGetMode, if peer don't support metadata + // transfer, should we drop connection? There is a possibility + // that peer can still tell us peers using PEX. + throw DL_ABORT_EX("Peer doesn't support ut_metadata extension. Goodbye."); + } + if(metadataSize_ > 0) { + if(attrs->metadataSize) { + if(metadataSize_ != attrs->metadataSize) { + throw DL_ABORT_EX("Wrong metadata_size. Which one is correct!?"); + } + } else { + attrs->metadataSize = metadataSize_; + dctx_->getFirstFileEntry()->setLength(metadataSize_); + dctx_->markTotalLengthIsKnown(); + dctx_->getOwnerRequestGroup()->initPieceStorage(); + + SharedHandle pieceStorage = + dctx_->getOwnerRequestGroup()->getPieceStorage(); + // We enter 'end game' mode from the start to get metadata + // quickly. + pieceStorage->enterEndGame(); } - if(!peer_->isSeeder()) { - peer_->reconfigureSessionResource(dctx_->getPieceLength(), - dctx_->getTotalLength()); - peer_->setAllBitfield(); - } - } else { - attrs->metadataSize = metadataSize_; - dctx_->getFirstFileEntry()->setLength(metadataSize_); - dctx_->markTotalLengthIsKnown(); - dctx_->getOwnerRequestGroup()->initPieceStorage(); - - SharedHandle pieceStorage = - dctx_->getOwnerRequestGroup()->getPieceStorage(); - // We enter 'end game' mode from the start to get metadata - // quickly. - pieceStorage->enterEndGame(); peer_->reconfigureSessionResource(dctx_->getPieceLength(), dctx_->getTotalLength()); peer_->setAllBitfield(); + } else { + throw DL_ABORT_EX("Peer didn't provide metadata_size." + " It seems that it doesn't have whole metadata."); } - } else if(attrs->metadata.empty()) { - throw DL_ABORT_EX("Peer didn't provide metadata_size." - " It seems that it doesn't have whole metadata."); } } diff --git a/src/Peer.cc b/src/Peer.cc index 74c97f1b..73d35f91 100644 --- a/src/Peer.cc +++ b/src/Peer.cc @@ -79,6 +79,7 @@ void Peer::allocateSessionResource(size_t pieceLength, uint64_t totalLength) delete res_; res_ = new PeerSessionResource(pieceLength, totalLength); res_->getPeerStat().downloadStart(); + updateSeeder(); } void Peer::reconfigureSessionResource(size_t pieceLength, uint64_t totalLength) diff --git a/test/HandshakeExtensionMessageTest.cc b/test/HandshakeExtensionMessageTest.cc index d00d1964..52eb888d 100644 --- a/test/HandshakeExtensionMessageTest.cc +++ b/test/HandshakeExtensionMessageTest.cc @@ -117,9 +117,17 @@ void HandshakeExtensionMessageTest::testDoReceivedAction() CPPUNIT_ASSERT_EQUAL((uint16_t)6889, peer->getPort()); CPPUNIT_ASSERT_EQUAL((uint8_t)1, peer->getExtensionMessageID("ut_pex")); CPPUNIT_ASSERT_EQUAL((uint8_t)2, peer->getExtensionMessageID("a2_dht")); + CPPUNIT_ASSERT(peer->isSeeder()); CPPUNIT_ASSERT_EQUAL((size_t)1024, attrs->metadataSize); CPPUNIT_ASSERT_EQUAL((uint64_t)1024, dctx->getTotalLength()); CPPUNIT_ASSERT(dctx->knowsTotalLength()); + + // See Peer is not marked as seeder if !attrs->metadata.empty() + peer->allocateSessionResource(1024, 1024*1024); + attrs->metadataSize = 1024; + attrs->metadata = std::string('0', attrs->metadataSize); + msg.doReceivedAction(); + CPPUNIT_ASSERT(!peer->isSeeder()); } void HandshakeExtensionMessageTest::testCreate()