On Mon, Feb 16, 2015 at 01:16:19AM +0100, Tomasz Buchert wrote: > The tricky HTTPS server returns this line: "HTTP/1.1 302". Note that > there is no "explanation" for the status code 302 (it should be > "Found"). Anyway, this is fine, the code seems to be prepared for > that case: elements is set to 3 in server.cc:128.
apt is since 0.8.0~pre2 (23 Aug 2010). I think back then it was also a sourceforge server triggering this. Note that this is a violation of the HTTP1.1 spec (see rfc7230 section 3.1.2) which allows for an empty reason-phrase, but the space before that is non-optional. > However, Owner is NULL (I don't know why, I don't know the code, but > it is) so Owner->Debug fails in server.cc:132. > > The attached patch checks whether Owner is NULL before dereferencing > it. This fixes this problem for me, but somebody who knows what Owner > is should make sure that it makes sense. Feel free to adjust the > patch to your needs, it's in public domain. <rambling> That is a good catch! 'Owner' refers here to the ServerMethod owning the ServerState (that was a very helpful explanation, wasn't it? ;) ). It boils down to this: In Sep 2013 I wanted to fix some bugs in https by using less curl and more of our own http code. For this I invented a bunch of Server classes as parents for http and https – in handsight, I really should have used another name, but well, anyway – expect that both were completely different in their implementation. Somehow I managed to pull it of anyway with the result that they share most of their State parsing/tracking which is quite helpful. It also means through that the actual Methods using the State are still very different so getting a common interface for them was hard. Somewhere down that line I took a shortcut giving the HttpsState a NULL for its owner as it 'never' really needed it and can hence be fixed 'later' correctly, right? Fast forward one and a half years and the 'never' as well as the 'later' is spoiled. Its a bit ironic that a debug message does this to me… </rambling> The proposed patch works just fine as the other users for 'Owner' aren't used by https and for http its always properly set (and nobody dies if a debug message isn't shown even if requested) and at that point in the release I guess everyone will be happy about a one-line fix. (Michael is uploading it any minute now) Attached is my fullblown 'proper' patch with a testcase I am going to apply to our /experimental branch for comparison in the meantime. Best regards David Kalnischkies
diff --git a/methods/https.cc b/methods/https.cc
index 3a5981b..444bdef 100644
--- a/methods/https.cc
+++ b/methods/https.cc
@@ -109,7 +109,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);
Reset();
@@ -313,13 +313,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
@@ -356,7 +354,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;
@@ -451,6 +449,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 411b714..f8d302d 100644
--- a/methods/https.h
+++ b/methods/https.h
@@ -52,7 +52,7 @@ class HttpsServerState : public ServerState
virtual ~HttpsServerState() {Close();};
};
-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;
@@ -65,13 +65,20 @@ class HttpsMethod : public pkgAcqMethod
void SetupProxy();
CURL *curl;
FetchResult Res;
- HttpsServerState *Server;
+ ServerState *Server;
bool ReceivedData;
+ // 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), File(NULL)
+
+ virtual bool Configuration(std::string Message);
+ virtual ServerState * CreateServerState(URI uri);
+
+ HttpsMethod() : ServerMethod("1.2",Pipeline | SendConfig), File(NULL)
{
File = 0;
curl = curl_easy_init();
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
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 cd52da6..6e1e44b 100644
--- 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) {
signature.asc
Description: Digital signature
