Merge branch 'pu/ifrange' into 'main'

Add AllowRange option to disable HTTP Range usage

See merge request apt-team/apt!188

4 files changed, 125 insertions, 6 deletions

diff --git a/doc/examples/configure-index b/doc/examples/configure-index
index f3f7f5ebc..f05981045 100644
--- a/doc/examples/configure-index
+++ b/doc/examples/configure-index
@@ -282,7 +282,8 @@ Acquire
     Timeout "30";
     ConnectionAttemptDelayMsec "250";
     Pipeline-Depth "5";
-    AllowRedirect  "true";
+    AllowRanges "<BOOL>";
+    AllowRedirect "<BOOL>";
 
     // Cache Control. Note these do not work with Squid 2.0.2
     No-Cache "false";
@@ -305,11 +306,11 @@ Acquire
 	SslCert "/etc/apt/some.pem";
 	CaPath  "/etc/ssl/certs";
 	Verify-Host "true";
-	AllowRedirect  "true";
+	AllowRanges "<BOOL>";
+	AllowRedirect "<BOOL>";
 
 	Timeout "30";
 	ConnectionAttemptDelayMsec "250";
-	AllowRedirect  "true";
 
 	// Cache Control. Note these do not work with Squid 2.0.2
 	No-Cache "false";
diff --git a/methods/basehttp.cc b/methods/basehttp.cc
index f2c4156e1..df34698cd 100644
--- a/methods/basehttp.cc
+++ b/methods/basehttp.cc
@@ -11,6 +11,7 @@
 #include <config.h>
 
 #include <apt-pkg/configuration.h>
+#include <apt-pkg/debversion.h>
 #include <apt-pkg/error.h>
 #include <apt-pkg/fileutl.h>
 #include <apt-pkg/strutl.h>
@@ -19,6 +20,7 @@
 #include <limits>
 #include <map>
 #include <string>
+#include <string_view>
 #include <vector>
 #include <ctype.h>
 #include <signal.h>
@@ -246,7 +248,7 @@ bool RequestState::HeaderLine(string const &Line)			/*{{{*/
       return true;
    }
 
-   if (stringcasecmp(Tag, "Accept-Ranges:") == 0)
+   if (Server->RangesAllowed && stringcasecmp(Tag, "Accept-Ranges:") == 0)
    {
       std::string ranges = ',' + Val + ',';
       ranges.erase(std::remove(ranges.begin(), ranges.end(), ' '), ranges.end());
@@ -255,6 +257,24 @@ bool RequestState::HeaderLine(string const &Line)			/*{{{*/
       return true;
    }
 
+   if (Server->RangesAllowed && stringcasecmp(Tag, "Via:") == 0)
+   {
+      auto const parts = VectorizeString(Val, ' ');
+      std::string_view const varnish{"(Varnish/"};
+      if (parts.size() != 3 || parts[1] != "varnish" || parts[2].empty() ||
+	    not APT::String::Startswith(parts[2], std::string{varnish}) ||
+	    parts[2].back() != ')')
+	 return true;
+      auto const version = parts[2].substr(varnish.length(), parts[2].length() - (varnish.length() + 1));
+      if (version.empty())
+	 return true;
+      std::string_view const varnishsupport{"6.4~"};
+      if (debVersioningSystem::CmpFragment(version.data(), version.data() + version.length(),
+					   varnishsupport.begin(), varnishsupport.end()) < 0)
+	 Server->RangesAllowed = false;
+      return true;
+   }
+
    return true;
 }
 									/*}}}*/
@@ -276,7 +296,6 @@ void ServerState::Reset()						/*{{{*/
    Persistent = false;
    Pipeline = false;
    PipelineAllowed = true;
-   RangesAllowed = true;
    PipelineAnswersReceived = 0;
 }
 									/*}}}*/
@@ -411,6 +430,13 @@ BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
       }
       /* else pass through for error message */
    }
+   // the server is not supporting ranges as much as we would like. Retry without ranges
+   else if (not Server->RangesAllowed && (Req.Result == 416 || Req.Result == 206))
+   {
+      RemoveFile("server", Queue->DestFile);
+      NextURI = Queue->Uri;
+      return TRY_AGAIN_OR_REDIRECT;
+   }
    // retry after an invalid range response without partial data
    else if (Req.Result == 416)
    {
@@ -607,6 +633,7 @@ int BaseHttpMethod::Loop()
 	 setPostfixForMethodNames(::URI(Queue->Uri).Host.c_str());
 	 AllowRedirect = ConfigFindB("AllowRedirect", true);
 	 PipelineDepth = ConfigFindI("Pipeline-Depth", 10);
+	 Server->RangesAllowed = ConfigFindB("AllowRanges", true);
 	 Debug = DebugEnabled();
       }
 
