detect redirection loops in acquire instead of workers

Having the detection handled in specific (http) workers means that a
redirection loop over different hostnames isn't detected. Its also not a
good idea have this implement in each method independently even if it
would work

6 files changed, 42 insertions, 45 deletions

diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index d3c948636..0963ea21e 100644
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -17,7 +17,8 @@
 
 class aptMethod : public pkgAcqMethod
 {
-   char const * const Binary;
+protected:
+   std::string Binary;
 
 public:
    virtual bool Configuration(std::string Message) APT_OVERRIDE
diff --git a/methods/http.cc b/methods/http.cc
index c61ca1c3f..cf5eae06d 100644
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -465,6 +465,12 @@ bool HttpServerState::RunData(FileFd * const File)
    return Owner->Flush() && !_error->PendingError();
 }
 									/*}}}*/
+bool HttpServerState::RunDataToDevNull()				/*{{{*/
+{
+   FileFd DevNull("/dev/null", FileFd::WriteOnly);
+   return RunData(&DevNull);
+}
+									/*}}}*/
 bool HttpServerState::ReadHeaderLines(std::string &Data)		/*{{{*/
 {
    return In.WriteTillEl(Data);
diff --git a/methods/http.h b/methods/http.h
index ac5f52314..2ac3b8c9b 100644
--- a/methods/http.h
+++ b/methods/http.h
@@ -106,6 +106,7 @@ struct HttpServerState: public ServerState
    virtual void Reset() APT_OVERRIDE { ServerState::Reset(); ServerFd = -1; };
 
    virtual bool RunData(FileFd * const File) APT_OVERRIDE;
+   virtual bool RunDataToDevNull() APT_OVERRIDE;
 
    virtual bool Open() APT_OVERRIDE;
    virtual bool IsOpen() APT_OVERRIDE;
diff --git a/methods/https.h b/methods/https.h
index 8592570c6..85cc7824e 100644
--- a/methods/https.h
+++ b/methods/https.h
@@ -39,6 +39,7 @@ class HttpsServerState : public ServerState
 
    /** \brief Transfer the data from the socket */
    virtual bool RunData(FileFd * const /*File*/) APT_OVERRIDE { return false; }
+   virtual bool RunDataToDevNull() APT_OVERRIDE { return false; }
 
    virtual bool Open() APT_OVERRIDE { return false; }
    virtual bool IsOpen() APT_OVERRIDE { return false; }
diff --git a/methods/server.cc b/methods/server.cc
index 6d147fe12..672a3d16d 100644
--- a/methods/server.cc
+++ b/methods/server.cc
@@ -297,12 +297,33 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
 	 else
 	    NextURI.clear();
 	 NextURI.append(DeQuoteString(Server->Location));
+	 if (Queue->Uri == NextURI)
+	 {
+	    SetFailReason("RedirectionLoop");
+	    _error->Error("Redirection loop encountered");
+	    if (Server->HaveContent == true)
+	       return ERROR_WITH_CONTENT_PAGE;
+	    return ERROR_UNRECOVERABLE;
+	 }
 	 return TRY_AGAIN_OR_REDIRECT;
       }
       else
       {
 	 NextURI = DeQuoteString(Server->Location);
 	 URI tmpURI = NextURI;
+	 if (tmpURI.Access == "http" && Binary == "https+http")
+	 {
+	    tmpURI.Access = "https+http";
+	    NextURI = tmpURI;
+	 }
+	 if (Queue->Uri == NextURI)
+	 {
+	    SetFailReason("RedirectionLoop");
+	    _error->Error("Redirection loop encountered");
+	    if (Server->HaveContent == true)
+	       return ERROR_WITH_CONTENT_PAGE;
+	    return ERROR_UNRECOVERABLE;
+	 }
 	 URI Uri = Queue->Uri;
 	 // same protocol redirects are okay
 	 if (tmpURI.Access == Uri.Access)
@@ -486,10 +507,6 @@ bool ServerMethod::Fetch(FetchItem *)
 // ServerMethod::Loop - Main loop					/*{{{*/
 int ServerMethod::Loop()
 {
-   typedef vector<string> StringVector;
-   typedef vector<string>::iterator StringVectorIterator;
-   map<string, StringVector> Redirected;
-
    signal(SIGTERM,SigTerm);
    signal(SIGINT,SigTerm);
    
@@ -720,46 +737,16 @@ int ServerMethod::Loop()
 	    File = 0;
 	    break;
 	 }
-	 
-         // Try again with a new URL
-         case TRY_AGAIN_OR_REDIRECT:
-         {
-            // Clear rest of response if there is content
-            if (Server->HaveContent)
-            {
-               File = new FileFd("/dev/null",FileFd::WriteExists);
-               Server->RunData(File);
-               delete File;
-               File = 0;
-            }
-
-            /* Detect redirect loops.  No more redirects are allowed
-               after the same URI is seen twice in a queue item. */
-            StringVector &R = Redirected[Queue->DestFile];
-            bool StopRedirects = false;
-            if (R.empty() == true)
-               R.push_back(Queue->Uri);
-            else if (R[0] == "STOP" || R.size() > 10)
-               StopRedirects = true;
-            else
-            {
-               for (StringVectorIterator I = R.begin(); I != R.end(); ++I)
-                  if (Queue->Uri == *I)
-                  {
-                     R[0] = "STOP";
-                     break;
-                  }
- 
-               R.push_back(Queue->Uri);
-            }
- 
-            if (StopRedirects == false)
-               Redirect(NextURI);
-            else
-               Fail();
- 
-            break;
-         }
+
+	 // Try again with a new URL
+	 case TRY_AGAIN_OR_REDIRECT:
+	 {
+	    // Clear rest of response if there is content
+	    if (Server->HaveContent)
+	       Server->RunDataToDevNull();
+	    Redirect(NextURI);
+	    break;
+	 }
 
 	 default:
 	 Fail(_("Internal error"));
diff --git a/methods/server.h b/methods/server.h
index af0914b9c..b23b0e50a 100644
--- a/methods/server.h
+++ b/methods/server.h
@@ -91,6 +91,7 @@ struct ServerState
 
    /** \brief Transfer the data from the socket */
    virtual bool RunData(FileFd * const File) = 0;
+   virtual bool RunDataToDevNull() = 0;
 
    virtual bool Open() = 0;
    virtual bool IsOpen() = 0;