use the same redirection handling for http and https

cURL which backs our https implementation can handle redirects on its
own, but by dealing with them on our own we gain finer control over which
redirections will be performed (we don't like https → http) and by whom
so that redirections to other hosts correctly spawn a new https method
dealing with these instead of letting the current one deal with it.

methods/http.cc

@@ -797,3 +797,32 @@ void HttpMethod::RotateDNS()						/*{{{*/
    ::RotateDNS();
 }
 									/*}}}*/
+ServerMethod::DealWithHeadersResult HttpMethod::DealWithHeaders(FetchResult &Res)/*{{{*/
+{
+   auto ret = ServerMethod::DealWithHeaders(Res);
+   if (ret != ServerMethod::FILE_IS_OPEN)
+      return ret;
+
+   // Open the file
+   delete File;
+   File = new FileFd(Queue->DestFile,FileFd::WriteAny);
+   if (_error->PendingError() == true)
+      return ERROR_NOT_FROM_SERVER;
+
+   FailFile = Queue->DestFile;
+   FailFile.c_str();   // Make sure we don't do a malloc in the signal handler
+   FailFd = File->Fd();
+   FailTime = Server->Date;
+
+   if (Server->InitHashes(Queue->ExpectedHashes) == false || Server->AddPartialFileToHashes(*File) == false)
+   {
+      _error->Errno("read",_("Problem hashing file"));
+      return ERROR_NOT_FROM_SERVER;
+   }
+   if (Server->StartPos > 0)
+      Res.ResumePoint = Server->StartPos;
+
+   SetNonBlock(File->Fd(),true);
+   return FILE_IS_OPEN;
+}
+									/*}}}*/

methods/http.h

@@ -130,6 +130,7 @@ class HttpMethod : public ServerMethod
 
    virtual std::unique_ptr<ServerState> CreateServerState(URI const &uri) APT_OVERRIDE;
    virtual void RotateDNS() APT_OVERRIDE;
+   virtual DealWithHeadersResult DealWithHeaders(FetchResult &Res) APT_OVERRIDE;
 
    protected:
    std::string AutoDetectProxyCmd;

methods/https.cc

@@ -60,6 +60,7 @@ HttpsMethod::parse_header(void *buffer, size_t size, size_t nmemb, void *userp)
 
    if (line.empty() == true)
    {
+      me->https->Server->JunkSize = 0;
       if (me->https->Server->Result != 416 && me->https->Server->StartPos != 0)
 	 ;
       else if (me->https->Server->Result == 416)
@@ -99,11 +100,13 @@ HttpsMethod::parse_header(void *buffer, size_t size, size_t nmemb, void *userp)
       // we expect valid data, so tell our caller we get the file now
       if (me->https->Server->Result >= 200 && me->https->Server->Result < 300)
       {
-	 if (me->https->Server->JunkSize == 0 && me->Res->Size != 0 && me->Res->Size > me->Res->ResumePoint)
+	 if (me->Res->Size != 0 && me->Res->Size > me->Res->ResumePoint)
 	    me->https->URIStart(*me->Res);
 	 if (me->https->Server->AddPartialFileToHashes(*(me->https->File)) == false)
 	    return 0;
       }
+      else
+	 me->https->Server->JunkSize = std::numeric_limits<decltype(me->https->Server->JunkSize)>::max();
    }
    else if (me->https->Server->HeaderLine(line) == false)
       return 0;
@@ -266,6 +269,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
    // options
    curl_easy_setopt(curl, CURLOPT_NOPROGRESS, true);
    curl_easy_setopt(curl, CURLOPT_FILETIME, true);
+   curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 0);
    // only allow curl to handle https, not the other stuff it supports
    curl_easy_setopt(curl, CURLOPT_PROTOCOLS, CURLPROTO_HTTPS);
    curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTPS);
@@ -372,10 +376,6 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
    curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, DL_MIN_SPEED);
    curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, timeout);
 
-   // set redirect options and default to 10 redirects
-   curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, AllowRedirect);
-   curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 10);
-
    // debug
    if (Debug == true)
       curl_easy_setopt(curl, CURLOPT_VERBOSE, true);
@@ -428,6 +428,8 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
    // met, CURLINFO_CONDITION_UNMET will be set to 1
    long curl_condition_unmet = 0;
    curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &curl_condition_unmet);
+   if (curl_condition_unmet == 1)
+      Server->Result = 304;
 
    File->Close();
    curl_slist_free_all(headers);
@@ -460,70 +462,54 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
       return false;
    }
 
-   // server says file not modified
-   if (Server->Result == 304 || curl_condition_unmet == 1)
+   switch (DealWithHeaders(Res))
    {
-      RemoveFile("https", File->Name());
-      Res.IMSHit = true;
-      Res.LastModified = Itm->LastModified;
-      Res.Size = 0;
-      URIDone(Res);
-      return true;
-   }
-   Res.IMSHit = false;
+      case ServerMethod::IMS_HIT:
+	 URIDone(Res);
+	 break;
 
