do not request files if we expect an IMS hit

If we have a file on disk and the hashes are the same in the new Release
file and the old one we have on disk we know that if we ask the server
for the file, we will at best get an IMS hit – at worse the server
doesn't support this and sends us the (unchanged) file and we have to
run all our checks on it again for nothing. So, we can save ourselves
(and the servers) some unneeded requests if we figure this out on our
own.

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 13e971e9ff7a1540075b5d32279e5c8d77772f39..511bbbc643166975ddd0cb0601c38483d1e993a3 100644 (file)
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -119,6 +119,16 @@ static bool AllowInsecureRepositories(indexRecords const * const MetaIndexParser
    return false;
 }
                                                                        /*}}}*/
+static HashStringList GetExpectedHashesFromFor(indexRecords * const Parser, std::string const MetaKey)/*{{{*/
+{
+   if (Parser == NULL)
+      return HashStringList();
+   indexRecords::checkSum * const R = Parser->Lookup(MetaKey);
+   if (R == NULL)
+      return HashStringList();
+   return R->Hashes;
+}
+                                                                       /*}}}*/
 
 // all ::HashesRequired and ::GetExpectedHashes implementations                /*{{{*/
 /* ::GetExpectedHashes is abstract and has to be implemented by all subclasses.
@@ -372,6 +382,25 @@ bool pkgAcqDiffIndex::TransactionState(TransactionStates const state)
 }
                                                                        /*}}}*/
 
+class APT_HIDDEN NoActionItem : public pkgAcquire::Item                        /*{{{*/
+/* The sole purpose of this class is having an item which does nothing to
+   reach its done state to prevent cleanup deleting the mentioned file.
+   Handy in cases in which we know we have the file already, like IMS-Hits. */
+{
+   IndexTarget const * const Target;
+   public:
+   virtual std::string DescURI() const {return Target->URI;};
+   virtual HashStringList GetExpectedHashes()  const {return HashStringList();};
+
+   NoActionItem(pkgAcquire * const Owner, IndexTarget const * const Target) :
+      pkgAcquire::Item(Owner), Target(Target)
+   {
+      Status = StatDone;
+      DestFile = GetFinalFileNameFromURI(Target->URI);
+   }
+};
+                                                                       /*}}}*/
+
 // Acquire::Item::Item - Constructor                                   /*{{{*/
 APT_IGNORE_DEPRECATED_PUSH
 pkgAcquire::Item::Item(pkgAcquire * const Owner) :
@@ -644,12 +673,7 @@ pkgAcqTransactionItem::~pkgAcqTransactionItem()                            /*{{{*/
                                                                        /*}}}*/
 HashStringList pkgAcqTransactionItem::GetExpectedHashesFor(std::string const MetaKey) const    /*{{{*/
 {
-   if (TransactionManager->MetaIndexParser == NULL)
-      return HashStringList();
-   indexRecords::checkSum * const R = TransactionManager->MetaIndexParser->Lookup(MetaKey);
-   if (R == NULL)
-      return HashStringList();
-   return R->Hashes;
+   return GetExpectedHashesFromFor(TransactionManager->MetaIndexParser, MetaKey);
 }
                                                                        /*}}}*/
 
@@ -939,6 +963,23 @@ void pkgAcqMetaBase::QueueIndexes(bool const verify)                       /*{{{*/
            return;
         }
 
+        if (RealFileExists(GetFinalFileNameFromURI((*Target)->URI)))
+        {
+           if (TransactionManager->LastMetaIndexParser != NULL)
+           {
+              HashStringList const newFile = GetExpectedHashesFromFor(TransactionManager->MetaIndexParser, (*Target)->MetaKey);
+              HashStringList const oldFile = GetExpectedHashesFromFor(TransactionManager->LastMetaIndexParser, (*Target)->MetaKey);
+              if (newFile == oldFile)
+              {
+                 // we have the file already, no point in trying to acquire it again
+                 new NoActionItem(Owner, *Target);
+                 continue;
+              }
+           }
+        }
+        else
+           trypdiff = false; // no file to patch
+
         // check if we have patches available
         trypdiff &= TransactionManager->MetaIndexParser->Exists((*Target)->MetaKey + ".diff/Index");
       }
