From d94b1d80d8326334d17f6a43061368e783b8e0aa Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 11 Aug 2016 18:24:35 +0200 Subject: don't sent Range requests if we know its not accepted If the server told us in a previous request that it isn't supporting Ranges with bytes via an Accept-Ranges header missing bytes, we don't try to formulate requests using Ranges. --- methods/http.cc | 2 +- methods/https.cc | 15 ++++++++------- methods/server.cc | 10 ++++++++++ methods/server.h | 1 + test/integration/test-apt-update-transactions | 2 +- test/integration/test-bug-lp1445239-download-loop | 1 - test/integration/test-partial-file-support | 8 +++++++- test/integration/test-releasefile-verification | 2 -- test/interactive-helper/aptwebserver.cc | 9 +++++++++ 9 files changed, 37 insertions(+), 13 deletions(-) diff --git a/methods/http.cc b/methods/http.cc index 8d34aa6e3..11136bb9a 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -906,7 +906,7 @@ void HttpMethod::SendReq(FetchItem *Itm) // Check for a partial file and send if-queries accordingly struct stat SBuf; - if (stat(Itm->DestFile.c_str(),&SBuf) >= 0 && SBuf.st_size > 0) + if (Server->RangesAllowed && stat(Itm->DestFile.c_str(),&SBuf) >= 0 && SBuf.st_size > 0) Req << "Range: bytes=" << std::to_string(SBuf.st_size) << "-\r\n" << "If-Range: " << TimeRFC1123(SBuf.st_mtime, false) << "\r\n"; else if (Itm->LastModified != 0) diff --git a/methods/https.cc b/methods/https.cc index c86f9407e..b2d05136c 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -382,8 +382,15 @@ bool HttpsMethod::Fetch(FetchItem *Itm) headers = curl_slist_append(headers, "Accept: text/*"); } + // go for it - if the file exists, append on it + File = new FileFd(Itm->DestFile, FileFd::WriteAny); + if (Server == nullptr || Server->Comp(Itm->Uri) == false) + Server = CreateServerState(Itm->Uri); + else + Server->Reset(false); + // if we have the file send an if-range query with a range header - if (stat(Itm->DestFile.c_str(),&SBuf) >= 0 && SBuf.st_size > 0) + if (Server->RangesAllowed && stat(Itm->DestFile.c_str(),&SBuf) >= 0 && SBuf.st_size > 0) { std::string Buf; strprintf(Buf, "Range: bytes=%lli-", (long long) SBuf.st_size); @@ -397,12 +404,6 @@ bool HttpsMethod::Fetch(FetchItem *Itm) curl_easy_setopt(curl, CURLOPT_TIMEVALUE, Itm->LastModified); } - // go for it - if the file exists, append on it - File = new FileFd(Itm->DestFile, FileFd::WriteAny); - if (Server == nullptr || Server->Comp(Itm->Uri) == false) - Server = CreateServerState(Itm->Uri); - else - Server->Reset(false); if (Server->InitHashes(Itm->ExpectedHashes) == false) return false; diff --git a/methods/server.cc b/methods/server.cc index d0b6ef3d7..aa1ee4754 100644 --- a/methods/server.cc +++ b/methods/server.cc @@ -227,6 +227,15 @@ bool ServerState::HeaderLine(string Line) return true; } + if (stringcasecmp(Tag, "Accept-Ranges:") == 0) + { + std::string ranges = ',' + Val + ','; + ranges.erase(std::remove(ranges.begin(), ranges.end(), ' '), ranges.end()); + if (ranges.find(",bytes,") == std::string::npos) + RangesAllowed = false; + return true; + } + return true; } /*}}}*/ @@ -252,6 +261,7 @@ void ServerState::Reset(bool const Everything) /*{{{*/ if (Everything) { Persistent = false; Pipeline = false; PipelineAllowed = true; + RangesAllowed = true; } } /*}}}*/ diff --git a/methods/server.h b/methods/server.h index 63acceae6..c3adba87a 100644 --- a/methods/server.h +++ b/methods/server.h @@ -51,6 +51,7 @@ struct ServerState enum {Header, Data} State; bool Persistent; bool PipelineAllowed; + bool RangesAllowed; std::string Location; // This is a Persistent attribute of the server itself. diff --git a/test/integration/test-apt-update-transactions b/test/integration/test-apt-update-transactions index d8154b283..ab678c133 100755 --- a/test/integration/test-apt-update-transactions +++ b/test/integration/test-apt-update-transactions @@ -82,7 +82,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" +webserverconfig 'aptwebserver::response-header::Accept-Ranges' 'none' "$1" testsetup 'http' diff --git a/test/integration/test-bug-lp1445239-download-loop b/test/integration/test-bug-lp1445239-download-loop index a12d5252d..6802840a5 100755 --- a/test/integration/test-bug-lp1445239-download-loop +++ b/test/integration/test-bug-lp1445239-download-loop @@ -12,7 +12,6 @@ setupenvironment configarchitecture 'amd64' changetowebserver -webserverconfig 'aptwebserver::support::range' 'true' TESTFILE='aptarchive/testfile' dd if=/dev/zero of=$TESTFILE bs=100k count=1 2>/dev/null diff --git a/test/integration/test-partial-file-support b/test/integration/test-partial-file-support index 1c5d120d8..9b5eed1e5 100755 --- a/test/integration/test-partial-file-support +++ b/test/integration/test-partial-file-support @@ -96,6 +96,7 @@ followuprequest() { testrun() { webserverconfig 'aptwebserver::support::range' 'true' + webserverconfig 'aptwebserver::response-header::Accept-Ranges' 'bytes' local DOWN='./downloaded/testfile' copysource $TESTFILE 0 $DOWN @@ -125,7 +126,11 @@ testrun() { testdownloadfile 'old data' "${1}/testfile" "$DOWN" '=' testwebserverlaststatuscode '200' "$DOWNLOADLOG" - webserverconfig 'aptwebserver::support::range' 'false' + if [ "${1%%:*}" = 'https' ] && expr match "$1" "^.*/redirectme$" >/dev/null; then + webserverconfig 'aptwebserver::response-header::Accept-Ranges' 'none' + else + webserverconfig 'aptwebserver::support::range' 'false' + fi copysource $TESTFILE 20 $DOWN testdownloadfile 'no server support' "${1}/testfile" "$DOWN" '=' @@ -148,4 +153,5 @@ changetohttpswebserver serverconfigs "https://localhost:${APTHTTPSPORT}" webserverconfig 'aptwebserver::redirect::replace::/redirectme/' "https://localhost:${APTHTTPSPORT}/" +serverconfigs "https://localhost:${APTHTTPSPORT}/redirectme" serverconfigs "http://localhost:${APTHTTPPORT}/redirectme" diff --git a/test/integration/test-releasefile-verification b/test/integration/test-releasefile-verification index 82e48ffa8..fec7b1302 100755 --- a/test/integration/test-releasefile-verification +++ b/test/integration/test-releasefile-verification @@ -12,8 +12,6 @@ buildaptarchive setupflataptarchive changetowebserver -webserverconfig 'aptwebserver::support::range' 'false' - prepare() { local DATE="${2:-now}" if [ "$DATE" = 'now' ]; then diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc index c32f286b2..950a17bc1 100644 --- a/test/interactive-helper/aptwebserver.cc +++ b/test/interactive-helper/aptwebserver.cc @@ -717,6 +717,15 @@ static void * handleClient(void * voidclient) /*{{{*/ condition.clear(); if (condition.empty() == false && strncmp(condition.c_str(), "bytes=", 6) == 0) { + std::string ranges = ',' + _config->Find("aptwebserver::response-header::Accept-Ranges") + ','; + ranges.erase(std::remove(ranges.begin(), ranges.end(), ' '), ranges.end()); + if (ranges.find(",bytes,") == std::string::npos) + { + // we handle it as an error here because we are a test server - a real one should just ignore it + sendError(client, 400, *m, sendContent, "Client does range requests we don't support", headers); + continue; + } + time_t cache; std::string ifrange; if (_config->FindB("aptwebserver::support::if-range", true) == true) -- cgit v1.2.3