derive more of https from http method

Bug #778375 uncovered that https wasn't properly integrated in the class
family tree of http as it was supposed to be leading to a NULL pointer
dereference. Fixing this 'properly' was deemed to much diff for
practically no gain that late in the release, so commit
0c2dc43d4fe1d026650b5e2920a021557f9534a6 just fixed the synptom, while
this commit here is fixing the cause plus adding a test.

diff --git a/methods/http.cc b/methods/http.cc
index ad1347d36fc5544d7db2e0b1318ee5642805c1e0..021b284d0fc09c666104e4e1878589a16d0d1a28 100644 (file)
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -772,8 +772,6 @@ bool HttpMethod::Configuration(string Message)
    if (ServerMethod::Configuration(Message) == false)
       return false;
 
-   DropPrivsOrDie();
-
    AllowRedirect = _config->FindB("Acquire::http::AllowRedirect",true);
    PipelineDepth = _config->FindI("Acquire::http::Pipeline-Depth",
                                  PipelineDepth);

diff --git a/methods/https.cc b/methods/https.cc
index 37a8ff5fd46a54067146279434a8eeabe936728c..32de42e4bd1b2689854423ac593b8b7c417f6979 100644 (file)
--- a/methods/https.cc
+++ b/methods/https.cc
@@ -37,16 +37,6 @@
                                                                        /*}}}*/
 using namespace std;
 
-bool HttpsMethod::Configuration(std::string Message)
-{
-   if (pkgAcqMethod::Configuration(Message) == false)
-      return false;
-
-   DropPrivsOrDie();
-
-   return true;
-}
-
 size_t
 HttpsMethod::parse_header(void *buffer, size_t size, size_t nmemb, void *userp)
 {
@@ -131,7 +121,7 @@ HttpsMethod::progress_callback(void *clientp, double dltotal, double /*dlnow*/,
 }
 
 // HttpsServerState::HttpsServerState - Constructor                    /*{{{*/
-HttpsServerState::HttpsServerState(URI Srv,HttpsMethod * /*Owner*/) : ServerState(Srv, NULL)
+HttpsServerState::HttpsServerState(URI Srv,HttpsMethod * Owner) : ServerState(Srv, Owner)
 {
    TimeOut = _config->FindI("Acquire::https::Timeout",TimeOut);
    ReceivedData = false;
@@ -335,13 +325,11 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
    curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, timeout);
 
    // set redirect options and default to 10 redirects
-   bool const AllowRedirect = _config->FindB("Acquire::https::AllowRedirect",
-       _config->FindB("Acquire::http::AllowRedirect",true));
    curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, AllowRedirect);
    curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 10);
 
    // debug
-   if(_config->FindB("Debug::Acquire::https", false))
+   if (Debug == true)
       curl_easy_setopt(curl, CURLOPT_VERBOSE, true);
 
    // error handling
@@ -378,7 +366,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
 
    // go for it - if the file exists, append on it
    File = new FileFd(Itm->DestFile, FileFd::WriteAny);
-   Server = new HttpsServerState(Itm->Uri, this);
+   Server = CreateServerState(Itm->Uri);
 
    // keep apt updated
    Res.Filename = Itm->DestFile;
@@ -473,6 +461,25 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
 
    return true;
 }
+                                                                       /*}}}*/
+// HttpsMethod::Configuration - Handle a configuration message         /*{{{*/
+bool HttpsMethod::Configuration(string Message)
+{
+   if (ServerMethod::Configuration(Message) == false)
+      return false;
+
+   AllowRedirect = _config->FindB("Acquire::https::AllowRedirect",
+       _config->FindB("Acquire::http::AllowRedirect", true));
+   Debug = _config->FindB("Debug::Acquire::https",false);
+
+   return true;
+}
+                                                                       /*}}}*/
+ServerState * HttpsMethod::CreateServerState(URI uri)                  /*{{{*/
+{
+   return new HttpsServerState(uri, this);
+}
+                                                                       /*}}}*/
 
 int main()
 {

diff --git a/methods/https.h b/methods/https.h
index 6917a6ff68981d311168ec85ae45ed1f9935b2bb..433a84680f4326e7db9433f770b0ae51ad185632 100644 (file)
--- a/methods/https.h
+++ b/methods/https.h
@@ -50,17 +50,14 @@ class HttpsServerState : public ServerState
 
    HttpsServerState(URI Srv, HttpsMethod *Owner);
    virtual ~HttpsServerState() {Close();};
-
-   bool ReceivedData;
 };
 
