detect 416 complete file in partial by expected hash

If we have the expected hashes we can check with them if the file we
have in partial we got a 416 for is the expected file. We detected this
with same-size before, but not every server sends a good Content-Range
header with a 416 response.

diff --git a/methods/https.cc b/methods/https.cc
index c6b75d9ad151a5b377a3b2d82b203a36c49f380d..712e9ee7357295585bdb250dda9380934d0a8598 100644 (file)
--- a/methods/https.cc
+++ b/methods/https.cc
@@ -40,7 +40,9 @@ using namespace std;
 struct APT_HIDDEN CURLUserPointer {
    HttpsMethod * const https;
    HttpsMethod::FetchResult * const Res;
-   CURLUserPointer(HttpsMethod * const https, HttpsMethod::FetchResult * const Res) : https(https), Res(Res) {}
+   HttpsMethod::FetchItem const * const Itm;
+   CURLUserPointer(HttpsMethod * const https, HttpsMethod::FetchResult * const Res,
+        HttpsMethod::FetchItem const * const Itm) : https(https), Res(Res), Itm(Itm) {}
 };
 
 size_t
@@ -61,13 +63,32 @@ HttpsMethod::parse_header(void *buffer, size_t size, size_t nmemb, void *userp)
    {
       if (me->https->Server->Result != 416 && me->https->Server->StartPos != 0)
         ;
-      else if (me->https->Server->Result == 416 && me->https->Server->Size == me->https->File->FileSize())
+      else if (me->https->Server->Result == 416)
       {
-         me->https->Server->Result = 200;
-        me->https->Server->StartPos = me->https->Server->Size;
-        // the actual size is not important for https as curl will deal with it
-        // by itself and e.g. doesn't bother us with transport-encoding…
-        me->https->Server->JunkSize = std::numeric_limits<unsigned long long>::max();
+        bool partialHit = false;
+        if (me->Itm->ExpectedHashes.usable() == true)
+        {
+           Hashes resultHashes(me->Itm->ExpectedHashes);
+           FileFd file(me->Itm->DestFile, FileFd::ReadOnly);
+           me->https->Server->Size = file.FileSize();
+           me->https->Server->Date = file.ModificationTime();
+           resultHashes.AddFD(file);
+           HashStringList const hashList = resultHashes.GetHashStringList();
+           partialHit = (me->Itm->ExpectedHashes == hashList);
+        }
+        else if (me->https->Server->Result == 416 && me->https->Server->Size == me->https->File->FileSize())
+           partialHit = true;
+
+        if (partialHit == true)
+        {
+           me->https->Server->Result = 200;
+           me->https->Server->StartPos = me->https->Server->Size;
+           // the actual size is not important for https as curl will deal with it
+           // by itself and e.g. doesn't bother us with transport-encoding…
+           me->https->Server->JunkSize = std::numeric_limits<unsigned long long>::max();
+        }
+        else
+           me->https->Server->StartPos = 0;
       }
       else
         me->https->Server->StartPos = 0;
@@ -221,7 +242,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
    maybe_add_auth (Uri, _config->FindFile("Dir::Etc::netrc"));
 
    FetchResult Res;
-   CURLUserPointer userp(this, &Res);
+   CURLUserPointer userp(this, &Res, Itm);
    // callbacks
    curl_easy_setopt(curl, CURLOPT_URL, static_cast<string>(Uri).c_str());
    curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, parse_header);

diff --git a/methods/https.h b/methods/https.h
index 6e32e8d3d0318302d7abf17ac4a65e7c84fb2ecf..57fc292ee2d68ac498c1e486c98eec2ee666709d 100644 (file)
--- a/methods/https.h
+++ b/methods/https.h
@@ -79,6 +79,7 @@ class HttpsMethod : public ServerMethod
    virtual bool Configuration(std::string Message);
    virtual ServerState * CreateServerState(URI uri);
    using pkgAcqMethod::FetchResult;
+   using pkgAcqMethod::FetchItem;
 
    HttpsMethod() : ServerMethod("1.2",Pipeline | SendConfig), File(NULL)
    {

diff --git a/methods/server.cc b/methods/server.cc
index 2116926b063a3e7abe503549dd8fc4b06ff36693..bd01c3e980e2195a3403b1d5a0dbbcf56f5aeb84 100644 (file)
--- a/methods/server.cc
+++ b/methods/server.cc
@@ -269,7 +269,7 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
       Res.LastModified = Queue->LastModified;
       return IMS_HIT;
    }
