[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#427909: Bug#212732: support redirects and interactive authentication (Progeny)



Attached is a patch against apt 0.7.19 (current in lenny/sid)
including just the Redirect support from Jeff Licquia's patch in
Bug#212732. 

As far as the issues described in Bug#66434 with bad redirection and
mod_speling, that seems mostly unlikely to be a problem these days thanks
to the md5 validation and signature support. The only way you could get
unexpected data is if your original Release and Release.gpg files were
redirected to the wrong place, but were completely consistent and had
corresponding Packages files and debs.

In the event that is a concern, the patch lets the user set the 
Acquire::http::AllowRedirect config option to false to block that behaviour.
It'd be possible to have an option to verify the filename part of the URL
is unchanged as well without much difficulty.

I bumped the library version, mostly so I could be sure I was testing the
right thing, but I presume this requires a libapt-pkg ABI bump anyway
(there's an Acquire::Redirect() callback added), so I left it in the
patch.

Cheers,
aj

diff -urb apt-0.7.19/apt-pkg/acquire-method.cc apt-0.7.19aj1/apt-pkg/acquire-method.cc
--- apt-0.7.19/apt-pkg/acquire-method.cc	2008-06-10 07:10:08.000000000 +1000
+++ apt-0.7.19aj1/apt-pkg/acquire-method.cc	2008-12-21 21:50:41.000000000 +1000
@@ -447,6 +447,38 @@
 }
 									/*}}}*/
 
+// AcqMethod::Redirect - Send a redirect message                       /*{{{*/
+// ---------------------------------------------------------------------
+/* This method sends the redirect message and also manipulates the queue
+   to keep the pipeline synchronized. */
+void pkgAcqMethod::Redirect(const string &NewURI)
+{
+   string CurrentURI = "<UNKNOWN>";
+   if (Queue != 0)
+      CurrentURI = Queue->Uri;
+ 
+   char S[1024];
+   snprintf(S, sizeof(S)-50, "103 Redirect\nURI: %s\nNew-URI: %s\n\n",
+         CurrentURI.c_str(), NewURI.c_str());
+
+   if (write(STDOUT_FILENO,S,strlen(S)) != (ssize_t)strlen(S))
+      exit(100);
+
+   // Change the URI for the request.
+   Queue->Uri = NewURI;
+
+   /* To keep the pipeline synchronized, move the current request to
+      the end of the queue, past the end of the current pipeline. */
+   FetchItem *I;
+   for (I = Queue; I->Next != 0; I = I->Next) ;
+   I->Next = Queue;
+   Queue = Queue->Next;
+   I->Next->Next = 0;
+   if (QueueBack == 0)
+      QueueBack = I->Next;
+}
+                                                                        /*}}}*/
+
 // AcqMethod::FetchResult::FetchResult - Constructor			/*{{{*/
 // ---------------------------------------------------------------------
 /* */
diff -urb apt-0.7.19/apt-pkg/acquire-method.h apt-0.7.19aj1/apt-pkg/acquire-method.h
--- apt-0.7.19/apt-pkg/acquire-method.h	2008-06-10 07:10:08.000000000 +1000
+++ apt-0.7.19aj1/apt-pkg/acquire-method.h	2008-12-21 21:50:02.000000000 +1000
@@ -84,6 +84,8 @@
    void Log(const char *Format,...);
    void Status(const char *Format,...);
    
+   void Redirect(const string &NewURI);
+ 
    int Run(bool Single = false);
    inline void SetFailExtraMsg(string Msg) {FailExtra = Msg;};
    
diff -urb apt-0.7.19/apt-pkg/acquire-worker.cc apt-0.7.19aj1/apt-pkg/acquire-worker.cc
--- apt-0.7.19/apt-pkg/acquire-worker.cc	2008-11-24 19:35:21.000000000 +1000
+++ apt-0.7.19aj1/apt-pkg/acquire-worker.cc	2008-12-21 21:42:06.000000000 +1000
@@ -220,6 +220,20 @@
 	 Status = LookupTag(Message,"Message");
 	 break;
 	    
+         // 103 Redirect
+         case 103:
+         {
+            if (Itm == 0)
+            {
+               _error->Error("Method gave invalid 103 Redirect message");
+               break;
+            }
+ 
+            string NewURI = LookupTag(Message,"New-URI",URI.c_str());
+            Itm->URI = NewURI;
+            break;
+         }
+   
 	 // 200 URI Start
 	 case 200:
 	 {
diff -urb apt-0.7.19/apt-pkg/makefile apt-0.7.19aj1/apt-pkg/makefile
--- apt-0.7.19/apt-pkg/makefile	2008-06-10 07:10:08.000000000 +1000
+++ apt-0.7.19aj1/apt-pkg/makefile	2008-12-21 21:54:58.000000000 +1000
@@ -13,7 +13,7 @@
 # methods/makefile - FIXME
 LIBRARY=apt-pkg
 LIBEXT=$(GLIBC_VER)$(LIBSTDCPP_VER)
-MAJOR=4.6
+MAJOR=4.7
 MINOR=0
 SLIBS=$(PTHREADLIB) $(INTLLIBS) -lutil
 APT_DOMAIN:=libapt-pkg$(MAJOR)
diff -urb apt-0.7.19/methods/http.cc apt-0.7.19aj1/methods/http.cc
--- apt-0.7.19/methods/http.cc	2008-11-24 19:32:23.000000000 +1000
+++ apt-0.7.19aj1/methods/http.cc	2008-12-21 22:25:28.000000000 +1000
@@ -39,6 +39,7 @@
 #include <errno.h>
 #include <string.h>
 #include <iostream>
+#include <map>
 #include <apti18n.h>
 
 // Internet stuff
@@ -57,6 +58,7 @@
 time_t HttpMethod::FailTime = 0;
 unsigned long PipelineDepth = 10;
 unsigned long TimeOut = 120;
+bool AllowRedirect = false;
 bool Debug = false;
 URI Proxy;
 
@@ -628,6 +630,12 @@
       return true;
    }
 
+   if (stringcasecmp(Tag,"Location:") == 0)
+   {
+      Location = Val;
+      return true;
+   }
+
    return true;
 }
 									/*}}}*/