diff --git a/test/integration/test-http-if-range b/test/integration/test-http-if-range
new file mode 100755
index 000000000..462d731cf
--- /dev/null
+++ b/test/integration/test-http-if-range
@@ -0,0 +1,91 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64'
+
+changetowebserver
+
+TESTFILE='aptarchive/testfile'
+HTTPFILE="http://localhost:${APTHTTPPORT}/testfile"
+DOWNFILE='./downloaded/testfile'
+DOWNLOADLOG='rootdir/tmp/testdownloadfile.log'
+
+testdownloadfile() {
+	rm -f "$DOWNLOADLOG"
+	msgtest "Testing download of file with" "$1"
+	if ! downloadfile "$HTTPFILE" "$DOWNFILE" > "$DOWNLOADLOG"; then
+		cat >&2 "$DOWNLOADLOG"
+		msgfail
+	else
+		msgpass
+	fi
+}
+
+nopartialfile() {
+	rm -f "$DOWNFILE"
+}
+validpartialfile() {
+	head -n 5 "$TESTFILE" > "$DOWNFILE"
+	touch -d "$(stat --format '%y' "${TESTFILE}")" "$DOWNFILE"
+}
+badolderpartialfile() {
+	head -n 5 "$TESTFILE" > "$DOWNFILE"
+	touch -d "$(stat --format '%y' "${TESTFILE}") - 1sec" "$DOWNFILE"
+}
+badnewerpartialfile() {
+	head -n 5 "$TESTFILE" > "$DOWNFILE"
+	touch -d 'now + 1hour' "$DOWNFILE"
+}
+fullfile() {
+	cp -a "$TESTFILE" "$DOWNFILE"
+}
+
+cp -a "${TESTDIR}/framework" "$TESTFILE"
+
+testrun() {
+	nopartialfile
+	testdownloadfile "no file $1"
+	testwebserverlaststatuscode "$2" "$DOWNLOADLOG"
+	testsuccess cmp "$TESTFILE" "$DOWNFILE"
+
+	validpartialfile
+	testdownloadfile "good partial file $1"
+	testwebserverlaststatuscode "$3" "$DOWNLOADLOG"
+	testsuccess cmp "$TESTFILE" "$DOWNFILE"
+
+	badolderpartialfile
+	testdownloadfile "bad old partial file $1"
+	testwebserverlaststatuscode "$4" "$DOWNLOADLOG"
+	testsuccess cmp "$TESTFILE" "$DOWNFILE"
+
+	badnewerpartialfile
+	testdownloadfile "bad new partial file $1"
+	testwebserverlaststatuscode "$4" "$DOWNLOADLOG"
+	testsuccess cmp "$TESTFILE" "$DOWNFILE"
+
+	fullfile
+	testdownloadfile "complete file $1"
+	testwebserverlaststatuscode "$5" "$DOWNLOADLOG"
+	testsuccess cmp "$TESTFILE" "$DOWNFILE"
+}
+
+testrun 'defaults' '200' '206' '200' '416'
+
+webserverconfig 'aptwebserver::support::range' 'false'
+testrun 'no ranges' '200' '200' '200' '200'
+webserverconfig 'aptwebserver::support::range' 'true'
+
+webserverconfig 'aptwebserver::support::if-range' 'false'
+# the second 206 is bad, but we are unable to detect this
+testrun 'buggy server' '200' '206' '206' '416'
+echo 'Acquire::http::localhost::AllowRanges "false";' > rootdir/etc/apt/apt.conf.d/noallowranges
+testrun 'range disabled by conf' '200' '200' '200' '200'
+rm rootdir/etc/apt/apt.conf.d/noallowranges
+# detect varnish < 6.4 automatically
+webserverconfig 'aptwebserver::response-header::Via' '1.1 varnish (Varnish/6.1)'
+testrun 'bad varnish' '200' '200' '200' '200'
+webserverconfig 'aptwebserver::response-header::Via' '1.1 varnish (Varnish/6.4)'
+testrun 'good varnish' '200' '206' '206' '416'
diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc
index 58ba54f84..d4bac24d1 100644
--- a/test/interactive-helper/aptwebserver.cc
+++ b/test/interactive-helper/aptwebserver.cc
@@ -828,7 +828,7 @@ static void * handleClient(int const client, size_t const id)		/*{{{*/
 		  ifrange = LookupTag(*m, "If-Range", "");
 	       bool validrange = (ifrange.empty() == true ||
 		     (RFC1123StrToTime(ifrange, cache) == true &&
-		      cache <= data.ModificationTime()));
+		      cache == data.ModificationTime()));
 
 	       // FIXME: support multiple byte-ranges (APT clients do not do this)
 	       if (condition.find(',') == std::string::npos)