request absolute URIs from proxies again (0.9.9.3 regession)

Commit 2b9c9b7f28b18f6ae3e422020e8934872b06c9f3 not only removes
keep-alive, but also changes the request URI send to proxies which are
required to be absolute URIs rather than the usual absolute paths.

Closes: 717891

diff --git a/methods/http.cc b/methods/http.cc
index 6e03e9d63edac56044e9739db9ca99729ff84aca..82456d78ba07fbd1a394a9667d0f1fd7281d5155 100644 (file)
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -682,15 +682,27 @@ void HttpMethod::SendReq(FetchItem *Itm,CircleBuf &Out)
    // Just in case.
    if (Itm->Uri.length() >= sizeof(Buf))
        abort();
-       
+
+   /* RFC 2616 §5.1.2 requires absolute URIs for requests to proxies,
+      but while its a must for all servers to accept absolute URIs,
+      it is assumed clients will sent an absolute path for non-proxies */
+   std::string requesturi;
+   if (Proxy.empty() == true || Proxy.Host.empty())
+      requesturi = Uri.Path;
+   else
+      requesturi = Itm->Uri;
+
+   // The "+" is encoded as a workaround for a amazon S3 bug
+   // see LP bugs #1003633 and #1086997.
+   requesturi = QuoteString(requesturi, "+~ ");
+
    /* Build the request. No keep-alive is included as it is the default
       in 1.1, can cause problems with proxies, and we are an HTTP/1.1
       client anyway.
       C.f. https://tools.ietf.org/wg/httpbis/trac/ticket/158 */
-   // see LP bugs #1003633 and #1086997. The "+" is encoded as a workaround
-   // for a amazon S3 bug
    sprintf(Buf,"GET %s HTTP/1.1\r\nHost: %s\r\n",
-          QuoteString(Uri.Path,"+~ ").c_str(),ProperHost.c_str());
+          requesturi.c_str(),ProperHost.c_str());
+
    // generate a cache control header (if needed)
    if (_config->FindB("Acquire::http::No-Cache",false) == true) 
    {

diff --git a/test/integration/test-bug-717891-abolute-uris-for-proxies b/test/integration/test-bug-717891-abolute-uris-for-proxies
new file mode 100755 (executable)
index 0000000..e9c3849
--- /dev/null
+++ b/test/integration/test-bug-717891-abolute-uris-for-proxies
@@ -0,0 +1,28 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+setupenvironment
+configarchitecture 'amd64'
+
+buildsimplenativepackage 'unrelated' 'all' '0.5~squeeze1' 'unstable'
+
+setupaptarchive
+changetowebserver --request-absolute='uri'
+
+msgtest 'Check that absolute paths are' 'not accepted'
+aptget update >/dev/null 2>&1 && msgfail || msgpass
+
+echo 'Acquire::http::Proxy "http://localhost:8080";' > rootdir/etc/apt/apt.conf.d/99proxy
+
+msgtest 'Check that requests to proxies are' 'absolute uris'
+aptget update >/dev/null 2>&1 && msgpass || msgfail
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following NEW packages will be installed:
+  unrelated
+0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
+Inst unrelated (0.5~squeeze1 unstable [all])
+Conf unrelated (0.5~squeeze1 unstable [all])' aptget install unrelated -s

diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc
index a8d191d0e332d261a6e603e4d70c02afe5e73fff..fde95fec9c8c33b2e4f6c30b070f2826a576ea3a 100644 (file)
--- a/test/interactive-helper/aptwebserver.cc
+++ b/test/interactive-helper/aptwebserver.cc
@@ -319,6 +319,33 @@ bool parseFirstLine(int const client, std::string const &request,  /*{{{*/
       sendError(client, 500, request, sendContent, "Filename contains an unencoded space");
       return false;
    }
+
+   std::string host = LookupTag(request, "Host", "");
+   if (host.empty() == true)
+   {
+      // RFC 2616 §14.23 requires Host
+      sendError(client, 400, request, sendContent, "Host header is required");
+      return false;
+   }
+   host = "http://" + host;
+
+   // Proxies require absolute uris, so this is a simple proxy-fake option
+   std::string const absolute = _config->Find("aptwebserver::request::absolute", "uri,path");
+   if (strncmp(host.c_str(), filename.c_str(), host.length()) == 0)
+   {
+      if (absolute.find("uri") == std::string::npos)
+      {
+        sendError(client, 400, request, sendContent, "Request is absoluteURI, but configured to not accept that");
+        return false;
+      }
+      // strip the host from the request to make it an absolute path
+      filename.erase(0, host.length());
+   }
+   else if (absolute.find("path") == std::string::npos)
+   {
+      sendError(client, 400, request, sendContent, "Request is absolutePath, but configured to not accept that");
+      return false;
+   }
    filename = DeQuoteString(filename);
 
    // this is not a secure server, but at least prevent the obvious …
@@ -342,6 +369,7 @@ int main(int const argc, const char * argv[])
 {
    CommandLine::Args Args[] = {
       {0, "port", "aptwebserver::port", CommandLine::HasArg},
+      {0, "request-absolute", "aptwebserver::request::absolute", CommandLine::HasArg},
       {'c',"config-file",0,CommandLine::ConfigFile},
       {'o',"option",0,CommandLine::ArbItem},
       {0,0,0,0}
@@ -447,14 +475,6 @@ int main(int const argc, const char * argv[])
            if (parseFirstLine(client, *m, filename, sendContent, closeConnection) == false)
               continue;
 
-           std::string host = LookupTag(*m, "Host", "");
-           if (host.empty() == true)
-           {
-              // RFC 2616 §14.23 requires Host
-              sendError(client, 400, *m, sendContent, "Host header is required");
-              continue;
-           }
-
            // string replacements in the requested filename
            ::Configuration::Item const *Replaces = _config->Tree("aptwebserver::redirect::replace");
            if (Replaces != NULL)