aria2 now doesn't assume download's completed just because file size matched

The only exception is zero-length file.  If server tells file is
zero-length and --checksum option is given, aria2 now correctly checks
its checksum. There is one known issue: If downloaded file is
zero-length file and .aria2 file exists, it will not be deleted on
successful verification, because .aria2 file is not loaded.

src/ChecksumCheckIntegrityEntry.cc

@@ -69,9 +69,7 @@ void ChecksumCheckIntegrityEntry::initValidator()
 void
 ChecksumCheckIntegrityEntry::onDownloadFinished
 (std::vector<Command*>& commands, DownloadEngine* e)
-{
-  getRequestGroup()->getDownloadContext()->setChecksumVerified(true);
-}
+{}
 
 void
 ChecksumCheckIntegrityEntry::onDownloadIncomplete

src/FtpNegotiationCommand.cc

@@ -74,6 +74,9 @@
 #include "CheckIntegrityEntry.h"
 #include "error_code.h"
 #include "SocketRecvBuffer.h"
+#ifdef ENABLE_MESSAGE_DIGEST
+# include "ChecksumCheckIntegrityEntry.h"
+#endif // ENABLE_MESSAGE_DIGEST
 
 namespace aria2 {
 
@@ -398,21 +401,32 @@ bool FtpNegotiationCommand::onFileSizeDetermined(off_t totalLength)
 
     if(getDownloadContext()->knowsTotalLength() &&
        getRequestGroup()->downloadFinishedByFileLength()) {
-      // TODO If metalink file does not contain size and it contains
-      // hash and file is not zero length, but remote server says the
-      // file size is 0, no hash check is performed in the current
-      // implementation. See also
+#ifdef ENABLE_MESSAGE_DIGEST
+      // TODO Known issue: if .aria2 file exists, it will not be
+      // deleted on successful verification, because .aria2 file is
+      // not loaded.  See also
       // HttpResponseCommand::handleOtherEncoding()
       getRequestGroup()->initPieceStorage();
-      getPieceStorage()->markAllPiecesDone();
-      getDownloadContext()->setChecksumVerified(true);
-      sequence_ = SEQ_DOWNLOAD_ALREADY_COMPLETED;
-      A2_LOG_NOTICE
-        (fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
-             getRequestGroup()->getGID(),
-             getRequestGroup()->getFirstFilePath().c_str()));
+      if(getDownloadContext()->isChecksumVerificationNeeded()) {
+        A2_LOG_DEBUG("Zero length file exists. Verify checksum.");
+        SharedHandle<ChecksumCheckIntegrityEntry> entry
+          (new ChecksumCheckIntegrityEntry(getRequestGroup()));
+        entry->initValidator();
+        getPieceStorage()->getDiskAdaptor()->openExistingFile();
+        getDownloadEngine()->getCheckIntegrityMan()->pushEntry(entry);
+        sequence_ = SEQ_EXIT;
+      } else
+#endif // ENABLE_MESSAGE_DIGEST
+        {
+          getPieceStorage()->markAllPiecesDone();
+          getDownloadContext()->setChecksumVerified(true);
+          sequence_ = SEQ_DOWNLOAD_ALREADY_COMPLETED;
+          A2_LOG_NOTICE
+            (fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
+                 getRequestGroup()->getGID(),
+                 getRequestGroup()->getFirstFilePath().c_str()));
+        }
       poolConnection();
-
       return false;
     }
 
@@ -421,8 +435,25 @@ bool FtpNegotiationCommand::onFileSizeDetermined(off_t totalLength)
     getPieceStorage()->getDiskAdaptor()->initAndOpenFile();
 
     if(getDownloadContext()->knowsTotalLength()) {
-      sequence_ = SEQ_DOWNLOAD_ALREADY_COMPLETED;
-      getPieceStorage()->markAllPiecesDone();
+      A2_LOG_DEBUG("File length becomes zero and it means download completed.");
+#ifdef ENABLE_MESSAGE_DIGEST
+      // TODO Known issue: if .aria2 file exists, it will not be
+      // deleted on successful verification, because .aria2 file is
+      // not loaded.  See also
+      // HttpResponseCommand::handleOtherEncoding()
+      if(getDownloadContext()->isChecksumVerificationNeeded()) {
+        A2_LOG_DEBUG("Verify checksum for zero-length file");
+        SharedHandle<ChecksumCheckIntegrityEntry> entry
+          (new ChecksumCheckIntegrityEntry(getRequestGroup()));
+        entry->initValidator();
+        getDownloadEngine()->getCheckIntegrityMan()->pushEntry(entry);
+        sequence_ = SEQ_EXIT;
+      } else
+#endif // ENABLE_MESSAGE_DIGEST
+        {
+          sequence_ = SEQ_DOWNLOAD_ALREADY_COMPLETED;
+          getPieceStorage()->markAllPiecesDone();
+        }
       poolConnection();
       return false;
     }