@@ -1085,20 +1126,6 @@ string pkgAcqMetaClearSig::Custom600Headers() const
 }
                                                                        /*}}}*/
 // pkgAcqMetaClearSig::Done - We got a file                            /*{{{*/
-class APT_HIDDEN DummyItem : public pkgAcquire::Item
-{
-   IndexTarget const * const Target;
-   public:
-   virtual std::string DescURI() const {return Target->URI;};
-   virtual HashStringList GetExpectedHashes()  const {return HashStringList();};
-
-   DummyItem(pkgAcquire * const Owner, IndexTarget const * const Target) :
-      pkgAcquire::Item(Owner), Target(Target)
-   {
-      Status = StatDone;
-      DestFile = GetFinalFileNameFromURI(Target->URI);
-   }
-};
 void pkgAcqMetaClearSig::Done(std::string const &Message,
                               HashStringList const &Hashes,
                               pkgAcquire::MethodConfig const * const Cnf)
@@ -1131,8 +1158,8 @@ void pkgAcqMetaClearSig::Done(std::string const &Message,
         // We got an InRelease file IMSHit, but we haven't one, which means
         // we had a valid Release/Release.gpg combo stepping in, which we have
         // to 'acquire' now to ensure list cleanup isn't removing them
-        new DummyItem(Owner, &DetachedDataTarget);
-        new DummyItem(Owner, &DetachedSigTarget);
+        new NoActionItem(Owner, &DetachedDataTarget);
+        new NoActionItem(Owner, &DetachedSigTarget);
       }
    }
 }
@@ -1578,11 +1605,7 @@ bool pkgAcqDiffIndex::ParseDiffIndex(string const &IndexDiffFile)        /*{{{*/
    HashStringList LocalHashes;
    // try avoiding calculating the hash here as this is costly
    if (TransactionManager->LastMetaIndexParser != NULL)