-   if (Server->Result != 200 && // OK
-	 Server->Result != 206 && // Partial
-	 Server->Result != 416) // invalid Range
-   {
-      char err[255];
-      snprintf(err, sizeof(err) - 1, "HttpError%i", Server->Result);
-      SetFailReason(err);
-      _error->Error("%i %s", Server->Result, Server->Code);
-      // unlink, no need keep 401/404 page content in partial/
-      RemoveFile("https", File->Name());
-      return false;
-   }
+      case ServerMethod::ERROR_WITH_CONTENT_PAGE:
+	 // unlink, no need keep 401/404 page content in partial/
+	 RemoveFile(Binary.c_str(), File->Name());
+      case ServerMethod::ERROR_UNRECOVERABLE:
+      case ServerMethod::ERROR_NOT_FROM_SERVER:
+	 return false;
 
-   // invalid range-request
-   if (Server->Result == 416)
-   {
-      RemoveFile("https", File->Name());
-      delete File;
-      Redirect(Itm->Uri);
-      return true;
-   }
+      case ServerMethod::TRY_AGAIN_OR_REDIRECT:
+	 Redirect(NextURI);
+	 break;
 
-   struct stat resultStat;
-   if (unlikely(stat(File->Name().c_str(), &resultStat) != 0))
-   {
-      _error->Errno("stat", "Unable to access file %s", File->Name().c_str());
-      return false;
-   }
-   Res.Size = resultStat.st_size;
+      case ServerMethod::FILE_IS_OPEN:
+	 struct stat resultStat;
+	 if (unlikely(stat(File->Name().c_str(), &resultStat) != 0))
+	 {
+	    _error->Errno("stat", "Unable to access file %s", File->Name().c_str());
+	    return false;
+	 }
+	 Res.Size = resultStat.st_size;
 
-   // Timestamp
-   curl_easy_getinfo(curl, CURLINFO_FILETIME, &Res.LastModified);
-   if (Res.LastModified != -1)
-   {
-      struct timeval times[2];
-      times[0].tv_sec = Res.LastModified;
-      times[1].tv_sec = Res.LastModified;
-      times[0].tv_usec = times[1].tv_usec = 0;
-      utimes(File->Name().c_str(), times);
-   }
-   else
-      Res.LastModified = resultStat.st_mtime;
+	 // Timestamp
+	 curl_easy_getinfo(curl, CURLINFO_FILETIME, &Res.LastModified);
+	 if (Res.LastModified != -1)
+	 {
+	    struct timeval times[2];
+	    times[0].tv_sec = Res.LastModified;
+	    times[1].tv_sec = Res.LastModified;
+	    times[0].tv_usec = times[1].tv_usec = 0;
+	    utimes(File->Name().c_str(), times);
+	 }
+	 else
+	    Res.LastModified = resultStat.st_mtime;
 
-   // take hashes
-   Res.TakeHashes(*(Server->GetHashes()));
+	 // take hashes
+	 Res.TakeHashes(*(Server->GetHashes()));
 
-   // keep apt updated
-   URIDone(Res);
+	 // keep apt updated
+	 URIDone(Res);
+	 break;
+   }
 
-   // cleanup
    delete File;
-
    return true;
 }
 									/*}}}*/

methods/server.cc

@@ -170,7 +170,7 @@ bool ServerState::HeaderLine(string Line)
       HaveContent = true;
 
       unsigned long long * DownloadSizePtr = &DownloadSize;
-      if (Result == 416)
+      if (Result == 416 || (Result >= 300 && Result < 400))
 	 DownloadSizePtr = &JunkSize;
 
       *DownloadSizePtr = strtoull(Val.c_str(), NULL, 10);
@@ -272,6 +272,7 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
       RemoveFile("server", Queue->DestFile);
       Res.IMSHit = true;
       Res.LastModified = Queue->LastModified;
+      Res.Size = 0;
       return IMS_HIT;
    }
 
@@ -288,7 +289,8 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
            && Server->Result != 304    // Not Modified
            && Server->Result != 306))  // (Not part of HTTP/1.1, reserved)
    {
-      if (Server->Location.empty() == true);
+      if (Server->Location.empty() == true)
+	 ;
       else if (Server->Location[0] == '/' && Queue->Uri.empty() == false)
       {
 	 URI Uri = Queue->Uri;
@@ -329,8 +331,10 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
 	 if (tmpURI.Access == Uri.Access)
 	    return TRY_AGAIN_OR_REDIRECT;
 	 // as well as http to https
-	 else if (Uri.Access == "http" && tmpURI.Access == "https")
+	 else if ((Uri.Access == "http" || Uri.Access == "https+http") && tmpURI.Access == "https")
 	    return TRY_AGAIN_OR_REDIRECT;
+	 else
+	    _error->Error("Redirection from %s to '%s' is forbidden", Uri.Access.c_str(), NextURI.c_str());
       }
       /* else pass through for error message */
    }
