detect Releasefile IMS hits even if the server doesn't

Not all servers we are talking to support If-Modified-Since and some are
not even sending Last-Modified for us, so in an effort to detect such
hits we run a hashsum check on the 'old' compared to the 'new' file, we
got the hashes for the 'new' already for "free" from the methods anyway
and hence just need to calculated the old ones.

This allows us to detect hits even with unsupported servers, which in
turn means we benefit from all the new hit behavior also here.

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 1090912f5cdb45e08a065b1c23e0959506e37db7..78dace12c69c2e6fdf6f1ba0450f6e6d6dfd084a 100644 (file)
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -253,7 +253,10 @@ bool pkgAcquire::Item::Rename(string From,string To)
    strprintf(S, _("rename failed, %s (%s -> %s)."), strerror(errno),
         From.c_str(),To.c_str());
    Status = StatError;
-   ErrorText += S;
+   if (ErrorText.empty())
+      ErrorText = S;
+   else
+      ErrorText = ErrorText + ": " + S;
    return false;
 }
                                                                        /*}}}*/
@@ -1794,7 +1797,7 @@ void pkgAcqMetaSig::Done(string Message,unsigned long long Size,
 
    if(AuthPass == false)
    {
-      if(CheckDownloadDone(Message) == true)
+      if(CheckDownloadDone(Message, Hashes) == true)
       {
          // destfile will be modified to point to MetaIndexFile for the
          // gpgv method, so we need to save it here
@@ -1837,7 +1840,8 @@ void pkgAcqMetaSig::Failed(string Message,pkgAcquire::MethodConfig *Cnf)/*{{{*/
          Status = StatDone;
       } else {
          _error->Error("%s", downgrade_msg.c_str());
-         Rename(MetaIndexFile, MetaIndexFile+".FAILED");
+        if (TransactionManager->IMSHit == false)
+           Rename(MetaIndexFile, MetaIndexFile+".FAILED");
         Item::Failed("Message: " + downgrade_msg, Cnf);
          TransactionManager->AbortTransaction();
          return;
@@ -1922,12 +1926,12 @@ void pkgAcqMetaIndex::Done(string Message,unsigned long long Size,      /*{{{*/
 {
    Item::Done(Message,Size,Hashes,Cfg);
 
-   if(CheckDownloadDone(Message))
+   if(CheckDownloadDone(Message, Hashes))
    {
       // we have a Release file, now download the Signature, all further
       // verify/queue for additional downloads will be done in the
       // pkgAcqMetaSig::Done() code
-      std::string MetaIndexFile = DestFile;
+      std::string const MetaIndexFile = DestFile;
       new pkgAcqMetaSig(Owner, TransactionManager, 
                         MetaIndexSigURI, MetaIndexSigURIDesc,
                         MetaIndexSigShortDesc, MetaIndexFile, IndexTargets, 
@@ -2008,7 +2012,7 @@ void pkgAcqMetaBase::QueueForSignatureVerify(const std::string &MetaIndexFile,
 }
                                                                        /*}}}*/
 // pkgAcqMetaBase::CheckDownloadDone                                   /*{{{*/
-bool pkgAcqMetaBase::CheckDownloadDone(const std::string &Message)
+bool pkgAcqMetaBase::CheckDownloadDone(const std::string &Message, HashStringList const &Hashes)
 {
    // We have just finished downloading a Release file (it is not
    // verified yet)
@@ -2031,7 +2035,18 @@ bool pkgAcqMetaBase::CheckDownloadDone(const std::string &Message)
 
    // make sure to verify against the right file on I-M-S hit
    IMSHit = StringToBool(LookupTag(Message,"IMS-Hit"),false);
-   if(IMSHit)
+   if (IMSHit == false)
+   {
+      // detect IMS-Hits servers haven't detected by Hash comparison
+      std::string FinalFile = GetFinalFilename();
+      if (RealFileExists(FinalFile) && Hashes.VerifyFile(FinalFile) == true)
+      {
+        IMSHit = true;
+        unlink(DestFile.c_str());
+      }
+   }
+
+   if(IMSHit == true)
    {
       // for simplicity, the transaction manager is always InRelease
       // even if it doesn't exist.
@@ -2273,7 +2288,7 @@ void pkgAcqMetaClearSig::Done(std::string Message,unsigned long long Size,
 
    if(AuthPass == false)
    {
-      if(CheckDownloadDone(Message) == true)
+      if(CheckDownloadDone(Message, Hashes) == true)
          QueueForSignatureVerify(DestFile, DestFile);
       return;
    }

diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h
index 33a28671c00595332f93e064f08c38aa4d95b346..646de84167a0b17ce60bc5440d41bd181cdfb104 100644 (file)
--- a/apt-pkg/acquire-item.h
+++ b/apt-pkg/acquire-item.h
@@ -429,7 +429,7 @@ class pkgAcqMetaBase  : public pkgAcquire::Item                             /*{{{*/
     *  \param Message The message block received from the fetch
     *  subprocess.
     */
-   bool CheckDownloadDone(const std::string &Message);
+   bool CheckDownloadDone(const std::string &Message, HashStringList const &Hashes);
 
    /** \brief Queue the downloaded Signature for verification */
    void QueueForSignatureVerify(const std::string &MetaIndexFile,

diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc
index bee01e6200777c5b2a5a6793b05b0cc63d5c91db..9254e20a3614f4e4df0c02b41987bd49299e6cbc 100644 (file)
--- a/apt-pkg/acquire-worker.cc
+++ b/apt-pkg/acquire-worker.cc
@@ -333,13 +333,12 @@ bool pkgAcquire::Worker::RunMessages()
 
            // see if there is a hash to verify
            HashStringList ReceivedHashes;
-           HashStringList expectedHashes = Owner->HashSums();
-           for (HashStringList::const_iterator hs = expectedHashes.begin(); hs != expectedHashes.end(); ++hs)
+           for (char const * const * type = HashString::SupportedHashes(); *type != NULL; ++type)
            {
-              std::string const tagname = hs->HashType() + "-Hash";
+              std::string const tagname = std::string(*type) + "-Hash";
               std::string const hashsum = LookupTag(Message, tagname.c_str());
               if (hashsum.empty() == false)
-                 ReceivedHashes.push_back(HashString(hs->HashType(), hashsum));
+                 ReceivedHashes.push_back(HashString(*type, hashsum));
            }
 
            if(_config->FindB("Debug::pkgAcquire::Auth", false) == true)
@@ -349,6 +348,7 @@ bool pkgAcquire::Worker::RunMessages()
               for (HashStringList::const_iterator hs = ReceivedHashes.begin(); hs != ReceivedHashes.end(); ++hs)
                  std::clog <<  "\t- " << hs->toStr() << std::endl;
               std::clog << "ExpectedHash:" << endl;
+              HashStringList expectedHashes = Owner->HashSums();
               for (HashStringList::const_iterator hs = expectedHashes.begin(); hs != expectedHashes.end(); ++hs)
                  std::clog <<  "\t- " << hs->toStr() << std::endl;
               std::clog << endl;

diff --git a/methods/https.cc b/methods/https.cc
index 712e9ee7357295585bdb250dda9380934d0a8598..fa143439a97169ebf8f82739a4388538c0d48a30 100644 (file)
--- a/methods/https.cc
+++ b/methods/https.cc
@@ -444,7 +444,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
       char err[255];
       snprintf(err, sizeof(err) - 1, "HttpError%i", Server->Result);
       SetFailReason(err);
-      _error->Error("%s", err);
+      _error->Error("%i %s", Server->Result, Server->Code);
       // unlink, no need keep 401/404 page content in partial/
       unlink(File->Name().c_str());
       return false;

diff --git a/test/integration/framework b/test/integration/framework
index 2a53e83656bbc8c21ecd798d0aed530cf7dcef8d..8c8936eaddf3a9ed01ede6f201f2e075de8a891f 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1005,6 +1005,15 @@ signreleasefiles() {
        msgdone "info"
 }
 
+redatereleasefiles() {
+       local DATE="$(date -d "$1" '+%a, %d %b %Y %H:%M:%S %Z')"
+       for release in $(find aptarchive/ -name 'Release'); do
+               sed -i "s/^Date: .*$/Date: ${DATE}/" $release
+               touch -d "$DATE" $release
+       done
+       signreleasefiles "${2:-Joe Sixpack}"
+}
+
 webserverconfig() {
        local WEBSERVER="${3:-http://localhost:8080}"
        local NOCHECK=false

diff --git a/test/integration/test-apt-update-expected-size b/test/integration/test-apt-update-expected-size
index 7efccaa57494827a6428e26df0609fc8e43e2fde..55a5da848fc6560e00bb4d1a518b05e3219ff9cf 100755 (executable)
--- a/test/integration/test-apt-update-expected-size
+++ b/test/integration/test-apt-update-expected-size
@@ -26,6 +26,7 @@ test_inreleasetoobig() {
 }
 
 test_packagestoobig() {
+       redatereleasefiles '+1hour'
        # append junk at the end of the Packages.gz/Packages
        SIZE="$(stat --printf=%s aptarchive/dists/unstable/main/binary-i386/Packages)"
        find aptarchive/dists -name 'Packages*' | while read pkg; do

diff --git a/test/integration/test-apt-update-not-modified b/test/integration/test-apt-update-not-modified
index 2dc56e76cc6f4d788de46d0115bc498840ca5900..a67ecb760e8ae20e7f6744e3d9cddfe2b5e59f96 100755 (executable)
--- a/test/integration/test-apt-update-not-modified
+++ b/test/integration/test-apt-update-not-modified
@@ -12,7 +12,7 @@ insertpackage 'unstable' 'apt' 'all' '1.0'
 setupaptarchive --no-update
 
 methodtest() {
-       msgmsg 'Test with' "$1"
+       msgmsg 'Test InRelease with' "$1"
        rm -rf rootdir/var/lib/apt/lists
        # get our cache populated
        testsuccess aptget update
@@ -36,6 +36,62 @@ Reading package lists..." aptget update
 Get:1 $1 unstable/main amd64 Packages [$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') B]
 Reading package lists..." aptget update
        testfileequal 'listsdir.lst' "$(listcurrentlistsdirectory)"
+
+       webserverconfig 'aptwebserver::support::modified-since' 'false'
+       webserverconfig 'aptwebserver::support::last-modified' 'false'
+       testsuccessequal "Get:1 $1 unstable InRelease [$(stat -c '%s' 'aptarchive/dists/unstable/InRelease') B]
+Reading package lists..." aptget update
+       webserverconfig 'aptwebserver::support::modified-since' 'true'
+       webserverconfig 'aptwebserver::support::last-modified' 'true'
+
+       msgmsg 'Test Release.gpg with' "$1"
+       rm -rf rootdir/var/lib/apt/lists
+       cp -a aptarchive/dists  aptarchive/dists.good
+       find aptarchive/dists -name 'InRelease' -delete
+       # get our cache populated
+       testsuccess aptget update
+       listcurrentlistsdirectory > listsdir.lst
+
+       # hit again with a good cache
+       testsuccessequal "Ign $1 unstable InRelease
+  404  Not Found
+Hit $1 unstable Release
+Hit $1 unstable Release.gpg
+Reading package lists..." aptget update
+       testfileequal 'listsdir.lst' "$(listcurrentlistsdirectory)"
+
+       # drop an architecture, which means the file should be gone now
+       configarchitecture 'i386'
+       sed '/_binary-amd64_Packages/ d' listsdir.lst > listsdir-without-amd64.lst
+       testsuccessequal "Ign $1 unstable InRelease
+  404  Not Found
+Hit $1 unstable Release
+Hit $1 unstable Release.gpg
+Reading package lists..." aptget update
+       testfileequal 'listsdir-without-amd64.lst' "$(listcurrentlistsdirectory)"
+
+       # readd arch so its downloaded again
+       configarchitecture 'amd64' 'i386'
+       testsuccessequal "Ign $1 unstable InRelease
+  404  Not Found
+Hit $1 unstable Release
+Hit $1 unstable Release.gpg
+Get:1 $1 unstable/main amd64 Packages [$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') B]
+Reading package lists..." aptget update
+       testfileequal 'listsdir.lst' "$(listcurrentlistsdirectory)"
+
+       webserverconfig 'aptwebserver::support::modified-since' 'false'
+       webserverconfig 'aptwebserver::support::last-modified' 'false'
+       testsuccessequal "Ign $1 unstable InRelease
+  404  Not Found
+Get:1 $1 unstable Release [$(stat -c '%s' 'aptarchive/dists/unstable/Release') B]
+Get:2 $1 unstable Release.gpg [$(stat -c '%s' 'aptarchive/dists/unstable/Release.gpg') B]
+Reading package lists..." aptget update
+       webserverconfig 'aptwebserver::support::modified-since' 'true'
+       webserverconfig 'aptwebserver::support::last-modified' 'true'
+
+       rm -rf aptarchive/dists
+       cp -a aptarchive/dists.good aptarchive/dists
 }
 
 changetowebserver

diff --git a/test/integration/test-apt-update-rollback b/test/integration/test-apt-update-rollback
index f4500b69d544b4d7806ab01f922de8aa9d118936..29fe1ab5627a89955c5b8116a78e01027da02364 100755 (executable)
--- a/test/integration/test-apt-update-rollback
+++ b/test/integration/test-apt-update-rollback
@@ -158,6 +158,7 @@ test_inrelease_to_broken_gzip() {
     msgmsg "Test InRelease to broken gzip"
     start_with_good_inrelease
 
+    redatereleasefiles '+2hours'
     # append junk at the end of the compressed file
     echo "lala" >> $APTARCHIVE/dists/unstable/main/source/Sources.gz
     touch -d '+2min' $APTARCHIVE/dists/unstable/main/source/Sources.gz

diff --git a/test/integration/test-apt-update-transactions b/test/integration/test-apt-update-transactions
index 67dd633f92659277bc8fb76e14c69ca5c3be19fa..63b31805600ffc05a34e6ade40faf48c49a8c89a 100755 (executable)
--- a/test/integration/test-apt-update-transactions
+++ b/test/integration/test-apt-update-transactions
@@ -47,6 +47,7 @@ testrun() {
 
 testsetup() {
        msgmsg 'Test with no initial data over' "$1"
+       redatereleasefiles 'now'
        rm -rf rootdir/var/lib/apt/lists
        mkdir -p rootdir/var/lib/apt/lists/partial
        listcurrentlistsdirectory > listsdir.lst
@@ -55,6 +56,7 @@ testsetup() {
        msgmsg 'Test with initial data over' "$1"
        testsuccess aptget update
        listcurrentlistsdirectory > listsdir.lst
+       redatereleasefiles '+1hour'
        testrun 'listsdir.lst'
 }