Browse Source

2008-08-07 Tatsuhiro Tsujikawa <tujikawa at rednoah dot com>

	Eliminated randomness from the test case. Removed #ifdef 
__MINGW32
	directive.
	* test/DefaultPieceStorageTest.cc

2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Fixed memory leak in test code
	* test/AnnounceListTest.cc
	* test/DefaultPeerListProcessorTest.cc
	* test/DefaultPeerStorageTest.cc
	* test/MetaFileUtilTest.cc
	
2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Fixed wrong argument passing to BitfieldMan::isBitSet()
	* src/DefaultPieceStorage.cc
	
2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Initialized _directIOAllowed
	* src/MultiDiskAdaptor.cc

2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Fixed memory leak
	* src/GZipDecoder.cc

2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Fixed memory leak
	* src/MetalinkParserStateMachine.cc

2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Fixed memory leak
	* src/Dictionary.cc

2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Fixed memory leak
	* src/IteratableChunkChecksumValidator.h

2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Fixed unmatched malloc/free.
	* src/IteratableChunkChecksumValidator.cc
	* src/IteratableChecksumValidator.cc

2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>

	Removed max chunk size check. This change fixes BUG#2040169
	* src/ChunkedDecoder.cc
	* src/ChunkedDecoder.h
	* test/ChunkedDecoderTest.cc
Tatsuhiro Tsujikawa 17 năm trước cách đây
mục cha
commit
5c35e9a416

+ 57 - 0
ChangeLog

@@ -1,3 +1,60 @@
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Eliminated randomness from the test case. Removed #ifdef __MINGW32
+	directive.
+	* test/DefaultPieceStorageTest.cc
+
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed memory leak in test code
+	* test/AnnounceListTest.cc
+	* test/DefaultPeerListProcessorTest.cc
+	* test/DefaultPeerStorageTest.cc
+	* test/MetaFileUtilTest.cc
+	
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed wrong argument passing to BitfieldMan::isBitSet()
+	* src/DefaultPieceStorage.cc
+	
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Initialized _directIOAllowed
+	* src/MultiDiskAdaptor.cc
+
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed memory leak
+	* src/GZipDecoder.cc
+
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed memory leak
+	* src/MetalinkParserStateMachine.cc
+
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed memory leak
+	* src/Dictionary.cc
+
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed memory leak
+	* src/IteratableChunkChecksumValidator.h
+
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed unmatched malloc/free.
+	* src/IteratableChunkChecksumValidator.cc
+	* src/IteratableChecksumValidator.cc
+
+2008-08-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Removed max chunk size check. This change fixes BUG#2040169
+	* src/ChunkedDecoder.cc
+	* src/ChunkedDecoder.h
+	* test/ChunkedDecoderTest.cc
+
 2008-08-05  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
 
 	* Release 0.15.1+2

+ 4 - 8
src/ChunkedDecoder.cc

@@ -49,7 +49,7 @@ ChunkedDecoder::~ChunkedDecoder() {}
 
 void ChunkedDecoder::init() {}
 
