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.

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);

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)
    {

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);

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

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'
 

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'

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;
 		     }