retry without partial data after a 416 response

[apt.git] / methods / http.cc

diff --git a/methods/http.cc b/methods/http.cc
index d2e03cfbc13352c295e13080d38c3f20a8beed73..2a8b7f0fccdfc883c9ef853ef825b205d604316d 100644 (file)
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -61,7 +61,7 @@ using namespace std;
 string HttpMethod::FailFile;
 int HttpMethod::FailFd = -1;
 time_t HttpMethod::FailTime = 0;
 string HttpMethod::FailFile;
 int HttpMethod::FailFd = -1;
 time_t HttpMethod::FailTime = 0;

-unsigned long PipelineDepth = 10;
+unsigned long PipelineDepth = 0;

 unsigned long TimeOut = 120;
 bool AllowRedirect = false;
 bool Debug = false;
 unsigned long TimeOut = 120;
 bool AllowRedirect = false;
 bool Debug = false;


@@ -602,8 +602,10 @@ bool ServerState::HeaderLine(string Line)
         return true;
 
       Size = strtoull(Val.c_str(), NULL, 10);
         return true;
 
       Size = strtoull(Val.c_str(), NULL, 10);

-      if (Size == ULLONG_MAX)
+      if (Size >= std::numeric_limits<unsigned long long>::max())

         return _error->Errno("HeaderLine", _("The HTTP server sent an invalid Content-Length header"));
         return _error->Errno("HeaderLine", _("The HTTP server sent an invalid Content-Length header"));

+      else if (Size == 0)
+        HaveContent = false;

       return true;
    }
 
       return true;
    }
 


