Re: Bug#64766: apt: The apt program does not obey the HTTP/1.1 RFC
On Fri, 26 May 2000 amb@gedanken.demon.co.uk wrote:
> The http method that comes with apt claims to be HTTP/1.1 compliant, but
> checking the behaviour against the RFC indicates non-compliances that may cause
> it to fail with HTTP/1.0 servers.
Here is a cute little patch that makes APT pre-emptively close the
connection if the server is not going to be persistant. That fixes the
only RFC incompatibility you found.
What this will fix is a corner case where a pipelining server sets
Connection: close in the middle of a request stream and then proceeds to
not linger. No such server should exist, that would be really seriously
damaged. It will also slightly speed up HTTP/1.0 servers since closing the
socket is done in parallel with establishing the next connection.
It doesn't help broken servers (any that give 'Connection Reset').
The only acceptable fix for that is to disable pipelining by default.
If someone does want to fix any server that does the 'Connection Reset'
bit, please check out:
http://www.apache.org/docs-1.2/misc/fin_wait_2.html
Read the appendix.
Jason
Index: http.cc
===================================================================
RCS file: /cvs/deity/apt/methods/http.cc,v
retrieving revision 1.45
diff -u -r1.45 http.cc
--- http.cc 2000/05/12 05:04:57 1.45
+++ http.cc 2000/05/28 04:10:56
@@ -6,15 +6,13 @@
HTTP Aquire Method - This is the HTTP aquire method for APT.
It uses HTTP/1.1 and many of the fancy options there-in, such as
- pipelining, range, if-range and so on. It accepts on the command line
- a list of url destination pairs and writes to stdout the status of the
- operation as defined in the APT method spec.
-
- It is based on a doubly buffered select loop. All the requests are
+ pipelining, range, if-range and so on.
+
+ It is based on a doubly buffered select loop. A groupe of requests are
fed into a single output buffer that is constantly fed out the
socket. This provides ideal pipelining as in many cases all of the
requests will fit into a single packet. The input socket is buffered
- the same way and fed into the fd for the file.
+ the same way and fed into the fd for the file (may be a pipe in future).
This double buffering provides fairly substantial transfer rates,
compared to wget the http method is about 4% faster. Most importantly,
@@ -258,9 +256,6 @@
// ServerState::Open - Open a connection to the server /*{{{*/
// ---------------------------------------------------------------------
/* This opens a connection to the server. */
-string LastHost;
-int LastPort = 0;
-struct addrinfo *LastHostAddr = 0;
bool ServerState::Open()
{
// Use the already open connection if possible.
@@ -270,7 +265,8 @@
Close();
In.Reset();
Out.Reset();
-
+ Persistent = true;
+
// Determine the proxy setting
if (getenv("http_proxy") == 0)
{
@@ -379,10 +375,15 @@
return 2;
I = J;
}
+
+ // Tidy up the connection persistance state.
+ if (Encoding == Closes && HaveContent == true)
+ Persistent = false;
+
return 0;
}
while (Owner->Go(false,this) == true);
-
+
return 1;
}
/*}}}*/
@@ -524,6 +525,18 @@
if (sscanf(Line.c_str(),"HTTP %u %[^\n]",&Result,Code) != 2)
return _error->Error("The http server sent an invalid reply header");
}
+
+ /* Check the HTTP response header to get the default persistance
+ state. */
+ if (Major < 1)
+ Persistent = false;
+ else
+ {
+ if (Major == 1 && Minor <= 0)
+ Persistent = false;
+ else
+ Persistent = true;
+ }
return true;
}
@@ -564,11 +577,19 @@
{
HaveContent = true;
if (stringcasecmp(Val,"chunked") == 0)
- Encoding = Chunked;
-
+ Encoding = Chunked;
return true;
}
+ if (stringcasecmp(Tag,"Connection:") == 0)
+ {
+ if (stringcasecmp(Val,"close") == 0)
+ Persistent = false;
+ if (stringcasecmp(Val,"keep-alive") == 0)
+ Persistent = true;
+ return true;
+ }
+
if (stringcasecmp(Tag,"Last-Modified:") == 0)
{
if (StrToTime(Val,Date) == false)
@@ -678,10 +699,12 @@
FD_ZERO(&rfds);
FD_ZERO(&wfds);
- // Add the server
- if (Srv->Out.WriteSpace() == true && Srv->ServerFd != -1)
+ /* Add the server. We only send more requests if the connection will
+ be persisting */
+ if (Srv->Out.WriteSpace() == true && Srv->ServerFd != -1
+ && Srv->Persistent == true)
FD_SET(Srv->ServerFd,&wfds);
- if (Srv->In.ReadSpace() == true && Srv->ServerFd != -1)
+ if (Srv->In.ReadSpace() == true && Srv->ServerFd != -1)
FD_SET(Srv->ServerFd,&rfds);
// Add the file
@@ -999,7 +1022,15 @@
delete Server;
Server = new ServerState(Queue->Uri,this);
}
-
+
+ /* If the server has explicitly said this is the last connection
+ then we pre-emptively shut down the pipeline and tear down
+ the connection. This will speed up HTTP/1.0 servers a tad
+ since we don't have to wait for the close sequence to
+ complete */
+ if (Server->Persistent == false)
+ Server->Close();
+
// Reset the pipeline
if (Server->ServerFd == -1)
QueueBack = Queue;
@@ -1082,7 +1113,7 @@
}
else
Fail(true);
-
+
break;
}
Index: http.h
===================================================================
RCS file: /cvs/deity/apt/methods/http.h,v
retrieving revision 1.7
diff -u -r1.7 http.h
--- http.h 1999/12/09 03:45:56 1.7
+++ http.h 2000/05/28 04:10:56
@@ -87,6 +87,9 @@
bool HaveContent;
enum {Chunked,Stream,Closes} Encoding;
enum {Header, Data} State;
+ bool Persistent;
+
+ // This is a Persistent attribute of the server itself.
bool Pipeline;
HttpMethod *Owner;
Reply to: