2010-09-15 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>

Fixed the bug that a file gets overwritten if -V is given and no
	hash is provided. Fixed the bug that --dry-run leads download
	error. Added RequestGroup::createCheckIntegrityEntry() which
	correctly creates CheckIntegrityEntry objects and open files based
	on -V option and the existence of control file.
	* src/AbstractCommand.cc
	* src/AbstractCommand.h
	* src/ChecksumCheckIntegrityEntry.cc
	* src/DownloadContext.cc
	* src/DownloadContext.h
	* src/FtpNegotiationCommand.cc
	* src/HttpResponseCommand.cc
	* src/PieceHashCheckIntegrityEntry.cc
	* src/RequestGroup.cc
	* src/RequestGroup.h
	* src/RequestGroupEntry.cc
	* src/RequestGroupEntry.h

ChangeLog

@@ -1,3 +1,23 @@
+2010-09-15  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	Fixed the bug that a file gets overwritten if -V is given and no
+	hash is provided. Fixed the bug that --dry-run leads download
+	error. Added RequestGroup::createCheckIntegrityEntry() which
+	correctly creates CheckIntegrityEntry objects and open files based
+	on -V option and the existence of control file.
+	* src/AbstractCommand.cc
+	* src/AbstractCommand.h
+	* src/ChecksumCheckIntegrityEntry.cc
+	* src/DownloadContext.cc
+	* src/DownloadContext.h
+	* src/FtpNegotiationCommand.cc
+	* src/HttpResponseCommand.cc
+	* src/PieceHashCheckIntegrityEntry.cc
+	* src/RequestGroup.cc
+	* src/RequestGroup.h
+	* src/RequestGroupEntry.cc
+	* src/RequestGroupEntry.h
+
 2010-09-13  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
 
 	Fixed compile error without zlib

src/AbstractCommand.cc

@@ -777,28 +777,9 @@ std::string AbstractCommand::resolveHostname
   return ipaddr;
 }
 
-// nextCommand is going to be managed by CheckIntegrityEntry which is
+void AbstractCommand::prepareForNextAction
-// created in side this function. Don't release nextCommand after this
+(const SharedHandle<CheckIntegrityEntry>& checkEntry)
-// function call.
-void AbstractCommand::prepareForNextAction(Command* nextCommand)
 {
-  SharedHandle<CheckIntegrityEntry> checkEntry;
-#ifdef ENABLE_MESSAGE_DIGEST
-  if(requestGroup_->downloadFinished() &&
-     getDownloadContext()->isChecksumVerificationNeeded()) {
-    if(getLogger()->info()) {
-      getLogger()->info(MSG_HASH_CHECK_NOT_DONE);
-    }
-    SharedHandle<ChecksumCheckIntegrityEntry> entry
-      (new ChecksumCheckIntegrityEntry(requestGroup_, nextCommand));
-    entry->setRedownload(true);
-    checkEntry = entry;
-  } else
-#endif // ENABLE_MESSAGE_DIGEST
-    {
-      checkEntry.reset
-        (new StreamCheckIntegrityEntry(requestGroup_, nextCommand));
-    }
   std::vector<Command*>* commands = new std::vector<Command*>();
   auto_delete_container<std::vector<Command*> > commandsDel(commands);
   requestGroup_->processCheckIntegrityEntry(*commands, checkEntry, e_);

src/AbstractCommand.h

@@ -176,7 +176,8 @@ protected:
 
   void setTimeout(time_t timeout) { timeout_ = timeout; }
 
-  void prepareForNextAction(Command* nextCommand = 0);
+  void prepareForNextAction
+  (const SharedHandle<CheckIntegrityEntry>& checkEntry);
 
   // Check if socket is connected. If socket is not connected and
   // there are other addresses to try, command is created using

src/ChecksumCheckIntegrityEntry.cc

@@ -54,8 +54,9 @@ ChecksumCheckIntegrityEntry::~ChecksumCheckIntegrityEntry() {}
 
 bool ChecksumCheckIntegrityEntry::isValidationReady()
 {
-  return !getRequestGroup()->getDownloadContext()->getChecksum().empty() &&
+  const SharedHandle<DownloadContext>& dctx =
-    !getRequestGroup()->getDownloadContext()->getChecksumHashAlgo().empty();
+    getRequestGroup()->getDownloadContext();
+  return dctx->isChecksumVerificationAvailable();
 }
 
 void ChecksumCheckIntegrityEntry::initValidator()

src/DownloadContext.cc

@@ -213,4 +213,15 @@ bool DownloadContext::isChecksumVerificationNeeded() const
     !checksum_.empty() && !checksumHashAlgo_.empty() && !checksumVerified_;
 }
 
+bool DownloadContext::isChecksumVerificationAvailable() const
+{
+  return !checksum_.empty() && !checksumHashAlgo_.empty();
+}
+
+bool DownloadContext::isPieceHashVerificationAvailable() const
+{
+  return !pieceHashAlgo_.empty() &&
+    pieceHashes_.size() > 0 && pieceHashes_.size() == getNumPieces();
+}
+
 } // namespace aria2

src/DownloadContext.h

@@ -206,6 +206,12 @@ public:
   // need to be done
   bool isChecksumVerificationNeeded() const;
 
+  // Returns true if whole hash(not piece hash) is available.
+  bool isChecksumVerificationAvailable() const;
+
+  // Returns true if piece hash(not whole file hash) is available.
+  bool isPieceHashVerificationAvailable() const;
+
   void setChecksumVerified(bool f)
   {
     checksumVerified_ = f;

src/FtpNegotiationCommand.cc

@@ -452,32 +452,20 @@ bool FtpNegotiationCommand::onFileSizeDetermined(uint64_t totalLength)
       return false;
     }
 
-    BtProgressInfoFileHandle infoFile
+    SharedHandle<CheckIntegrityEntry> checkIntegrityEntry =
-      (new DefaultBtProgressInfoFile(getDownloadContext(),
+      getRequestGroup()->createCheckIntegrityEntry();
-                                     getPieceStorage(),
+    if(checkIntegrityEntry.isNull()) {
-                                     getOption().get()));
+      sequence_ = SEQ_DOWNLOAD_ALREADY_COMPLETED;
-    if(!infoFile->exists() &&
+      poolConnection();
-       getRequestGroup()->downloadFinishedByFileLength()) {
+      return false;
-      getPieceStorage()->markAllPiecesDone();
-      // See also RequestGroup::createInitialCommand()
-      if(!getOption()->getAsBool(PREF_CHECK_INTEGRITY) ||
-         !getDownloadContext()->isChecksumVerificationNeeded()) {
-        getDownloadContext()->setChecksumVerified(true);
-        sequence_ = SEQ_DOWNLOAD_ALREADY_COMPLETED;
-        getLogger()->notice(MSG_DOWNLOAD_ALREADY_COMPLETED,
-                            util::itos(getRequestGroup()->getGID()).c_str(),
-                            getRequestGroup()->getFirstFilePath().c_str());
-        poolConnection();
-        return false;
-      }
     }
-    getRequestGroup()->loadAndOpenFile(infoFile);
+    checkIntegrityEntry->pushNextCommand(this);
     // We have to make sure that command that has Request object must
     // have segment after PieceStorage is initialized. See
     // AbstractCommand::execute()
     getSegmentMan()->getSegmentWithIndex(getCuid(), 0);
 
-    prepareForNextAction(this);
+    prepareForNextAction(checkIntegrityEntry);
 
     disableReadCheckSocket();
   }
@@ -960,6 +948,7 @@ void FtpNegotiationCommand::poolConnection() const
 void FtpNegotiationCommand::onDryRunFileFound()
 {
   getPieceStorage()->markAllPiecesDone();
+  getDownloadContext()->setChecksumVerified(true);
   poolConnection();
   sequence_ = SEQ_HEAD_OK;
 }

src/HttpResponseCommand.cc

@@ -265,23 +265,11 @@ bool HttpResponseCommand::handleDefaultEncoding
     return true;
   }
 
-  BtProgressInfoFileHandle infoFile
+  SharedHandle<CheckIntegrityEntry> checkEntry =
-    (new DefaultBtProgressInfoFile(getDownloadContext(),
+    getRequestGroup()->createCheckIntegrityEntry();
-                                   getPieceStorage(),
+  if(checkEntry.isNull()) {
-                                   getOption().get()));
+    return true;
-  if(!infoFile->exists() && getRequestGroup()->downloadFinishedByFileLength()) {
-    getPieceStorage()->markAllPiecesDone();
-    // See also RequestGroup::createInitialCommand()
-    if(!getOption()->getAsBool(PREF_CHECK_INTEGRITY) ||
-       !getDownloadContext()->isChecksumVerificationNeeded()) {
-      getDownloadContext()->setChecksumVerified(true);
-      getLogger()->notice(MSG_DOWNLOAD_ALREADY_COMPLETED,
-                          util::itos(getRequestGroup()->getGID()).c_str(),
-                          getRequestGroup()->getFirstFilePath().c_str());
-      return true;
-    }
   }
-  getRequestGroup()->loadAndOpenFile(infoFile);
   File file(getRequestGroup()->getFirstFilePath());
   // We have to make sure that command that has Request object must
   // have segment after PieceStorage is initialized. See
@@ -306,8 +294,11 @@ bool HttpResponseCommand::handleDefaultEncoding
   }
   // After command is passed to prepareForNextAction(), it is managed
   // by CheckIntegrityEntry.
-  prepareForNextAction(command);
+  checkEntry->pushNextCommand(command);
   command = 0;