-class HttpsMethod : public pkgAcqMethod
+class HttpsMethod : public ServerMethod
 {
    // minimum speed in bytes/se that triggers download timeout handling
    static const int DL_MIN_SPEED = 10;
 
    virtual bool Fetch(FetchItem *);
-   virtual bool Configuration(std::string Message);
 
    static size_t parse_header(void *buffer, size_t size, size_t nmemb, void *userp);
    static size_t write_data(void *buffer, size_t size, size_t nmemb, void *userp);
@@ -69,12 +66,19 @@ class HttpsMethod : public pkgAcqMethod
    void SetupProxy();
    CURL *curl;
    FetchResult Res;
-   HttpsServerState *Server;
+   ServerState *Server;
+
+   // Used by ServerMethods unused by https
+   virtual void SendReq(FetchItem *) { exit(42); }
+   virtual void RotateDNS() { exit(42); }
 
    public:
    FileFd *File;
 
-   HttpsMethod() : pkgAcqMethod("1.2",Pipeline | SendConfig), Server(NULL), File(NULL)
+   virtual bool Configuration(std::string Message);
+   virtual ServerState * CreateServerState(URI uri);
+
+   HttpsMethod() : ServerMethod("1.2",Pipeline | SendConfig), File(NULL)
    {
       curl = curl_easy_init();
    };

diff --git a/methods/server.cc b/methods/server.cc
index c17f27f7375a18d5fbec90b7c320b7435be78095..91ec824d115120c2c62ccf996efa80bfbd6a93bf 100644 (file)
--- a/methods/server.cc
+++ b/methods/server.cc
@@ -238,7 +238,12 @@ ServerState::ServerState(URI Srv, ServerMethod *Owner) : ServerName(Srv), TimeOu
 
 bool ServerMethod::Configuration(string Message)                       /*{{{*/
 {
-   return pkgAcqMethod::Configuration(Message);
+   if (pkgAcqMethod::Configuration(Message) == false)
+      return false;
+
+   DropPrivsOrDie();
+
+   return true;
 }
                                                                        /*}}}*/
 

diff --git a/methods/server.h b/methods/server.h
index b974ec89a1b2f6604468e8f2cecd91794024fb83..3b232dcacab3bf985f1d06cd7b02f120d4c7eb35 100644 (file)
--- a/methods/server.h
+++ b/methods/server.h
@@ -37,6 +37,7 @@ struct ServerState
    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;
+   bool ReceivedData;
    time_t Date;
    bool HaveContent;
    enum {Chunked,Stream,Closes} Encoding;
@@ -75,7 +76,7 @@ struct ServerState
 
    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; JunkSize = 0;
-                StartPos = 0; Encoding = Closes; time(&Date); HaveContent = false;
+                StartPos = 0; ReceivedData = false; Encoding = Closes; time(&Date); HaveContent = false;
                 State = Header; Persistent = false; Pipeline = true; MaximumSize = 0;};
    virtual bool WriteResponse(std::string const &Data) = 0;
 