src/HttpResponseCommand.cc

@@ -74,7 +74,8 @@
 #include "SocketRecvBuffer.h"
 #include "MetalinkHttpEntry.h"
 #ifdef ENABLE_MESSAGE_DIGEST
-#include "Checksum.h"
+# include "Checksum.h"
+# include "ChecksumCheckIntegrityEntry.h"
 #endif // ENABLE_MESSAGE_DIGEST
 #ifdef HAVE_ZLIB
 # include "GZipDecodingStreamFilter.h"
@@ -431,18 +432,28 @@ bool HttpResponseCommand::handleOtherEncoding
   // For zero-length file, check existing file comparing its size
   if(!chunkedUsed && getDownloadContext()->knowsTotalLength() &&
      getRequestGroup()->downloadFinishedByFileLength()) {
-    // TODO If metalink file does not contain size and it contains
-    // hash and file is not zero length, but remote server says the
-    // file size is 0, no hash check is performed in the current
-    // implementation. See also
-    // FtpNegotiationCommand::onFileSizeDetermined()
     getRequestGroup()->initPieceStorage();
-    getPieceStorage()->markAllPiecesDone();
-    getDownloadContext()->setChecksumVerified(true);
-    A2_LOG_NOTICE
-      (fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
-           getRequestGroup()->getGID(),
-           getRequestGroup()->getFirstFilePath().c_str()));
+#ifdef ENABLE_MESSAGE_DIGEST
+    // TODO Known issue: if .aria2 file exists, it will not be deleted
+    // on successful verification, because .aria2 file is not loaded.
+    // See also FtpNegotiationCommand::onFileSizeDetermined()
+    if(getDownloadContext()->isChecksumVerificationNeeded()) {
+      A2_LOG_DEBUG("Zero length file exists. Verify checksum.");
+      SharedHandle<ChecksumCheckIntegrityEntry> entry
+        (new ChecksumCheckIntegrityEntry(getRequestGroup()));
+      entry->initValidator();
+      getPieceStorage()->getDiskAdaptor()->openExistingFile();
+      getDownloadEngine()->getCheckIntegrityMan()->pushEntry(entry);
+    } else
+#endif // ENABLE_MESSAGE_DIGEST
+      {
+        getPieceStorage()->markAllPiecesDone();
+        getDownloadContext()->setChecksumVerified(true);
+        A2_LOG_NOTICE
+          (fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
+               getRequestGroup()->getGID(),
+               getRequestGroup()->getFirstFilePath().c_str()));
+      }
     poolConnection();
     return true;
   }
@@ -455,7 +466,22 @@ bool HttpResponseCommand::handleOtherEncoding
   // is called. So zero-length file is complete if chunked encoding is
   // not used.
   if(!chunkedUsed && getDownloadContext()->knowsTotalLength()) {
-    getRequestGroup()->getPieceStorage()->markAllPiecesDone();
+    A2_LOG_DEBUG("File length becomes zero and it means download completed.");
+    // TODO Known issue: if .aria2 file exists, it will not be deleted
+    // on successful verification, because .aria2 file is not loaded.
+    // See also FtpNegotiationCommand::onFileSizeDetermined()
+#ifdef ENABLE_MESSAGE_DIGEST
+    if(getDownloadContext()->isChecksumVerificationNeeded()) {
+      A2_LOG_DEBUG("Verify checksum for zero-length file");
+      SharedHandle<ChecksumCheckIntegrityEntry> entry
+        (new ChecksumCheckIntegrityEntry(getRequestGroup()));
+      entry->initValidator();
+      getDownloadEngine()->getCheckIntegrityMan()->pushEntry(entry);
+    } else
+#endif // ENABLE_MESSAGE_DIGEST
+      {
+        getRequestGroup()->getPieceStorage()->markAllPiecesDone();
+      }
     poolConnection();
     return true;
   }

src/IteratableChecksumValidator.cc