@@ -900,7 +908,9 @@
      1 - IMS hit
      3 - Unrecoverable error 
      4 - Error with error content page
-     5 - Unrecoverable non-server error (close the connection) */
+     5 - Unrecoverable non-server error (close the connection) 
+     6 - Try again with a new or changed URI
+ */
 int HttpMethod::DealWithHeaders(FetchResult &Res,ServerState *Srv)
 {
    // Not Modified
@@ -912,6 +922,27 @@
       return 1;
    }
    
+   /* Redirect
+    *
+    * Note that it is only OK for us to treat all redirection the same
+    * because we *always* use GET, not other HTTP methods.  There are
+    * three redirection codes for which it is not appropriate that we
+    * redirect.  Pass on those codes so the error handling kicks in.
+    */
+   if (AllowRedirect
+       && (Srv->Result > 300 && Srv->Result < 400)
+       && (Srv->Result != 300       // Multiple Choices
+           && Srv->Result != 304    // Not Modified
+           && Srv->Result != 306))  // (Not part of HTTP/1.1, reserved)
+   {
+      if (!Srv->Location.empty())
+      {
+         NextURI = Srv->Location;
+         return 6;
+      }
+      /* else pass through for error message */
+   }
+ 
    /* We have a reply we dont handle. This should indicate a perm server
       failure */
    if (Srv->Result < 200 || Srv->Result >= 300)
@@ -1026,6 +1057,7 @@
    if (pkgAcqMethod::Configuration(Message) == false)
       return false;
    
+   AllowRedirect = _config->FindB("Acquire::http::AllowRedirect",true);
    TimeOut = _config->FindI("Acquire::http::Timeout",TimeOut);
    PipelineDepth = _config->FindI("Acquire::http::Pipeline-Depth",
 				  PipelineDepth);
@@ -1039,6 +1071,10 @@
 /* */
 int HttpMethod::Loop()
 {
+   typedef vector<string> StringVector;
+   typedef vector<string>::iterator StringVectorIterator;
+   map<string, StringVector> Redirected;
+
    signal(SIGTERM,SigTerm);
    signal(SIGINT,SigTerm);
    
@@ -1225,6 +1261,46 @@
 	    break;
 	 }
 	 
+         // Try again with a new URL
+         case 6:
+         {
+            // Clear rest of response if there is content
+            if (Server->HaveContent)
+            {
+               File = new FileFd("/dev/null",FileFd::WriteExists);
+               Server->RunData();
+               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.size() == 0)
+               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;
+         }
+
 	 default:
 	 Fail(_("Internal error"));
 	 break;
diff -urb apt-0.7.19/methods/http.h apt-0.7.19aj1/methods/http.h
--- apt-0.7.19/methods/http.h	2008-06-10 07:10:09.000000000 +1000
+++ apt-0.7.19aj1/methods/http.h	2008-12-21 21:35:53.000000000 +1000
@@ -99,6 +99,7 @@
    enum {Chunked,Stream,Closes} Encoding;
    enum {Header, Data} State;
    bool Persistent;
+   string Location;
    
    // This is a Persistent attribute of the server itself.
    bool Pipeline;
@@ -143,6 +144,8 @@
    static time_t FailTime;
    static void SigTerm(int);
    
+   string NextURI;
+   
    public:
    friend class ServerState;
 
diff -urb apt-0.7.19/methods/makefile apt-0.7.19aj1/methods/makefile
--- apt-0.7.19/methods/makefile	2008-11-24 19:32:23.000000000 +1000
+++ apt-0.7.19aj1/methods/makefile	2008-12-21 21:57:50.000000000 +1000
@@ -7,7 +7,7 @@
 BIN := $(BIN)/methods
 
 # FIXME..
-LIB_APT_PKG_MAJOR = 4.6
+LIB_APT_PKG_MAJOR = 4.7
 APT_DOMAIN := libapt-pkg$(LIB_APT_PKG_MAJOR)
 
 # The file method

Attachment: signature.asc
Description: Digital signature


Reply to: