summaryrefslogtreecommitdiff
path: root/methods/basehttp.cc
diff options
context:
space:
mode:
authorJulian Andres Klode <julian.klode@canonical.com>2018-08-31 16:07:07 +0200
committerJulian Andres Klode <julian.klode@canonical.com>2018-09-28 13:13:26 +0200
commit3de7454c796f245371c33076ae01529d6d36d715 (patch)
tree63022f84a52894f738b9f3eb7aabe227cf9bd09c /methods/basehttp.cc
parentd66bd6e5e9ae96676e805cce43937a0528cebe1b (diff)
http: Stop pipeline after close only if it was not filled before
It is perfectly valid behavior for a server to respond with Connection: close eventually, even when pipelining. Turning off pipelining due to that is wrong. For example, some Ubuntu mirrors close the connection after 101 requests. If I have more packages to install, only the first 101 would benefit from pipelining. This commit introduces a new check to only turn of pipelining for future connections if the pipeline for this connection did not have 3 successful fetches before, that should work quite well to detect broken server/proxy combinations like in bug 832113. (cherry picked from commit df696650b7a8c58bbd92e0e1619e956f21010a96) LP: #1794957
Diffstat (limited to 'methods/basehttp.cc')
-rw-r--r--methods/basehttp.cc24
1 files changed, 19 insertions, 5 deletions
diff --git a/methods/basehttp.cc b/methods/basehttp.cc
index 91e71b5d4..6e8173c78 100644
--- a/methods/basehttp.cc
+++ b/methods/basehttp.cc
@@ -39,6 +39,10 @@ string BaseHttpMethod::FailFile;
int BaseHttpMethod::FailFd = -1;
time_t BaseHttpMethod::FailTime = 0;
+// Number of successful requests in a pipeline needed to continue
+// pipelining after a connection reset.
+constexpr int PIPELINE_MIN_SUCCESSFUL_ANSWERS_TO_CONTINUE = 3;
+
// ServerState::RunHeaders - Get the headers before the data /*{{{*/
// ---------------------------------------------------------------------
/* Returns 0 if things are OK, 1 if an IO error occurred and 2 if a header
@@ -215,8 +219,11 @@ bool RequestState::HeaderLine(string const &Line) /*{{{*/
/* 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)
+ if (Result >= 200 && Result < 300 && Server->PipelineAnswersReceived < PIPELINE_MIN_SUCCESSFUL_ANSWERS_TO_CONTINUE)
+ {
Server->PipelineAllowed = false;
+ Server->PipelineAnswersReceived = 0;
+ }
}
else if (stringcasecmp(Val,"keep-alive") == 0)
Server->Persistent = true;
@@ -267,6 +274,7 @@ void ServerState::Reset() /*{{{*/
Pipeline = false;
PipelineAllowed = true;
RangesAllowed = true;
+ PipelineAnswersReceived = 0;
}
/*}}}*/
@@ -593,8 +601,10 @@ int BaseHttpMethod::Loop()
Server->Close();
// Reset the pipeline
- if (Server->IsOpen() == false)
+ if (Server->IsOpen() == false) {
QueueBack = Queue;
+ Server->PipelineAnswersReceived = 0;
+ }
// Connect to the host
switch (Server->Open())
@@ -752,6 +762,10 @@ int BaseHttpMethod::Loop()
BeforeI = I;
}
}
+ if (Server->Pipeline == true)
+ {
+ Server->PipelineAnswersReceived++;
+ }
Res.TakeHashes(*resultHashes);
URIDone(Res);
}
@@ -861,9 +875,9 @@ unsigned long long BaseHttpMethod::FindMaximumObjectSizeInQueue() const /*{{{*/
return MaxSizeInQueue;
}
/*}}}*/
-BaseHttpMethod::BaseHttpMethod(std::string &&Binary, char const * const Ver,unsigned long const Flags) :/*{{{*/
- aptAuthConfMethod(std::move(Binary), Ver, Flags), Server(nullptr), PipelineDepth(10),
- AllowRedirect(false), Debug(false)
+BaseHttpMethod::BaseHttpMethod(std::string &&Binary, char const *const Ver, unsigned long const Flags) /*{{{*/
+ : aptAuthConfMethod(std::move(Binary), Ver, Flags), Server(nullptr),
+ AllowRedirect(false), Debug(false), PipelineDepth(10)
{
}
/*}}}*/