diff --git a/test/integration/test-bug-778375-server-has-no-reason-phrase b/test/integration/test-bug-778375-server-has-no-reason-phrase
new file mode 100755 (executable)
index 0000000..23481ef
--- /dev/null
+++ b/test/integration/test-bug-778375-server-has-no-reason-phrase
@@ -0,0 +1,40 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+
+setupenvironment
+configarchitecture 'native'
+
+echo 'found' > aptarchive/working
+changetohttpswebserver  -o 'aptwebserver::redirect::replace::/redirectme/=/' \
+       -o 'aptwebserver::httpcode::200=200' -o 'aptwebserver::httpcode::404=404' \
+       -o 'aptwebserver::httpcode::301=301'
+
+testdownload() {
+       rm -f downfile
+       msgtest "download of a $1 via" "${3%%:*}"
+       $2 --nomsg downloadfile "$3" downfile
+
+       cp rootdir/tmp/testsuccess.output download.log
+       #looking for "HTTP server doesn't give Reason-Phrase for 200"
+       testsuccess grep 'give Reason-Phrase for' download.log
+
+       if [ "$2" = 'testsuccess' ]; then
+               testfileequal downfile 'found'
+       else
+               testfailure test -e downfile
+       fi
+}
+
+runtest() {
+       testdownload 'file works' 'testsuccess' "$1/working"
+       testdownload 'file via redirect works' 'testsuccess' "$1/redirectme/working"
+
+       testdownload 'non-existent file fails' 'testfailure' "$1/failing"
+       testdownload 'non-existent file via redirect fails' 'testfailure' "$1/redirectme/failing"
+}
+
+runtest 'http://localhost:8080'
+runtest 'https://localhost:4433'

diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc
index 3403bbdd284f963f6d4e436bbd7304a2adb05d20..644629a33bc33b4ef8d791395644c01c4a2da50d 100644 (file)
--- a/test/interactive-helper/aptwebserver.cc
+++ b/test/interactive-helper/aptwebserver.cc
@@ -27,58 +27,58 @@
 #include <string>
 #include <vector>
 
