From ee229c6f00a0a01b99e37d91c14060a29d41a3a6 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Sun, 3 May 2009 06:26:44 +0000
Subject: [PATCH] 2009-05-03  Tatsuhiro Tsujikawa 
 <t-tujikawa@users.sourceforge.net>

	Fixed the bug that with --check-integrity option aria2 reports
	downloaded file size is 0 even if some parts of file is
	correctly downloaded.  This bug is reproducible for the download
	that a file includes last piece is missing. This bug doesn't
	reveal for single-torrent with file allocation on.
	* src/IteratableChecksumValidator.cc
	* test/IteratableChecksumValidatorTest.cc
---
 ChangeLog                                    | 10 ++++++
 src/IteratableChunkChecksumValidator.cc      | 27 ++++++++---------
 test/IteratableChunkChecksumValidatorTest.cc | 32 ++++++++++++++++++++
 3 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a973d0c42..934e8d8ba 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2009-05-03  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	Fixed the bug that with --check-integrity option aria2 reports
+	downloaded file size is 0 even if some parts of file is correctly
+	downloaded.  This bug is reproducible for the download that a file
+	includes last piece is missing. This bug doesn't reveal for
+	single-torrent with file allocation on.
+	* src/IteratableChecksumValidator.cc
+	* test/IteratableChecksumValidatorTest.cc
+
 2009-05-01  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
 
 	Avoid circular reference between RequestGroup and AdaptiveURISelector
diff --git a/src/IteratableChunkChecksumValidator.cc b/src/IteratableChunkChecksumValidator.cc
index dc03fc67b..d28883df3 100644
--- a/src/IteratableChunkChecksumValidator.cc
+++ b/src/IteratableChunkChecksumValidator.cc
@@ -83,23 +83,22 @@ void IteratableChunkChecksumValidator::validateChunk()
     std::string actualChecksum;
     try {
       actualChecksum = calculateActualChecksum();
+      if(actualChecksum == _dctx->getPieceHashes()[_currentIndex]) {
+	_bitfield->setBit(_currentIndex);
+      } else {
+	_logger->info(EX_INVALID_CHUNK_CHECKSUM,
+		      _currentIndex,
+		      Util::itos(getCurrentOffset(), true).c_str(),
+		      _dctx->getPieceHashes()[_currentIndex].c_str(),
+		      actualChecksum.c_str());
+	_bitfield->unsetBit(_currentIndex);
+      }
     } catch(RecoverableException& ex) {
       _logger->debug("Caught exception while validating piece index=%d. Some part of file may be missing. Continue operation.", ex, _currentIndex);
       _bitfield->unsetBit(_currentIndex);
-      _currentIndex++;
-      return;
     }
-    if(actualChecksum == _dctx->getPieceHashes()[_currentIndex]) {
-      _bitfield->setBit(_currentIndex);
-    } else {
-      _logger->info(EX_INVALID_CHUNK_CHECKSUM,
-		    _currentIndex,
-		    Util::itos(getCurrentOffset(), true).c_str(),
-		    _dctx->getPieceHashes()[_currentIndex].c_str(),
-		    actualChecksum.c_str());
-      _bitfield->unsetBit(_currentIndex);
-    }
-    _currentIndex++;
+
+    ++_currentIndex;
     if(finished()) {
       _pieceStorage->setBitfield(_bitfield->getBitfield(), _bitfield->getBitfieldLength());
     }
@@ -152,7 +151,7 @@ std::string IteratableChunkChecksumValidator::digest(off_t offset, size_t length
   while(curoffset < max) {
     size_t r = _pieceStorage->getDiskAdaptor()->readData(_buffer, BUFSIZE,
 							 curoffset);
-    if(r == 0) {
+    if(r == 0 || r < static_cast<size_t>(woffset)) {
       throw DlAbortEx
 	(StringFormat(EX_FILE_READ, _dctx->getActualBasePath().c_str(),
 		      strerror(errno)).str());
diff --git a/test/IteratableChunkChecksumValidatorTest.cc b/test/IteratableChunkChecksumValidatorTest.cc
index 4fd1f6290..bc89a19f2 100644
--- a/test/IteratableChunkChecksumValidatorTest.cc
+++ b/test/IteratableChunkChecksumValidatorTest.cc
@@ -15,6 +15,7 @@ class IteratableChunkChecksumValidatorTest:public CppUnit::TestFixture {
 
   CPPUNIT_TEST_SUITE(IteratableChunkChecksumValidatorTest);
   CPPUNIT_TEST(testValidate);
+  CPPUNIT_TEST(testValidate_readError);
   CPPUNIT_TEST_SUITE_END();
 private:
 
@@ -24,6 +25,7 @@ public:
   }
 
   void testValidate();
+  void testValidate_readError();
 };
 
 
@@ -70,4 +72,34 @@ void IteratableChunkChecksumValidatorTest::testValidate() {
   CPPUNIT_ASSERT(ps->hasPiece(2));
 }
 
+void IteratableChunkChecksumValidatorTest::testValidate_readError() {
+  Option option;
+  SharedHandle<SingleFileDownloadContext> dctx
+    (new SingleFileDownloadContext(100, 500, "chunkChecksumTestFile250.txt"));
+  std::deque<std::string> hashes(&csArray[0], &csArray[3]);
+  hashes.push_back("ffffffffffffffffffffffffffffffffffffffff");
+  hashes.push_back("ffffffffffffffffffffffffffffffffffffffff");
+  dctx->setPieceHashes(hashes);
+  dctx->setPieceHashAlgo("sha1");
+  SharedHandle<DefaultPieceStorage> ps(new DefaultPieceStorage(dctx, &option));
+  ps->initStorage();
+  ps->getDiskAdaptor()->openFile();
+
+  IteratableChunkChecksumValidator validator(dctx, ps);
+  validator.init();
+
+  while(!validator.finished()) {
+    validator.validateChunk();
+  }
+
+  CPPUNIT_ASSERT(ps->hasPiece(0));
+  CPPUNIT_ASSERT(ps->hasPiece(1));
+  CPPUNIT_ASSERT(!ps->hasPiece(2)); // #2 piece is not valid because
+				    // #program expects its size is
+				    // #100, but it reads only 50
+				    // #bytes and raises error.
+  CPPUNIT_ASSERT(!ps->hasPiece(3));
+  CPPUNIT_ASSERT(!ps->hasPiece(4));
+}
+
 } // namespace aria2