-   
+
    /* Redirect
     *
     * Note that it is only OK for us to treat all redirection the same
@@ -314,7 +314,20 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
       struct stat SBuf;
       if (stat(Queue->DestFile.c_str(),&SBuf) >= 0 && SBuf.st_size > 0)
       {
-        if ((unsigned long long)SBuf.st_size == Server->Size)
+        bool partialHit = false;
+        if (Queue->ExpectedHashes.usable() == true)
+        {
+           Hashes resultHashes(Queue->ExpectedHashes);
+           FileFd file(Queue->DestFile, FileFd::ReadOnly);
+           Server->Size = file.FileSize();
+           Server->Date = file.ModificationTime();
+           resultHashes.AddFD(file);
+           HashStringList const hashList = resultHashes.GetHashStringList();
+           partialHit = (Queue->ExpectedHashes == hashList);
+        }
+        else if ((unsigned long long)SBuf.st_size == Server->Size)
+           partialHit = true;
+        if (partialHit == true)
         {
            // the file is completely downloaded, but was not moved
            if (Server->HaveContent == true)
@@ -351,7 +364,7 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
    // This is some sort of 2xx 'data follows' reply
    Res.LastModified = Server->Date;
    Res.Size = Server->Size;
-   
+
    // Open the file
    delete File;
    File = new FileFd(Queue->DestFile,FileFd::WriteAny);

diff --git a/test/integration/framework b/test/integration/framework
index 03c18818998ea736a7b768c83d3946a8e5709540..2a53e83656bbc8c21ecd798d0aed530cf7dcef8d 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1128,7 +1128,7 @@ acquire::cdrom::autodetect 0;" > rootdir/etc/apt/apt.conf.d/00cdrom
 downloadfile() {
        local PROTO="${1%%:*}"
        if ! apthelper -o Debug::Acquire::${PROTO}=1 -o Debug::pkgAcquire::Worker=1 \
-               download-file "$1" "$2" 2>&1 ; then
+               download-file "$1" "$2" "$3" 2>&1 ; then
                return 1
        fi
        # only if the file exists the download was successful

diff --git a/test/integration/test-apt-update-transactions b/test/integration/test-apt-update-transactions
index f028ac0c771bcdf1b3dea75f4554ff9bea2e1ef9..67dd633f92659277bc8fb76e14c69ca5c3be19fa 100755 (executable)
--- a/test/integration/test-apt-update-transactions
+++ b/test/integration/test-apt-update-transactions
@@ -63,6 +63,7 @@ testsetup 'file'
 changetowebserver
 webserverconfig 'aptwebserver::support::modified-since' 'false' "$1"
 webserverconfig 'aptwebserver::support::last-modified' 'false' "$1"  # curl is clever and sees hits here also
+webserverconfig 'aptwebserver::support::range' 'false' "$1"
 
 testsetup 'http'
 

diff --git a/test/integration/test-partial-file-support b/test/integration/test-partial-file-support
index 85046b3eb92616369d45ace64cf5c671ebda0988..c07af7bd0f39484371cb14f513abdbd18b53a534 100755 (executable)
--- a/test/integration/test-partial-file-support
+++ b/test/integration/test-partial-file-support
@@ -17,8 +17,8 @@ DOWNLOADLOG='rootdir/tmp/testdownloadfile.log'
 
 testdownloadfile() {
        rm -f "$DOWNLOADLOG"
-       msgtest "Testing download of file $2 with" "$1"
-       if ! downloadfile "$2" "$3" > "$DOWNLOADLOG"; then
+       msgtest "Testing download of file $2 with" "$1 $5"
+       if ! downloadfile "$2" "$3" "$5" > "$DOWNLOADLOG"; then
                cat >&2 "$DOWNLOADLOG"
                msgfail
        else
@@ -78,6 +78,12 @@ followuprequest() {
        testdownloadfile 'completely downloaded file' "${1}/testfile" "$DOWN" '='
        testwebserverlaststatuscode '416' "$DOWNLOADLOG"
 
+       webserverconfig 'aptwebserver::support::content-range' 'false'
+       copysource $TESTFILE 1M $DOWN
+       testdownloadfile 'completely downloaded file' "${1}/testfile" "$DOWN" '=' "SHA1:$(sha1sum "$TESTFILE" | cut -d' ' -f 1)"
+       testwebserverlaststatuscode '416' "$DOWNLOADLOG"
+       webserverconfig 'aptwebserver::support::content-range' 'true'
+
        copysource $TESTFILE 1M $DOWN
        copysource "${TESTFILE}2" 20 "${DOWN}2"
        msgtest 'Testing download of files with' 'completely downloaded file + partial file'

diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc
index 6a411e24ea1e18a5fe11bf87c4ef0698f0572f07..c933060e75e7918a7eec6fc09afa456da3f2c50c 100644 (file)
--- a/test/interactive-helper/aptwebserver.cc
+++ b/test/interactive-helper/aptwebserver.cc
@@ -745,9 +745,12 @@ static void * handleClient(void * voidclient)                              /*{{{*/
                     }
                     else
                     {
-                       std::ostringstream contentrange;
-                       contentrange << "Content-Range: bytes */" << filesize;
-                       headers.push_back(contentrange.str());
+                       if (_config->FindB("aptwebserver::support::content-range", true) == true)
+                       {
+                          std::ostringstream contentrange;
+                          contentrange << "Content-Range: bytes */" << filesize;
+                          headers.push_back(contentrange.str());
+                       }
                        sendError(client, 416, *m, sendContent, "", headers);
                        break;
                     }