-static bool readChunkSize(size_t& chunkSize, std::string& in)
+static bool readChunkSize(uint64_t& chunkSize, std::string& in)
 {
   std::string::size_type crlfPos = in.find(A2STR::CRLF);
   if(crlfPos == std::string::npos) {
@@ -59,14 +59,14 @@ static bool readChunkSize(size_t& chunkSize, std::string& in)
   if(extPos == std::string::npos || crlfPos < extPos) {
     extPos = crlfPos;
   }
-  chunkSize = Util::parseUInt(in.substr(0, extPos), 16);
+  chunkSize = Util::parseULLInt(in.substr(0, extPos), 16);
   in.erase(0, crlfPos+2);
   return true;
 }
 
-static bool readData(std::string& out, size_t& chunkSize, std::string& in)
+static bool readData(std::string& out, uint64_t& chunkSize, std::string& in)
 {
-  size_t readlen = std::min(chunkSize, in.size());
+  uint64_t readlen = std::min(chunkSize, static_cast<uint64_t>(in.size()));
   out.append(in.begin(), in.begin()+readlen);
   in.erase(0, readlen);
   chunkSize -= readlen;
@@ -94,10 +94,6 @@ std::string ChunkedDecoder::decode(const unsigned char* inbuf, size_t inlen)
 	  _state = STREAM_END;
 	  break;
 	} else {
-	  if(_chunkSize > MAX_CHUNK_SIZE) {
-	    throw DlAbortEx
-	      (StringFormat(EX_TOO_LARGE_CHUNK, _chunkSize).str());
-	  }
 	  _state = READ_DATA;
 	}
       } else {

+ 1 - 3
src/ChunkedDecoder.h

@@ -49,12 +49,10 @@ private:
 
   std::string _buf;
 
-  size_t _chunkSize;
+  uint64_t _chunkSize;
 
   STATE _state;
   
-  static const size_t MAX_CHUNK_SIZE = 1024*1024;
-
   static const std::string NAME;
 
 public:

+ 1 - 1
src/DefaultPieceStorage.cc

@@ -189,7 +189,7 @@ bool DefaultPieceStorage::getMissingFastPieceIndex(size_t& index,
 			     bitfieldMan->getTotalLength());
     for(std::deque<size_t>::const_iterator itr = peer->getPeerAllowedIndexSet().begin();
 	itr != peer->getPeerAllowedIndexSet().end(); itr++) {
-      if(!bitfieldMan->isBitSet(index) && peer->hasPiece(*itr)) {
+      if(!bitfieldMan->isBitSet(*itr) && peer->hasPiece(*itr)) {
 	tempBitfield.setBit(*itr);
       }
     }

+ 6 - 2
src/Dictionary.cc

@@ -67,8 +67,12 @@ void Dictionary::put(const std::string& name, MetaEntry* entry) {
 
 void Dictionary::remove(const std::string& name)
 {
-  table.erase(name);
-  order.erase(std::remove(order.begin(), order.end(), name), order.end());
+  std::map<std::string, MetaEntry*>::iterator i = table.find(name);
+  if(i != table.end()) {
+    delete i->second;
+    table.erase(i);
+    order.erase(std::remove(order.begin(), order.end(), name), order.end());
+  }
 }
 
 void Dictionary::accept(MetaEntryVisitor* v) const {

+ 3 - 0
src/GZipDecoder.cc

@@ -49,6 +49,8 @@ GZipDecoder::~GZipDecoder()
 
 void GZipDecoder::init()
 {
+  _finished = false;
+  release();
   _strm = new z_stream();
   _strm->zalloc = Z_NULL;
   _strm->zfree = Z_NULL;
@@ -66,6 +68,7 @@ void GZipDecoder::release()
 {
   if(_strm) {
     inflateEnd(_strm);
+    delete _strm;
     _strm = 0;
   }
 }

+ 8 - 2
src/IteratableChecksumValidator.cc

@@ -59,7 +59,11 @@ IteratableChecksumValidator::IteratableChecksumValidator(const SingleFileDownloa
 
 IteratableChecksumValidator::~IteratableChecksumValidator()
 {
+#ifdef HAVE_POSIX_MEMALIGN
+  free(_buffer);
+#else // !HAVE_POSIX_MEMALIGN
   delete [] _buffer;
+#endif // !HAVE_POSIX_MEMALIGN
 }
 
 void IteratableChecksumValidator::validateChunk()
@@ -100,10 +104,12 @@ uint64_t IteratableChecksumValidator::getTotalLength() const
 void IteratableChecksumValidator::init()
 {
 #ifdef HAVE_POSIX_MEMALIGN
+  free(_buffer);
   _buffer = (unsigned char*)Util::allocateAlignedMemory(ALIGNMENT, BUFSIZE);
-#else
+#else // !HAVE_POSIX_MEMALIGN
+  delete [] _buffer;
   _buffer = new unsigned char[BUFSIZE];
-#endif // HAVE_POSIX_MEMALIGN
+#endif // !HAVE_POSIX_MEMALIGN
   _pieceStorage->getDiskAdaptor()->enableDirectIO();
   _currentOffset = 0;
   _ctx.reset(new MessageDigestContext());

+ 8 - 2
src/IteratableChunkChecksumValidator.cc

@@ -66,7 +66,11 @@ IteratableChunkChecksumValidator(const DownloadContextHandle& dctx,
 
 IteratableChunkChecksumValidator::~IteratableChunkChecksumValidator()
 {
+#ifdef HAVE_POSIX_MEMALIGN
+  free(_buffer);
+#else // !HAVE_POSIX_MEMALIGN
   delete [] _buffer;
+#endif // !HAVE_POSIX_MEMALIGN
 }
 
 
@@ -115,10 +119,12 @@ std::string IteratableChunkChecksumValidator::calculateActualChecksum()
 void IteratableChunkChecksumValidator::init()
 {
 #ifdef HAVE_POSIX_MEMALIGN
+  free(_buffer);
   _buffer = (unsigned char*)Util::allocateAlignedMemory(ALIGNMENT, BUFSIZE);
-#else
+#else // !HAVE_POSIX_MEMALIGN
+  delete [] _buffer;
   _buffer = new unsigned char[BUFSIZE];
-#endif // HAVE_POSIX_MEMALIGN
+#endif // !HAVE_POSIX_MEMALIGN
   if(_dctx->getFileEntries().size() == 1) {
     _pieceStorage->getDiskAdaptor()->enableDirectIO();
   }

+ 1 - 1
src/IteratableChunkChecksumValidator.h

@@ -50,7 +50,7 @@ class IteratableChunkChecksumValidator:public IteratableValidator
 private:
   SharedHandle<DownloadContext> _dctx;
   SharedHandle<PieceStorage> _pieceStorage;
-  BitfieldMan* _bitfield;
+  SharedHandle<BitfieldMan> _bitfield;
   size_t _currentIndex;
   Logger* _logger;
   SharedHandle<MessageDigestContext> _ctx;

+ 2 - 1
src/MetalinkParserStateMachine.cc

@@ -161,7 +161,8 @@ void MetalinkParserStateMachine::setFinState()
 
 void MetalinkParserStateMachine::setSkipTagState(MetalinkParserState* prevSate)
 {
-  _state = new SkipTagMetalinkParserState(prevSate);
+  _skipTagState = new SkipTagMetalinkParserState(prevSate);
+  _state = _skipTagState;
 }
 
 void MetalinkParserStateMachine::restoreSavedState()

+ 3 - 1
src/MultiDiskAdaptor.cc

@@ -145,7 +145,9 @@ void DiskWriterEntry::disableDirectIO()
 }
 
 MultiDiskAdaptor::MultiDiskAdaptor():
-  pieceLength(0), _maxOpenFiles(DEFAULT_MAX_OPEN_FILES) {}
+  pieceLength(0),
+  _maxOpenFiles(DEFAULT_MAX_OPEN_FILES),
+  _directIOAllowed(false) {}
 
 MultiDiskAdaptor::~MultiDiskAdaptor() {}
 

+ 26 - 17
test/AnnounceListTest.cc

@@ -1,7 +1,7 @@
 #include "AnnounceList.h"
 #include "MetaFileUtil.h"
 #include "Exception.h"
-#include "Dictionary.h"
+#include "List.h"
 #include <cppunit/extensions/HelperMacros.h>
 
 namespace aria2 {
@@ -43,11 +43,12 @@ CPPUNIT_TEST_SUITE_REGISTRATION( AnnounceListTest );
 
 void AnnounceListTest::testSingleElementList() {
   std::string peersString = "ll8:tracker1el8:tracker2el8:tracker3ee";
-  Dictionary* announces = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<List> announces
+    (dynamic_cast<List*>(MetaFileUtil::bdecoding(peersString)));
 
   // ANNOUNCE_LIST
   // [ [ tracker1 ], [ tracker2 ], [ tracker3 ] ]
-  AnnounceList announceList(announces);
+  AnnounceList announceList(announces.get());
   
   CPPUNIT_ASSERT(!announceList.allTiersFailed());
   std::string url =  announceList.getAnnounce();
@@ -89,10 +90,12 @@ void AnnounceListTest::testSingleElementList() {
 
 void AnnounceListTest::testMultiElementList() {
   std::string peersString = "ll8:tracker18:tracker28:tracker3ee";
-  Dictionary* announces = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<List> announces
+    (dynamic_cast<List*>(MetaFileUtil::bdecoding(peersString)));
+
   // ANNOUNCE_LIST
   // [ [ tracker1, tracker2, tracker3 ] ]
-  AnnounceList announceList(announces);
+  AnnounceList announceList(announces.get());
   
   CPPUNIT_ASSERT(!announceList.allTiersFailed());
   std::string url = announceList.getAnnounce();
@@ -121,11 +124,12 @@ void AnnounceListTest::testMultiElementList() {
 
 void AnnounceListTest::testSingleAndMulti() {
   std::string peersString = "ll8:tracker18:tracker2el8:tracker3ee";
-  Dictionary* announces = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<List> announces
+    (dynamic_cast<List*>(MetaFileUtil::bdecoding(peersString)));
 
   // ANNOUNCE_LIST
   // [ [ tracker1, tracker2 ], [ tracker3 ] ]
-  AnnounceList announceList(announces);
+  AnnounceList announceList(announces.get());
 
   std::string url = announceList.getAnnounce();
   CPPUNIT_ASSERT_EQUAL(std::string("tracker1"), url);
@@ -147,20 +151,22 @@ void AnnounceListTest::testSingleAndMulti() {
 
 void AnnounceListTest::testNoGroup() {
   std::string peersString = "llee";
-  Dictionary* announces = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<List> announces
+    (dynamic_cast<List*>(MetaFileUtil::bdecoding(peersString)));
 
-  AnnounceList announceList(announces);
+  AnnounceList announceList(announces.get());
 
   CPPUNIT_ASSERT(announceList.countTier() == 0);
 }
 
 void AnnounceListTest::testNextEventIfAfterStarted() {
   std::string peersString = "ll8:tracker1ee";
-  Dictionary* announces = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<List> announces
+    (dynamic_cast<List*>(MetaFileUtil::bdecoding(peersString)));
 
   // ANNOUNCE_LIST
   // [ [ tracker1 ] ]
-  AnnounceList announceList(announces);
+  AnnounceList announceList(announces.get());
   announceList.setEvent(AnnounceTier::STOPPED);
   announceList.announceFailure();
   announceList.resetTier();
@@ -176,11 +182,12 @@ void AnnounceListTest::testNextEventIfAfterStarted() {
 
 void AnnounceListTest::testEvent() {
   std::string peersString = "ll8:tracker1el8:tracker2el8:tracker3ee";
-  Dictionary* announces = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<List> announces
+    (dynamic_cast<List*>(MetaFileUtil::bdecoding(peersString)));
 
   // ANNOUNCE_LIST
   // [ [ tracker1 ], [ tracker2 ], [ tracker3 ] ]
-  AnnounceList announceList(announces);
+  AnnounceList announceList(announces.get());
 
   announceList.setEvent(AnnounceTier::STOPPED);
   announceList.announceSuccess();
@@ -200,11 +207,12 @@ void AnnounceListTest::testEvent() {
 
 void AnnounceListTest::testCountStoppedAllowedTier() {
   std::string peersString = "ll8:tracker1el8:tracker2el8:tracker3ee";
-  Dictionary* announces = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<List> announces
+    (dynamic_cast<List*>(MetaFileUtil::bdecoding(peersString)));
 
   // ANNOUNCE_LIST
   // [ [ tracker1 ], [ tracker2 ], [ tracker3 ] ]
-  AnnounceList announceList(announces);
+  AnnounceList announceList(announces.get());
 
   CPPUNIT_ASSERT_EQUAL((size_t)0, announceList.countStoppedAllowedTier());
   announceList.setEvent(AnnounceTier::STARTED);
@@ -227,11 +235,12 @@ void AnnounceListTest::testCountStoppedAllowedTier() {
 
 void AnnounceListTest::testCountCompletedAllowedTier() {
   std::string peersString = "ll8:tracker1el8:tracker2el8:tracker3ee";
-  Dictionary* announces = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<List> announces
+    (dynamic_cast<List*>(MetaFileUtil::bdecoding(peersString)));
 
   // ANNOUNCE_LIST
   // [ [ tracker1 ], [ tracker2 ], [ tracker3 ] ]
-  AnnounceList announceList(announces);
+  AnnounceList announceList(announces.get());
 
   CPPUNIT_ASSERT_EQUAL((size_t)0, announceList.countCompletedAllowedTier());
   announceList.setEvent(AnnounceTier::STARTED);

+ 17 - 9
test/ChunkedDecoderTest.cc

@@ -110,16 +110,24 @@ void ChunkedDecoderTest::testDecode()
 
 void ChunkedDecoderTest::testDecode_tooLargeChunkSize()
 {
-  // Feed chunkSize == ChunkedDecoder::MAX_CHUNK_SIZE + 1 == 0x100001.
-  std::basic_string<unsigned char> msg =
-    reinterpret_cast<const unsigned char*>("100001\r\n");
-
-  ChunkedDecoder decoder;
-  try {
+  // chunkSize should be under 2^64-1
+  {
+    std::basic_string<unsigned char> msg =
+      reinterpret_cast<const unsigned char*>("ffffffffffffffff\r\n");
+    ChunkedDecoder decoder;
     decoder.decode(msg.c_str(), msg.size());
-    CPPUNIT_FAIL("exception must be thrown.");
-  } catch(DlAbortEx& e) {
-    // success
+  }
+  // chunkSize 2^64 causes error
+  {
+    std::basic_string<unsigned char> msg =
+      reinterpret_cast<const unsigned char*>("10000000000000000\r\n");
+    ChunkedDecoder decoder;
+    try {
+      decoder.decode(msg.c_str(), msg.size());
+      CPPUNIT_FAIL("exception must be thrown.");
+    } catch(DlAbortEx& e) {
+      // success
+    }
   }
 }
 

+ 4 - 2
test/DefaultPeerListProcessorTest.cc

@@ -30,7 +30,8 @@ void DefaultPeerListProcessorTest::testExtractPeer() {
   DefaultPeerListProcessor proc;
   std::string peersString = "d5:peersld2:ip11:192.168.0.17:peer id20:aria2-000000000000004:porti2006eeee";
 
-  Dictionary* dic = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<Dictionary> dic
+    (dynamic_cast<Dictionary*>(MetaFileUtil::bdecoding(peersString)));
   
   CPPUNIT_ASSERT(proc.canHandle(dic->get("peers")));
 
@@ -46,7 +47,8 @@ void DefaultPeerListProcessorTest::testExtract2Peers() {
   DefaultPeerListProcessor proc;
   std::string peersString = "d5:peersld2:ip11:192.168.0.17:peer id20:aria2-000000000000004:porti2006eed2:ip11:192.168.0.27:peer id20:aria2-000000000000004:porti2007eeee";
 
-  Dictionary* dic = (Dictionary*)MetaFileUtil::bdecoding(peersString);
+  SharedHandle<Dictionary> dic
+    (dynamic_cast<Dictionary*>(MetaFileUtil::bdecoding(peersString)));
 
   std::deque<SharedHandle<Peer> > peers;
   proc.extractPeer(peers, dic->get("peers"));

+ 4 - 0
test/DefaultPeerStorageTest.cc

@@ -35,6 +35,10 @@ public:
     btRuntime.reset(new BtRuntime());
   }
 
+  void tearDown() {
+    delete option;
+  }
+
   void testCountPeer();
   void testDeleteUnusedPeer();
   void testAddPeer();

+ 2 - 7
test/DefaultPieceStorageTest.cc

@@ -95,7 +95,7 @@ void DefaultPieceStorageTest::testGetMissingPiece() {
 }
 
 void DefaultPieceStorageTest::testGetMissingFastPiece() {
-  DefaultPieceStorage pss(btContext, option);
+  DefaultPieceStorage pss(btContext, option, false);
   pss.setEndGamePieceNum(0);
 
   peer->setAllBitfield();
@@ -118,19 +118,14 @@ void DefaultPieceStorageTest::testHasMissingPiece() {
 }
 
 void DefaultPieceStorageTest::testCompletePiece() {
-  DefaultPieceStorage pss(btContext, option, true);
+  DefaultPieceStorage pss(btContext, option, false);
   pss.setEndGamePieceNum(0);
 
   peer->setAllBitfield();
 
   SharedHandle<Piece> piece = pss.getMissingPiece(peer);
-#ifdef __MINGW32__
-  CPPUNIT_ASSERT_EQUAL(std::string("piece: index=2, length=128"),
-		       piece->toString());
-#else // !__MINGW32__
   CPPUNIT_ASSERT_EQUAL(std::string("piece: index=0, length=128"),
 		       piece->toString());
-#endif // !__MINGW32__
 
   CPPUNIT_ASSERT_EQUAL((uint64_t)0ULL, pss.getCompletedLength());
 

+ 3 - 3
test/MetaFileUtilTest.cc

@@ -28,9 +28,9 @@ public:
 CPPUNIT_TEST_SUITE_REGISTRATION( MetaFileUtilTest );
 
 void MetaFileUtilTest::testParseMetaFile() {
-  MetaEntry* entry = MetaFileUtil::parseMetaFile("test.torrent");
-  Dictionary* d = dynamic_cast<Dictionary*>(entry);
-  CPPUNIT_ASSERT(d != NULL);
+  SharedHandle<MetaEntry> entry(MetaFileUtil::parseMetaFile("test.torrent"));
+  SharedHandle<Dictionary> d = dynamic_pointer_cast<Dictionary>(entry);
+  CPPUNIT_ASSERT(!d.isNull());
 }
 
 void MetaFileUtilTest::testBdecoding() {