-static char const * httpcodeToStr(int const httpcode)          /*{{{*/
+static std::string httpcodeToStr(int const httpcode)                   /*{{{*/
 {
    switch (httpcode)
    {
       // Informational 1xx
-      case 100: return "100 Continue";
-      case 101: return "101 Switching Protocols";
+      case 100: return _config->Find("aptwebserver::httpcode::100", "100 Continue");
+      case 101: return _config->Find("aptwebserver::httpcode::101", "101 Switching Protocols");
       // Successful 2xx
-      case 200: return "200 OK";
-      case 201: return "201 Created";
-      case 202: return "202 Accepted";
-      case 203: return "203 Non-Authoritative Information";
-      case 204: return "204 No Content";
-      case 205: return "205 Reset Content";
-      case 206: return "206 Partial Content";
+      case 200: return _config->Find("aptwebserver::httpcode::200", "200 OK");
+      case 201: return _config->Find("aptwebserver::httpcode::201", "201 Created");
+      case 202: return _config->Find("aptwebserver::httpcode::202", "202 Accepted");
+      case 203: return _config->Find("aptwebserver::httpcode::203", "203 Non-Authoritative Information");
+      case 204: return _config->Find("aptwebserver::httpcode::204", "204 No Content");
+      case 205: return _config->Find("aptwebserver::httpcode::205", "205 Reset Content");
+      case 206: return _config->Find("aptwebserver::httpcode::206", "206 Partial Content");
       // Redirections 3xx
-      case 300: return "300 Multiple Choices";
-      case 301: return "301 Moved Permanently";
-      case 302: return "302 Found";
-      case 303: return "303 See Other";
-      case 304: return "304 Not Modified";
-      case 305: return "304 Use Proxy";
-      case 307: return "307 Temporary Redirect";
+      case 300: return _config->Find("aptwebserver::httpcode::300", "300 Multiple Choices");
+      case 301: return _config->Find("aptwebserver::httpcode::301", "301 Moved Permanently");
+      case 302: return _config->Find("aptwebserver::httpcode::302", "302 Found");
+      case 303: return _config->Find("aptwebserver::httpcode::303", "303 See Other");
+      case 304: return _config->Find("aptwebserver::httpcode::304", "304 Not Modified");
+      case 305: return _config->Find("aptwebserver::httpcode::305", "305 Use Proxy");
+      case 307: return _config->Find("aptwebserver::httpcode::307", "307 Temporary Redirect");
       // Client errors 4xx
-      case 400: return "400 Bad Request";
-      case 401: return "401 Unauthorized";
-      case 402: return "402 Payment Required";
-      case 403: return "403 Forbidden";
-      case 404: return "404 Not Found";
-      case 405: return "405 Method Not Allowed";
-      case 406: return "406 Not Acceptable";
-      case 407: return "407 Proxy Authentication Required";
-      case 408: return "408 Request Time-out";
-      case 409: return "409 Conflict";
-      case 410: return "410 Gone";
-      case 411: return "411 Length Required";
-      case 412: return "412 Precondition Failed";
-      case 413: return "413 Request Entity Too Large";
-      case 414: return "414 Request-URI Too Large";
-      case 415: return "415 Unsupported Media Type";
-      case 416: return "416 Requested range not satisfiable";
-      case 417: return "417 Expectation Failed";
-      case 418: return "418 I'm a teapot";
+      case 400: return _config->Find("aptwebserver::httpcode::400", "400 Bad Request");
+      case 401: return _config->Find("aptwebserver::httpcode::401", "401 Unauthorized");
+      case 402: return _config->Find("aptwebserver::httpcode::402", "402 Payment Required");
+      case 403: return _config->Find("aptwebserver::httpcode::403", "403 Forbidden");
+      case 404: return _config->Find("aptwebserver::httpcode::404", "404 Not Found");
+      case 405: return _config->Find("aptwebserver::httpcode::405", "405 Method Not Allowed");
+      case 406: return _config->Find("aptwebserver::httpcode::406", "406 Not Acceptable");
+      case 407: return _config->Find("aptwebserver::httpcode::407", "407 Proxy Authentication Required");
+      case 408: return _config->Find("aptwebserver::httpcode::408", "408 Request Time-out");
+      case 409: return _config->Find("aptwebserver::httpcode::409", "409 Conflict");
+      case 410: return _config->Find("aptwebserver::httpcode::410", "410 Gone");
+      case 411: return _config->Find("aptwebserver::httpcode::411", "411 Length Required");
+      case 412: return _config->Find("aptwebserver::httpcode::412", "412 Precondition Failed");
+      case 413: return _config->Find("aptwebserver::httpcode::413", "413 Request Entity Too Large");
+      case 414: return _config->Find("aptwebserver::httpcode::414", "414 Request-URI Too Large");
+      case 415: return _config->Find("aptwebserver::httpcode::415", "415 Unsupported Media Type");
+      case 416: return _config->Find("aptwebserver::httpcode::416", "416 Requested range not satisfiable");
+      case 417: return _config->Find("aptwebserver::httpcode::417", "417 Expectation Failed");
+      case 418: return _config->Find("aptwebserver::httpcode::418", "418 I'm a teapot");
       // Server error 5xx
-      case 500: return "500 Internal Server Error";
-      case 501: return "501 Not Implemented";
-      case 502: return "502 Bad Gateway";
-      case 503: return "503 Service Unavailable";
-      case 504: return "504 Gateway Time-out";
-      case 505: return "505 HTTP Version not supported";
-   }
-   return NULL;
+      case 500: return _config->Find("aptwebserver::httpcode::500", "500 Internal Server Error");
+      case 501: return _config->Find("aptwebserver::httpcode::501", "501 Not Implemented");
+      case 502: return _config->Find("aptwebserver::httpcode::502", "502 Bad Gateway");
+      case 503: return _config->Find("aptwebserver::httpcode::503", "503 Service Unavailable");
+      case 504: return _config->Find("aptwebserver::httpcode::504", "504 Gateway Time-out");
+      case 505: return _config->Find("aptwebserver::httpcode::505", "505 HTTP Version not supported");
+   }
+   return "";
 }
                                                                        /*}}}*/
 static bool chunkedTransferEncoding(std::list<std::string> const &headers) {