@@ -358,11 +362,10 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
 	    // 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);
+	       // nuke the sent error page
+	       Server->RunDataToDevNull();
+	       Server->HaveContent = false;
 	    }
-	    Server->HaveContent = false;
 	    Server->StartPos = Server->TotalFileSize;
 	    Server->Result = 200;
 	 }
@@ -378,10 +381,13 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
       failure */
    if (Server->Result < 200 || Server->Result >= 300)
    {
-      std::string err;
-      strprintf(err, "HttpError%u", Server->Result);
-      SetFailReason(err);
-      _error->Error("%u %s", Server->Result, Server->Code);
+      if (_error->PendingError() == false)
+      {
+	 std::string err;
+	 strprintf(err, "HttpError%u", Server->Result);
+	 SetFailReason(err);
+	 _error->Error("%u %s", Server->Result, Server->Code);
+      }
       if (Server->HaveContent == true)
 	 return ERROR_WITH_CONTENT_PAGE;
       return ERROR_UNRECOVERABLE;
@@ -390,27 +396,6 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
    // This is some sort of 2xx 'data follows' reply
    Res.LastModified = Server->Date;
    Res.Size = Server->TotalFileSize;
-   
-   // Open the file
-   delete File;
-   File = new FileFd(Queue->DestFile,FileFd::WriteAny);
-   if (_error->PendingError() == true)
-      return ERROR_NOT_FROM_SERVER;
-
-   FailFile = Queue->DestFile;
-   FailFile.c_str();   // Make sure we don't do a malloc in the signal handler
-   FailFd = File->Fd();
-   FailTime = Server->Date;
-
-   if (Server->InitHashes(Queue->ExpectedHashes) == false || Server->AddPartialFileToHashes(*File) == false)
-   {
-      _error->Errno("read",_("Problem hashing file"));
-      return ERROR_NOT_FROM_SERVER;
-   }
-   if (Server->StartPos > 0)
-      Res.ResumePoint = Server->StartPos;
-
-   SetNonBlock(File->Fd(),true);
    return FILE_IS_OPEN;
 }
 									/*}}}*/
@@ -729,12 +714,7 @@ int ServerMethod::Loop()
 	 case ERROR_WITH_CONTENT_PAGE:
 	 {
 	    Fail();
-	    
-	    // Send to content to dev/null
-	    File = new FileFd("/dev/null",FileFd::WriteExists);
-	    Server->RunData(File);
-	    delete File;
-	    File = 0;
+	    Server->RunDataToDevNull();
 	    break;
 	 }
 

methods/server.h

@@ -141,7 +141,7 @@ class ServerMethod : public aptMethod
       TRY_AGAIN_OR_REDIRECT
    };
    /** \brief Handle the retrieved header data */
-   DealWithHeadersResult DealWithHeaders(FetchResult &Res);
+   virtual DealWithHeadersResult DealWithHeaders(FetchResult &Res);
 
    // In the event of a fatal signal this file will be closed and timestamped.
    static std::string FailFile;

test/integration/test-apt-https-no-redirect

@@ -11,7 +11,9 @@ insertpackage 'stable' 'apt' 'all' '1'
 setupaptarchive --no-update
 
 echo 'alright' > aptarchive/working
-changetohttpswebserver  -o "aptwebserver::redirect::replace::/redirectme/=http://localhost:${APTHTTPPORT}/"
+changetohttpswebserver
+webserverconfig 'aptwebserver::redirect::replace::/redirectme/' "http://localhost:${APTHTTPPORT}/"
+webserverconfig 'aptwebserver::redirect::replace::/redirectme2/' "https://localhost:${APTHTTPSPORT}/"
 
 msgtest 'download of a file works via' 'http'
 testsuccess --nomsg downloadfile "http://localhost:${APTHTTPPORT}/working" httpfile
@@ -25,4 +27,4 @@ msgtest 'download of a file does not work if' 'https redirected to http'
 testfailure --nomsg downloadfile "https://localhost:${APTHTTPSPORT}/redirectme/working" redirectfile
 
 msgtest 'libcurl has forbidden access in last request to' 'http resource'
-testsuccess --nomsg grep -q -E -- 'Protocol "?http"? not supported or disabled in libcurl' rootdir/tmp/testfailure.output
+testsuccess --nomsg grep -q -E -- "Redirection from https to 'http://.*' is forbidden" rootdir/tmp/testfailure.output

test/integration/test-partial-file-support

@@ -146,3 +146,6 @@ serverconfigs "http://localhost:${APTHTTPPORT}"
 changetohttpswebserver
 
 serverconfigs "https://localhost:${APTHTTPSPORT}"
+
+webserverconfig 'aptwebserver::redirect::replace::/redirectme/' "https://localhost:${APTHTTPSPORT}/"
+serverconfigs "http://localhost:${APTHTTPPORT}/redirectme"