@@ -616,8 +618,14 @@ bool ServerState::HeaderLine(string Line)
    if (stringcasecmp(Tag,"Content-Range:") == 0)
    {
       HaveContent = true;
    if (stringcasecmp(Tag,"Content-Range:") == 0)
    {
       HaveContent = true;

-      
-      if (sscanf(Val.c_str(),"bytes %llu-%*u/%llu",&StartPos,&Size) != 2)
+
+      // §14.16 says 'byte-range-resp-spec' should be a '*' in case of 416
+      if (Result == 416 && sscanf(Val.c_str(), "bytes */%llu",&Size) == 1)
+      {
+        StartPos = 1; // ignore Content-Length, it would override Size
+        HaveContent = false;
+      }
+      else if (sscanf(Val.c_str(),"bytes %llu-%*u/%llu",&StartPos,&Size) != 2)

         return _error->Error(_("The HTTP server sent an invalid Content-Range header"));
       if ((unsigned long long)StartPos > Size)
         return _error->Error(_("This HTTP server has broken range support"));
         return _error->Error(_("The HTTP server sent an invalid Content-Range header"));
       if ((unsigned long long)StartPos > Size)
         return _error->Error(_("This HTTP server has broken range support"));


@@ -667,7 +675,12 @@ void HttpMethod::SendReq(FetchItem *Itm,CircleBuf &Out)
 
    // The HTTP server expects a hostname with a trailing :port
    char Buf[1000];
 
    // The HTTP server expects a hostname with a trailing :port
    char Buf[1000];

-   string ProperHost = Uri.Host;
+   string ProperHost;
+
+   if (Uri.Host.find(':') != string::npos)
+      ProperHost = '[' + Uri.Host + ']';
+   else
+      ProperHost = Uri.Host;

    if (Uri.Port != 0)
    {
       sprintf(Buf,":%u",Uri.Port);
    if (Uri.Port != 0)
    {
       sprintf(Buf,":%u",Uri.Port);


@@ -677,24 +690,27 @@ void HttpMethod::SendReq(FetchItem *Itm,CircleBuf &Out)
    // Just in case.
    if (Itm->Uri.length() >= sizeof(Buf))
        abort();
    // Just in case.
    if (Itm->Uri.length() >= sizeof(Buf))
        abort();

-       
-   /* Build the request. We include a keep-alive header only for non-proxy
-      requests. This is to tweak old http/1.0 servers that do support keep-alive
-      but not HTTP/1.1 automatic keep-alive. Doing this with a proxy server 
-      will glitch HTTP/1.0 proxies because they do not filter it out and 
-      pass it on, HTTP/1.1 says the connection should default to keep alive
-      and we expect the proxy to do this */
+
+   /* 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())
    if (Proxy.empty() == true || Proxy.Host.empty())

-      sprintf(Buf,"GET %s HTTP/1.1\r\nHost: %s\r\nConnection: keep-alive\r\n",
-             QuoteString(Uri.Path,"~").c_str(),ProperHost.c_str());
+      requesturi = Uri.Path;

    else
    else

-   {
-      /* Generate a cache control header if necessary. We place a max
-                cache age on index files, optionally set a no-cache directive
-                and a no-store directive for archives. */
-      sprintf(Buf,"GET %s HTTP/1.1\r\nHost: %s\r\n",
-             Itm->Uri.c_str(),ProperHost.c_str());
-   }
+      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 */
+   sprintf(Buf,"GET %s HTTP/1.1\r\nHost: %s\r\n",
+          requesturi.c_str(),ProperHost.c_str());
+

    // generate a cache control header (if needed)
    if (_config->FindB("Acquire::http::No-Cache",false) == true) 
    {
    // generate a cache control header (if needed)
    if (_config->FindB("Acquire::http::No-Cache",false) == true) 
    {


@@ -758,7 +774,7 @@ void HttpMethod::SendReq(FetchItem *Itm,CircleBuf &Out)
           Base64Encode(Uri.User + ":" + Uri.Password) + "\r\n";
    }
    Req += "User-Agent: " + _config->Find("Acquire::http::User-Agent",
           Base64Encode(Uri.User + ":" + Uri.Password) + "\r\n";
    }
    Req += "User-Agent: " + _config->Find("Acquire::http::User-Agent",

-               "Debian APT-HTTP/1.3 ("VERSION")") + "\r\n\r\n";
+               "Debian APT-HTTP/1.3 (" PACKAGE_VERSION ")") + "\r\n\r\n";

    
    if (Debug == true)
       cerr << Req << endl;
    
    if (Debug == true)
       cerr << Req << endl;


@@ -971,12 +987,7 @@ HttpMethod::DealWithHeaders(FetchResult &Res,ServerState *Srv)
       {
         URI Uri = Queue->Uri;
         if (Uri.Host.empty() == false)
       {
         URI Uri = Queue->Uri;
         if (Uri.Host.empty() == false)

-        {
-           if (Uri.Port != 0)
-              strprintf(NextURI, "http://%s:%u", Uri.Host.c_str(), Uri.Port);
-           else
-              NextURI = "http://" + Uri.Host;
-        }
+            NextURI = URI::SiteOnly(Uri);

         else
            NextURI.clear();
         NextURI.append(DeQuoteString(Srv->Location));
         else
            NextURI.clear();
         NextURI.append(DeQuoteString(Srv->Location));


@@ -985,10 +996,20 @@ HttpMethod::DealWithHeaders(FetchResult &Res,ServerState *Srv)
       else
       {
          NextURI = DeQuoteString(Srv->Location);
       else
       {
          NextURI = DeQuoteString(Srv->Location);

-         return TRY_AGAIN_OR_REDIRECT;
+         URI tmpURI = NextURI;
+         // Do not allow a redirection to switch protocol
+         if (tmpURI.Access == "http")
+            return TRY_AGAIN_OR_REDIRECT;

       }
       /* else pass through for error message */
    }
       }
       /* else pass through for error message */
    }

+   // retry after an invalid range response without partial data
+   else if (Srv->Result == 416 && FileExists(Queue->DestFile) == true &&
+        unlink(Queue->DestFile.c_str()) == 0)
+   {
+      NextURI = Queue->Uri;
+      return TRY_AGAIN_OR_REDIRECT;
+   }

  
    /* We have a reply we dont handle. This should indicate a perm server
       failure */
  
    /* We have a reply we dont handle. This should indicate a perm server
       failure */


@@ -1394,7 +1415,7 @@ bool HttpMethod::AutoDetectProxy()
    char buf[512];
    int InFd = Pipes[0];
    close(Pipes[1]);
    char buf[512];
    int InFd = Pipes[0];
    close(Pipes[1]);

-   int res = read(InFd, buf, sizeof(buf));
+   int res = read(InFd, buf, sizeof(buf)-1);

    ExecWait(Process, "ProxyAutoDetect", true);
 
    if (res < 0)
    ExecWait(Process, "ProxyAutoDetect", true);
 
    if (res < 0)