dispose http(s) 416 error page as non-content

Real webservers (like apache) actually send an error page with a 416
response, but our client didn't expect it leaving the page on the socket
to be parsed as response for the next request (http) or as file content
(https), which isn't what we want at all… Symptom is a "Bad header line"
as html usually doesn't parse that well to an http-header.

This manifests itself e.g. if we have a complete file (or larger) in
partial/ which isn't discarded by If-Range as the server doesn't support
it (or it is just newer, think: mirror rotation).
It is a sort-of regression of 78c72d0ce22e00b194251445aae306df357d5c1a,
which removed the filesize - 1 trick, but this had its own problems…

To properly test this our webserver gains the ability to reply with
transfer-encoding: chunked as most real webservers will use it to send
the dynamically generated error pages.

(The tests and their binary helpers had to be slightly modified to
apply, but the patch to fix the issue itself is unchanged.)

Closes: 768797

4 files changed, 30 insertions, 15 deletions

diff --git a/methods/http.cc b/methods/http.cc
index f2a4a4db6..1b996db98 100644
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -440,6 +440,8 @@ bool HttpServerState::RunData(FileFd * const File)
          loss of the connection means we are done */
       if (Encoding == Closes)
 	 In.Limit(-1);
+      else if (JunkSize != 0)
+	 In.Limit(JunkSize);
       else
 	 In.Limit(Size - StartPos);
       
diff --git a/methods/https.cc b/methods/https.cc
index 0499af0c5..65a744e2a 100644
--- a/methods/https.cc
+++ b/methods/https.cc
@@ -59,6 +59,9 @@ HttpsMethod::parse_header(void *buffer, size_t size, size_t nmemb, void *userp)
       {
          me->Server->Result = 200;
 	 me->Server->StartPos = me->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->Server->JunkSize = std::numeric_limits<unsigned long long>::max();
       }
       else
 	 me->Server->StartPos = 0;
@@ -76,13 +79,18 @@ size_t
 HttpsMethod::write_data(void *buffer, size_t size, size_t nmemb, void *userp)
 {
    HttpsMethod *me = (HttpsMethod *)userp;
+   size_t buffer_size = size * nmemb;
+   // we don't need to count the junk here, just drop anything we get as
+   // we don't always know how long it would be, e.g. in chunked encoding.
+   if (me->Server->JunkSize != 0)
+      return buffer_size;
 
    if (me->Res.Size == 0)
       me->URIStart(me->Res);
-   if(me->File->Write(buffer, size*nmemb) != true)
+   if(me->File->Write(buffer, buffer_size) != true)
       return false;
 
-   return size*nmemb;
+   return buffer_size;
 }
 
 int
diff --git a/methods/server.cc b/methods/server.cc
index 92d94e638..cb0341d5f 100644
--- a/methods/server.cc
+++ b/methods/server.cc
@@ -55,6 +55,7 @@ ServerState::RunHeadersResult ServerState::RunHeaders(FileFd * const File,
    Minor = 0; 
    Result = 0; 
    Size = 0; 
+   JunkSize = 0;
    StartPos = 0;
    Encoding = Closes;
    HaveContent = false;
@@ -163,14 +164,14 @@ bool ServerState::HeaderLine(string Line)
 	 Encoding = Stream;
       HaveContent = true;
 
-      // The length is already set from the Content-Range header
-      if (StartPos != 0)
-	 return true;
+      unsigned long long * SizePtr = &Size;
+      if (Result == 416)
+	 SizePtr = &JunkSize;
 
-      Size = strtoull(Val.c_str(), NULL, 10);
-      if (Size >= std::numeric_limits<unsigned long long>::max())
+      *SizePtr = strtoull(Val.c_str(), NULL, 10);
+      if (*SizePtr >= std::numeric_limits<unsigned long long>::max())
 	 return _error->Errno("HeaderLine", _("The HTTP server sent an invalid Content-Length header"));
-      else if (Size == 0)
+      else if (*SizePtr == 0)
 	 HaveContent = false;
       return true;
    }
@@ -187,10 +188,7 @@ bool ServerState::HeaderLine(string Line)
 
       // §14.16 says 'byte-range-resp-spec' should be a '*' in case of 416
       if (Result == 416 && sscanf(Val.c_str(), "bytes */%llu",&Size) == 1)
-      {
-	 StartPos = 1; // ignore Content-Length, it would override Size
-	 HaveContent = false;
-      }
+	 ; // we got the expected filesize which is all we wanted
       else if (sscanf(Val.c_str(),"bytes %llu-%*u/%llu",&StartPos,&Size) != 2)
 	 return _error->Error(_("The HTTP server sent an invalid Content-Range header"));
       if ((unsigned long long)StartPos > Size)
@@ -308,9 +306,15 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
 	 if ((unsigned long long)SBuf.st_size == Server->Size)
 	 {
 	    // the file is completely downloaded, but was not moved
+	    if (Server->HaveContent == true)
+	    {
+	       // Send to error page to dev/null
+	       FileFd DevNull("/dev/null",FileFd::WriteExists);
+	       Server->RunData(&DevNull);
+	    }
+	    Server->HaveContent = false;
 	    Server->StartPos = Server->Size;
 	    Server->Result = 200;
-	    Server->HaveContent = false;
 	 }
 	 else if (unlink(Queue->DestFile.c_str()) == 0)
 	 {
diff --git a/methods/server.h b/methods/server.h
index f5e68d902..1b81e3549 100644
--- a/methods/server.h
+++ b/methods/server.h
@@ -34,7 +34,8 @@ struct ServerState
    char Code[360];
 
    // These are some statistics from the last parsed header lines
-   unsigned long long Size;
+   unsigned long long Size; // size of the usable content (aka: the file)
+   unsigned long long JunkSize; // size of junk content (aka: server error pages)
    unsigned long long StartPos;
    time_t Date;
    bool HaveContent;
@@ -71,7 +72,7 @@ struct ServerState
    RunHeadersResult RunHeaders(FileFd * const File, const std::string &Uri);
 
    bool Comp(URI Other) const {return Other.Host == ServerName.Host && Other.Port == ServerName.Port;};
-   virtual void Reset() {Major = 0; Minor = 0; Result = 0; Code[0] = '\0'; Size = 0;
+   virtual void Reset() {Major = 0; Minor = 0; Result = 0; Code[0] = '\0'; Size = 0; JunkSize = 0;
 		 StartPos = 0; Encoding = Closes; time(&Date); HaveContent = false;
 		 State = Header; Persistent = false; Pipeline = true;};
    virtual bool WriteResponse(std::string const &Data) = 0;