don't try pipelining if server closes connections

If a server closes a connection after sending us a file that tends to
mean that its a type of server who always closes the connection – it is
therefore relatively pointless to try pipelining with it even if it
isn't a problem by itself: apt is just restarting the pipeline each
time after it got served one file and the connection is closed.

The problem starts if one or more proxies are between the server and apt
and they disagree about how the connection should be as in the
bugreporters case where the responses apt gets contain both Keep-Alive
and Proxy-Connection headers (which apt both ignores) indicating a
proxy is trying to keep a connection open while the response also
contains "Connection: close" indicating the opposite which apt
understands and respects as it is required to do.

We avoid stepping into this abyss by not performing pipelining anymore
if we got a respond with the indication to close connection if the
response was otherwise a success – error messages are sent by some
servers via this method as their pages tend to be created dynamically
and hence their size isn't known a priori to them.

Closes: #832113

diff --git a/methods/server.cc b/methods/server.cc
index aa1ee47543e0f7080b01f6869861ca7ca31626a6..af7276badd7a9f2ca2b7670b78ebb14b4f4f2efc 100644 (file)
--- a/methods/server.cc
+++ b/methods/server.cc
@@ -208,8 +208,16 @@ bool ServerState::HeaderLine(string Line)
    if (stringcasecmp(Tag,"Connection:") == 0)
    {
       if (stringcasecmp(Val,"close") == 0)
+      {
         Persistent = false;
-      if (stringcasecmp(Val,"keep-alive") == 0)
+        Pipeline = false;
+        /* Some servers send error pages (as they are dynamically generated)
+           for simplicity via a connection close instead of e.g. chunked,
+           so assuming an always closing server only if we get a file + close */
+        if (Result >= 200 && Result < 300)
+           PipelineAllowed = false;
+      }
+      else if (stringcasecmp(Val,"keep-alive") == 0)
         Persistent = true;
       return true;
    }