-   {
-      indexRecords::checkSum * const R = TransactionManager->LastMetaIndexParser->Lookup(Target->MetaKey);
-      if (R != NULL)
-        LocalHashes = R->Hashes;
-   }
+      LocalHashes = GetExpectedHashesFromFor(TransactionManager->LastMetaIndexParser, Target->MetaKey);
    if (LocalHashes.usable() == false)
    {
       FileFd fd(CurrentPackagesFile, FileFd::ReadOnly);

diff --git a/test/integration/framework b/test/integration/framework
index b253deb915d5ee93a3eec3a8521ba691787da0da..56c4a12161611fa5f98f603a76fb615050e67d58 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -835,7 +835,9 @@ buildaptarchivefromincoming() {
 
 buildaptarchivefromfiles() {
        msginfo "Build APT archive for ${CCMD}$(basename $0)${CINFO} based on prebuild files…"
-       find aptarchive -name 'Packages' -o -name 'Sources' -o -name 'Translation-*' | while read line; do
+       local DIR='aptarchive'
+       if [ -d "${DIR}/dists" ]; then DIR="${DIR}/dists"; fi
+       find "$DIR" -name 'Packages' -o -name 'Sources' -o -name 'Translation-*' | while read line; do
                msgninfo "\t${line} file… "
                compressfile "$line" "$1"
                msgdone "info"

diff --git a/test/integration/test-apt-update-expected-size b/test/integration/test-apt-update-expected-size
index 55bba81885f0b045c2fca0084114aaa04cb59b90..24ca8513325f2149425757d2ac079a9fe4e7b795 100755 (executable)
--- a/test/integration/test-apt-update-expected-size
+++ b/test/integration/test-apt-update-expected-size
@@ -26,7 +26,9 @@ test_inreleasetoobig() {
 }
 
 test_packagestoobig() {
-       redatereleasefiles '+1hour'
+       insertpackage 'unstable' 'foo' 'all' '1.0'
+       buildaptarchivefromfiles '+1 hour'
+       signreleasefiles
        # 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-file b/test/integration/test-apt-update-file
index 665f94fa590546935b7ab094de2ff58da9db64c6..94b604f0e7dc9b88cf991e2984f05e85054061a6 100755 (executable)
--- a/test/integration/test-apt-update-file
+++ b/test/integration/test-apt-update-file
@@ -26,14 +26,29 @@ testsuccess aptget update
 # the release files aren't an IMS-hit, but the indexes are
 redatereleasefiles '+1 hour'
 
+# we don't download the index if it isn't updated
 testsuccess aptget update -o Debug::pkgAcquire::Auth=1
+# file:/ isn't shown in the log, so see if it was downloaded anyhow
 cp -a rootdir/tmp/testsuccess.output rootdir/tmp/update.output
+canary="SHA512:$(bzcat aptarchive/dists/unstable/main/binary-amd64/Packages.bz2 | sha512sum |cut -f1 -d' ')"
+testfailure grep -- "$canary" rootdir/tmp/update.output
+
+testfoo() {
+       # foo is still available
+       testsuccess aptget install -s foo
+       testsuccess aptcache showsrc foo
+       testsuccess aptget source foo --print-uris
+}
+testfoo
+
+# the release file is new again, the index still isn't, but it is somehow gone now from disk
+redatereleasefiles '+2 hour'
+find rootdir/var/lib/apt/lists -name '*_Packages*' -delete
 
-# ensure that the hash of the uncompressed file was verified even on a local ims hit
+testsuccess aptget update -o Debug::pkgAcquire::Auth=1
+# file:/ isn't shown in the log, so see if it was downloaded anyhow
+cp -a rootdir/tmp/testsuccess.output rootdir/tmp/update.output
 canary="SHA512:$(bzcat aptarchive/dists/unstable/main/binary-amd64/Packages.bz2 | sha512sum |cut -f1 -d' ')"
 testsuccess grep -- "$canary" rootdir/tmp/update.output
 
-# foo is still available
-testsuccess aptget install -s foo
-testsuccess aptcache showsrc foo
-testsuccess aptget source foo --print-uris
+testfoo

diff --git a/test/integration/test-apt-update-not-modified b/test/integration/test-apt-update-not-modified
index a490f00de3bfc246676c3dbf3d3e582cb7bcc030..32818658fc6048f5da086c35859f37fad14feac1 100755 (executable)
--- a/test/integration/test-apt-update-not-modified
+++ b/test/integration/test-apt-update-not-modified
@@ -133,6 +133,25 @@ Reading package lists..." aptget update
 
        rm -rf aptarchive/dists
        cp -a aptarchive/dists.good aptarchive/dists
+
+       # new release file, but the indexes are the same
+       redatereleasefiles '+2 hours'
+
+       rm -rf rootdir/var/lib/apt/lists.good
+       cp -a rootdir/var/lib/apt/lists rootdir/var/lib/apt/lists.good
+       testsuccessequal "Get:1 $1 unstable InRelease [$(stat -c '%s' 'aptarchive/dists/unstable/InRelease') B]
+Reading package lists..." aptget update
+
+       rm -rf rootdir/var/lib/apt/lists
+       cp -a rootdir/var/lib/apt/lists.good rootdir/var/lib/apt/lists
+       find rootdir/var/lib/apt/lists -name '*_Packages*' -delete
+       testsuccessequal "Get:1 $1 unstable InRelease [$(stat -c '%s' 'aptarchive/dists/unstable/InRelease') B]
+Get:2 $1 unstable/main amd64 Packages [$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') B]
+Get:3 $1 unstable/main i386 Packages [$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-i386/Packages.gz') B]
+Reading package lists..." aptget update
+
+       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 6fd9017158671fa5d259b1c3fd8379bc12fb022e..6ecf322b209fbcbcec4bc961f8ec0770056e950b 100755 (executable)
--- a/test/integration/test-apt-update-rollback
+++ b/test/integration/test-apt-update-rollback
@@ -158,7 +158,10 @@ test_inrelease_to_broken_gzip() {
     msgmsg "Test InRelease to broken gzip"
     start_with_good_inrelease
 
-    redatereleasefiles '+2hours'
+    break_repository_sources_index '+1hour'
+    generatereleasefiles '+2hours'
+    signreleasefiles
+
     # 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
@@ -166,6 +169,7 @@ test_inrelease_to_broken_gzip() {
     rm $APTARCHIVE/dists/unstable/main/source/Sources
 
     testfailure aptget update
+    testsuccess grep 'Hash Sum mismatch' rootdir/tmp/testfailure.output
     testfileequal lists.before "$(listcurrentlistsdirectory)"
 }
 

diff --git a/test/integration/test-apt-update-transactions b/test/integration/test-apt-update-transactions
index 152e1617a0d1bc614416e649cc2cf8e4fd05b916..a5dac1737a74d60b9b004a0dacfc5e9f9f740e65 100755 (executable)
--- a/test/integration/test-apt-update-transactions
+++ b/test/integration/test-apt-update-transactions
@@ -29,6 +29,12 @@ restorefile() {
 }
 
 testrun() {
+       rm -rf aptarchive/dists.good
+       cp -a aptarchive/dists aptarchive/dists.good
+       insertpackage 'unstable' 'bar' 'all' '1.0'
+       insertsource 'unstable' 'bar' 'all' '1.0'
+       buildaptarchivefromfiles '+1 hour'
+
        # produce an unsigned repository
        find aptarchive \( -name 'Release.gpg' -o -name 'InRelease' \) -delete
        testfailure aptget update --no-allow-insecure-repositories
@@ -37,20 +43,27 @@ testrun() {
        # signed but broken
        signreleasefiles
 
+       onehashbroken() {
+               testfailure aptget update
+               # each file generates two messages with this string
+               testequal '2' grep --count 'Hash Sum mismatch' rootdir/tmp/testfailure.output
+               testfileequal "$1" "$(listcurrentlistsdirectory)"
+       }
+
        breakfile aptarchive/dists/unstable/main/binary-i386/Packages
-       testfailure aptget update
-       testfileequal "$1" "$(listcurrentlistsdirectory)"
+       onehashbroken "$1"
        restorefile aptarchive/dists/unstable/main/binary-i386/Packages
 
        breakfile aptarchive/dists/unstable/main/source/Sources
-       testfailure aptget update
-       testfileequal "$1" "$(listcurrentlistsdirectory)"
+       onehashbroken "$1"
        restorefile aptarchive/dists/unstable/main/source/Sources
+
+       rm -rf aptarchive/dists
+       cp -a aptarchive/dists.good aptarchive/dists
 }
 
 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
@@ -60,7 +73,6 @@ testsetup() {
        rm -rf rootdir/var/lib/apt/lists
        testsuccess aptget update -o Debug::pkgAcquire::Worker=1
        listcurrentlistsdirectory > listsdir.lst
-       redatereleasefiles '+1hour'
        testrun 'listsdir.lst'
 }
 

diff --git a/test/integration/test-cve-2013-1051-InRelease-parsing b/test/integration/test-cve-2013-1051-InRelease-parsing
index e38e40cc91c896ed954aa58204e1db5148988104..d9917455320fb159ff04edf8409b4764f5e7e2ad 100755 (executable)
--- a/test/integration/test-cve-2013-1051-InRelease-parsing
+++ b/test/integration/test-cve-2013-1051-InRelease-parsing
@@ -39,10 +39,15 @@ sed -i '/^-----BEGIN PGP SIGNATURE-----/,/^-----END PGP SIGNATURE-----/ s/^$/  /
 cat aptarchive/dists/stable/Release >> aptarchive/dists/stable/InRelease
 touch -d '+1hour' aptarchive/dists/stable/InRelease
 
-# ensure the update fails
-# useful for debugging to add "-o Debug::pkgAcquire::auth=true"
-msgtest 'apt-get update for should fail with the modified' 'InRelease'
-aptget update 2>&1 | grep -E -q '(Writing more data than expected|Hash Sum mismatch)' > /dev/null && msgpass || msgfail
+# ensure the update doesn't load bad data as good data
+# Note that we will pick up the InRelease itself as we download no other
+# indexes which would trigger a hashsum mismatch, but we ignore the 'bad'
+# part of the InRelease
+listcurrentlistsdirectory | sed '/_InRelease/ d' > listsdir.lst
+msgtest 'apt-get update should ignore unsigned data in the' 'InRelease'
+testsuccessequal "Get:1 http://localhost:8080 stable InRelease [$(stat -c%s aptarchive/dists/stable/InRelease) B]
+Reading package lists..." --nomsg aptget update
+testfileequal './listsdir.lst' "$(listcurrentlistsdirectory | sed '/_InRelease/ d')"
 
 # ensure there is no package
 testfailureequal 'Reading package lists...