@@ -61,23 +61,24 @@ IteratableChecksumValidator::~IteratableChecksumValidator() {}
 
 void IteratableChecksumValidator::validateChunk()
 {
-  if(!finished()) {
-    unsigned char buf[4096];
-    size_t length = pieceStorage_->getDiskAdaptor()->readData
-      (buf, sizeof(buf), currentOffset_);
-    ctx_->update(buf, length);
-    currentOffset_ += length;
-    if(finished()) {
-      std::string actualDigest = ctx_->digest();
-      if(dctx_->getDigest() == actualDigest) {
-        pieceStorage_->markAllPiecesDone();
-      } else {
-        A2_LOG_INFO(fmt("Checksum validation failed. expected=%s, actual=%s",
-                        util::toHex(dctx_->getDigest()).c_str(),
-                        util::toHex(actualDigest).c_str()));
-        BitfieldMan bitfield(dctx_->getPieceLength(), dctx_->getTotalLength());
-        pieceStorage_->setBitfield(bitfield.getBitfield(), bitfield.getBitfieldLength());
-      }
+  // Don't guard with !finished() to allow zero-length file to be
+  // verified.
+  unsigned char buf[4096];
+  size_t length = pieceStorage_->getDiskAdaptor()->readData
+    (buf, sizeof(buf), currentOffset_);
+  ctx_->update(buf, length);
+  currentOffset_ += length;
+  if(finished()) {
+    std::string actualDigest = ctx_->digest();
+    if(dctx_->getDigest() == actualDigest) {
+      pieceStorage_->markAllPiecesDone();
+      dctx_->setChecksumVerified(true);
+    } else {
+      A2_LOG_INFO(fmt("Checksum validation failed. expected=%s, actual=%s",
+                      util::toHex(dctx_->getDigest()).c_str(),
+                      util::toHex(actualDigest).c_str()));
+      BitfieldMan bitfield(dctx_->getPieceLength(), dctx_->getTotalLength());
+      pieceStorage_->setBitfield(bitfield.getBitfield(), bitfield.getBitfieldLength());
     }
   }
 }

src/RequestGroup.cc

@@ -167,10 +167,11 @@ RequestGroup::RequestGroup(const SharedHandle<Option>& option)
 
 RequestGroup::~RequestGroup() {}
 
-bool RequestGroup::isCheckIntegrityReady() const
+bool RequestGroup::isCheckIntegrityReady()
 {
   return option_->getAsBool(PREF_CHECK_INTEGRITY) &&
-    (downloadContext_->isChecksumVerificationAvailable() ||
+    ((downloadContext_->isChecksumVerificationAvailable() &&
+      downloadFinishedByFileLength()) ||
      downloadContext_->isPieceHashVerificationAvailable());
 }
 
@@ -252,27 +253,21 @@ SharedHandle<CheckIntegrityEntry> RequestGroup::createCheckIntegrityEntry()
     } 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);
-      ChecksumCheckIntegrityEntry* tempEntry =
-        new ChecksumCheckIntegrityEntry(this);
-      tempEntry->setRedownload(true);
-      checkEntry.reset(tempEntry);
-    } else
-#endif // ENABLE_MESSAGE_DIGEST
-      {
-        downloadContext_->setChecksumVerified(true);
-        A2_LOG_NOTICE(fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
-                          gid_, downloadContext_->getBasePath().c_str()));
-      }
-  } else {
+  } else if(downloadFinishedByFileLength() &&
+            downloadContext_->isChecksumVerificationAvailable()) {
+    pieceStorage_->markAllPiecesDone();
     loadAndOpenFile(infoFile);
-    checkEntry.reset(new StreamCheckIntegrityEntry(this));
-  }
+    ChecksumCheckIntegrityEntry* tempEntry =
+      new ChecksumCheckIntegrityEntry(this);
+    tempEntry->setRedownload(true);
+    checkEntry.reset(tempEntry);
+  } else
+#endif // ENABLE_MESSAGE_DIGEST
+    {
+      loadAndOpenFile(infoFile);
+      checkEntry.reset(new StreamCheckIntegrityEntry(this));
+    }
   return checkEntry;
 }
 
@@ -689,8 +684,6 @@ void RequestGroup::adjustFilename
   }
   if(infoFile->exists()) {
     // Use current filename
-  } else if(downloadFinishedByFileLength()) {
-    // File was downloaded already, no need to change file name.
   } else {
     File outfile(getFirstFilePath());    
     if(outfile.exists() && option_->getAsBool(PREF_CONTINUE) &&

src/RequestGroup.h

@@ -195,12 +195,13 @@ private:
   void removeDefunctControlFile
   (const SharedHandle<BtProgressInfoFile>& progressInfoFile);
 
-  bool isCheckIntegrityReady() const;
 public:
   RequestGroup(const SharedHandle<Option>& option);
 
   ~RequestGroup();
 
+  bool isCheckIntegrityReady();
+
   const SharedHandle<SegmentMan>& getSegmentMan() const
   {
     return segmentMan_;