+
+  prepareForNextAction(checkEntry);
+
   if(getRequest()->getMethod() == Request::METHOD_HEAD) {
     poolConnection();
     getRequest()->setMethod(Request::METHOD_GET);
@@ -506,6 +497,7 @@ void HttpResponseCommand::poolConnection()
 void HttpResponseCommand::onDryRunFileFound()
 {
   getPieceStorage()->markAllPiecesDone();
+  getDownloadContext()->setChecksumVerified(true);
   poolConnection();
 }
 

src/PieceHashCheckIntegrityEntry.cc

@@ -51,9 +51,7 @@ bool PieceHashCheckIntegrityEntry::isValidationReady()
 {
   const SharedHandle<DownloadContext>& dctx =
     getRequestGroup()->getDownloadContext();
-  return !dctx->getPieceHashAlgo().empty() &&
+  return dctx->isPieceHashVerificationAvailable();
-    dctx->getPieceHashes().size() > 0 &&
-    dctx->getPieceHashes().size() == dctx->getNumPieces();
 }
 
 void PieceHashCheckIntegrityEntry::initValidator()

src/RequestGroup.cc

@@ -164,6 +164,13 @@ RequestGroup::RequestGroup(const SharedHandle<Option>& option):
 
 RequestGroup::~RequestGroup() {}
 
+bool RequestGroup::isCheckIntegrityReady() const
+{
+  return option_->getAsBool(PREF_CHECK_INTEGRITY) &&
+    (downloadContext_->isChecksumVerificationAvailable() ||
+     downloadContext_->isPieceHashVerificationAvailable());
+}
+
 bool RequestGroup::downloadFinished() const
 {
   if(pieceStorage_.isNull()) {
@@ -207,6 +214,68 @@ void RequestGroup::closeFile()
   }
 }
 
+// TODO The function name is not intuitive at all.. it does not convey
+// that this function open file.
+SharedHandle<CheckIntegrityEntry> RequestGroup::createCheckIntegrityEntry()
+{
+  BtProgressInfoFileHandle infoFile
+    (new DefaultBtProgressInfoFile(downloadContext_, pieceStorage_,
+                                   option_.get()));
+  SharedHandle<CheckIntegrityEntry> checkEntry;
+  if(option_->getAsBool(PREF_CHECK_INTEGRITY) &&
+     downloadContext_->isPieceHashVerificationAvailable()) {
+    // When checking piece hash, we don't care file is downloaded and
+    // infoFile exists.
+    loadAndOpenFile(infoFile);
+    checkEntry.reset(new StreamCheckIntegrityEntry(this));
+  } else if(infoFile->exists()) {
+    loadAndOpenFile(infoFile);
+    if(downloadFinished()) {
+#ifdef ENABLE_MESSAGE_DIGEST
+      if(downloadContext_->isChecksumVerificationNeeded()) {
+        if(logger_->info()) {
+          logger_->info(MSG_HASH_CHECK_NOT_DONE);
+        }
+        SharedHandle<ChecksumCheckIntegrityEntry> tempEntry
+          (new ChecksumCheckIntegrityEntry(this));
+        tempEntry->setRedownload(true);
+        checkEntry = tempEntry;
+      } else
+#endif // ENABLE_MESSAGE_DIGEST
+        {
+          downloadContext_->setChecksumVerified(true);
+          logger_->notice(MSG_DOWNLOAD_ALREADY_COMPLETED,
+                          util::itos(gid_).c_str(),
+                          downloadContext_->getBasePath().c_str());
+        }
+    } else {
+      checkEntry.reset(new StreamCheckIntegrityEntry(this));
+    }
+  } else if(downloadFinishedByFileLength()) {
+    pieceStorage_->markAllPiecesDone();
+#ifdef ENABLE_MESSAGE_DIGEST
+    if(option_->getAsBool(PREF_CHECK_INTEGRITY) &&
+       downloadContext_->isChecksumVerificationAvailable()) {
+      loadAndOpenFile(infoFile);
+      SharedHandle<ChecksumCheckIntegrityEntry> tempEntry
+        (new ChecksumCheckIntegrityEntry(this));
+      tempEntry->setRedownload(true);
+      checkEntry = tempEntry;
+    } else
+#endif // ENABLE_MESSAGE_DIGEST
+      {
+        downloadContext_->setChecksumVerified(true);
+        logger_->notice(MSG_DOWNLOAD_ALREADY_COMPLETED,
+                        util::itos(gid_).c_str(),
+                        downloadContext_->getBasePath().c_str());
+      }
+  } else {
+    loadAndOpenFile(infoFile);
+    checkEntry.reset(new StreamCheckIntegrityEntry(this));
+  }
+  return checkEntry;
+}
+
 void RequestGroup::createInitialCommand
 (std::vector<Command*>& commands, DownloadEngine* e)
 {
@@ -409,7 +478,7 @@ void RequestGroup::createInitialCommand
     if(option_->getAsBool(PREF_DRY_RUN) ||
        downloadContext_->getTotalLength() == 0) {
       createNextCommand(commands, e, 1);
-    }else {
+    } else {
       if(e->getRequestGroupMan()->isSameFileBeingDownloaded(this)) {
         throw DOWNLOAD_FAILURE_EXCEPTION
           (StringFormat(EX_DUPLICATE_FILE_DOWNLOAD,
@@ -421,50 +490,16 @@ void RequestGroup::createInitialCommand
                                            SharedHandle<PieceStorage>(),
                                            option_.get())));
       initPieceStorage();
-      BtProgressInfoFileHandle infoFile
+      SharedHandle<CheckIntegrityEntry> checkEntry =
-        (new DefaultBtProgressInfoFile(downloadContext_, pieceStorage_,
+        createCheckIntegrityEntry();
-                                       option_.get()));
+      if(!checkEntry.isNull()) {
-      bool finishedBySize =
+        processCheckIntegrityEntry(commands, checkEntry, e);
-        !infoFile->exists() && downloadFinishedByFileLength();
-      if(finishedBySize) {
-        pieceStorage_->markAllPiecesDone();
-        if(!option_->getAsBool(PREF_CHECK_INTEGRITY) ||
-           !downloadContext_->isChecksumVerificationNeeded()) {
-          // If --check-integrity=false and no checksum is provided,
-          // and .aria2 file does not exist, we just report download
-          // finished. We need
-          // DownloadContext::setChecksumVerified(true): without this,
-          // aria2 reports error for this download.
-          downloadContext_->setChecksumVerified(true);
-          logger_->notice(MSG_DOWNLOAD_ALREADY_COMPLETED,
-                          util::itos(gid_).c_str(),
-                          downloadContext_->getBasePath().c_str());
-        } else {
-          finishedBySize = false;
-        }
-      }
-      if(!finishedBySize) {
-        loadAndOpenFile(infoFile);
-        SharedHandle<CheckIntegrityEntry> checkIntegrityEntry;
-#ifdef ENABLE_MESSAGE_DIGEST
-        if(downloadFinished() &&
-           downloadContext_->isChecksumVerificationNeeded()) {
-          if(logger_->info()) {
-            logger_->info(MSG_HASH_CHECK_NOT_DONE);
-          }
-          SharedHandle<ChecksumCheckIntegrityEntry> entry
-            (new ChecksumCheckIntegrityEntry(this));
-          entry->setRedownload(true);
-          checkIntegrityEntry = entry;
-        } else
-#endif // ENABLE_MESSAGE_DIGEST
-          {
-            checkIntegrityEntry.reset(new StreamCheckIntegrityEntry(this));
-          }
-        processCheckIntegrityEntry(commands, checkIntegrityEntry, e);
       }
     }
   } else {
+    // TODO dry-run mode?
+    // TODO file size is known in this context?
+
     // In this context, multiple FileEntry objects are in
     // DownloadContext.
     if(e->getRequestGroupMan()->isSameFileBeingDownloaded(this)) {
@@ -488,7 +523,7 @@ void RequestGroup::createInitialCommand
       pieceStorage_->getDiskAdaptor()->openFile();
     } else {
       if(pieceStorage_->getDiskAdaptor()->fileExists()) {
-        if(!option_->getAsBool(PREF_CHECK_INTEGRITY) &&
+        if(!isCheckIntegrityReady() &&
            !option_->getAsBool(PREF_ALLOW_OVERWRITE)) {
           // TODO we need this->haltRequested = true?
           throw DOWNLOAD_FAILURE_EXCEPTION
@@ -615,9 +650,7 @@ bool RequestGroup::downloadFinishedByFileLength()
 {
   // assuming that a control file doesn't exist.
   if(!isPreLocalFileCheckEnabled() ||
-     option_->getAsBool(PREF_ALLOW_OVERWRITE) ||
+     option_->getAsBool(PREF_ALLOW_OVERWRITE)) {
-     (option_->getAsBool(PREF_CHECK_INTEGRITY) &&
-      !downloadContext_->getPieceHashes().empty())) {
     return false;
   }
   if(!downloadContext_->knowsTotalLength()) {
@@ -656,7 +689,7 @@ void RequestGroup::adjustFilename
       // File exists but user decided to resume it.
     } else {
 #ifdef ENABLE_MESSAGE_DIGEST
-      if(outfile.exists() && option_->getAsBool(PREF_CHECK_INTEGRITY)) {
+      if(outfile.exists() && isCheckIntegrityReady()) {
         // check-integrity existing file
       } else {
 #endif // ENABLE_MESSAGE_DIGEST
@@ -700,7 +733,7 @@ void RequestGroup::loadAndOpenFile(const BtProgressInfoFileHandle& progressInfoF
         pieceStorage_->markPiecesDone(outfile.size());
       } else {
 #ifdef ENABLE_MESSAGE_DIGEST
-        if(outfile.exists() && option_->getAsBool(PREF_CHECK_INTEGRITY)) {
+        if(outfile.exists() && isCheckIntegrityReady()) {
           pieceStorage_->getDiskAdaptor()->openExistingFile();
         } else {
 #endif // ENABLE_MESSAGE_DIGEST

src/RequestGroup.h

@@ -196,6 +196,8 @@ private:
 
   void removeDefunctControlFile
   (const SharedHandle<BtProgressInfoFile>& progressInfoFile);
+
+  bool isCheckIntegrityReady() const;
 public:
   // The copy of option is stored in RequestGroup object.
   RequestGroup(const SharedHandle<Option>& option);
@@ -207,6 +209,8 @@ public:
     return segmentMan_;
   }
 
+  SharedHandle<CheckIntegrityEntry> createCheckIntegrityEntry();
+
   // Returns first bootstrap commands to initiate a download.
   // If this is HTTP/FTP download and file size is unknown, only 1 command
   // (usually, HttpInitiateConnection or FtpInitiateConnection) will be created.

src/RequestGroupEntry.cc

@@ -60,4 +60,10 @@ Command* RequestGroupEntry::popNextCommand()
   return temp;
 }
 
+void RequestGroupEntry::pushNextCommand(Command* nextCommand)
+{
+  delete nextCommand_;
+  nextCommand_ = nextCommand;
+}
+
 } // namespace aria2

src/RequestGroupEntry.h

@@ -65,6 +65,8 @@ public:
 
   Command* popNextCommand();
 
+  void pushNextCommand(Command* nextCommand);
+  
   bool operator==(const RequestGroupEntry& entry) const
   {
     return this == &entry;