[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#64766: marked as done (apt: The apt program does not obey the HTTP/1.1 RFC)



Your message dated Sat, 27 May 2000 22:29:23 -0600 (MDT)
with message-id <[🔎] Pine.LNX.3.96.1000527221119.26509H-100000@wakko.deltatee.com>
and subject line Bug#64766: apt: The apt program does not obey the HTTP/1.1 RFC
has caused the attached Bug report to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what I am
talking about this indicates a serious mail system misconfiguration
somewhere.  Please contact me immediately.)

Darren Benham
(administrator, Debian Bugs database)

--------------------------------------
Received: (at submit) by bugs.debian.org; 27 May 2000 06:51:56 +0000
>From amb@gedanken.demon.co.uk Sat May 27 01:51:56 2000
Return-path: <amb@gedanken.demon.co.uk>
Received: from anchor-post-32.mail.demon.net [194.217.242.90] 
	by master.debian.org with esmtp (Exim 3.12 2 (Debian))
	id 12vaRv-00072w-00; Sat, 27 May 2000 01:51:55 -0500
Received: from gedanken.demon.co.uk ([158.152.211.20] helo=g2.gedanken)
	by anchor-post-32.mail.demon.net with esmtp (Exim 2.12 #1)
	id 12vaRr-0006AW-0W
	for submit@bugs.debian.org; Sat, 27 May 2000 07:51:52 +0100
Received: from amb by g2.gedanken with local (Exim 3.12 #1 (Debian))
	id 12vNME-0002oI-00; Fri, 26 May 2000 17:53:10 +0100
From: amb@gedanken.demon.co.uk
Subject: apt: The apt program does not obey the HTTP/1.1 RFC
To: submit@bugs.debian.org
X-Mailer: bug 3.2.10
Message-Id: <[🔎] E12vNME-0002oI-00@g2.gedanken>
Date: Fri, 26 May 2000 17:53:10 +0100
Delivered-To: submit@bugs.debian.org

Package: apt
Version: 0.3.18
Severity: normal

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.


Compliance Check
----------------

In the following text the quotes from the HTTP/1.1 RFC (RFC2616) are shown with
a '|' in the left margin.


| 8 Connections
| 
| 8.1 Persistent Connections
| 
| 8.1.1 Purpose

This is a description of persistent connections and why they are a good thing.
I have skipped the next few paragraphs.

|    HTTP implementations SHOULD implement persistent connections.

apt does this.

| 8.1.2 Overall Operation
| 
|    A significant difference between HTTP/1.1 and earlier versions of
|    HTTP is that persistent connections are the default behavior of any
|    HTTP connection. That is, unless otherwise indicated, the client
|    SHOULD assume that the server will maintain a persistent connection,
|    even after error responses from the server.

apt does this.

|    Persistent connections provide a mechanism by which a client and a
|    server can signal the close of a TCP connection. This signaling takes
|    place using the Connection header field (section 14.10). Once a close
|    has been signaled, the client MUST NOT send any more requests on that
|    connection.

I think that apt does this.

| 8.1.2.1 Negotiation
| 
|    An HTTP/1.1 server MAY assume that a HTTP/1.1 client intends to
|    maintain a persistent connection unless a Connection header including
|    the connection-token "close" was sent in the request. If the server
|    chooses to close the connection immediately after sending the
|    response, it SHOULD send a Connection header including the
|    connection-token close.
|
|    An HTTP/1.1 client MAY expect a connection to remain open, but would
|    decide to keep it open based on whether the response from a server
|    contains a Connection header with the connection-token close. In case
|    the client does not want to maintain a connection for more than that
|    request, it SHOULD send a Connection header including the
|    connection-token close.

apt does not send "Connection: close" headers because it wants to use persistent
connections where possible.

|    If either the client or the server sends the close token in the
|    Connection header, that request becomes the last one for the
|    connection.

## Non-compliance ##

apt ignores the "Connection: close" header that comes from the server if there
is one.

|    Clients and servers SHOULD NOT assume that a persistent connection is
|    maintained for HTTP versions less than 1.1 unless it is explicitly
|    signaled. See section 19.6.2 for more information on backward
|    compatibility with HTTP/1.0 clients.

## Non-compliance ##

apt assumes that HTTP/1.0 servers will have persistent connections.

|    In order to remain persistent, all messages on the connection MUST
|    have a self-defined message length (i.e., one not defined by closure
|    of the connection), as described in section 4.4.

apt does not send requests with bodies so this is not applicable.

| 8.1.2.2 Pipelining
| 
|    A client that supports persistent connections MAY "pipeline" its
|    requests (i.e., send multiple requests without waiting for each
|    response). A server MUST send its responses to those requests in the
|    same order that the requests were received.

This applies to servers and not clients, apt expects the server to behave
correctly.

|    Clients which assume persistent connections and pipeline immediately
|    after connection establishment SHOULD be prepared to retry their
|    connection if the first pipelined attempt fails. If a client does
|    such a retry, it MUST NOT pipeline before it knows the connection is
|    persistent. Clients MUST also be prepared to resend their requests if
|    the server closes the connection before sending all of the
|    corresponding responses.

## Non-compliance ##

apt will not retry if the there is a failure.  There also seem to be conditions
with HTTP/1.0 servers where apt apt expects persistent connections, but failure
to read the second set of data is seen as a server failure.

|    Clients SHOULD NOT pipeline requests using non-idempotent methods or
|    non-idempotent sequences of methods (see section 9.1.2). Otherwise, a
|    premature termination of the transport connection could lead to
|    indeterminate results. A client wishing to send a non-idempotent
|    request SHOULD wait to send that request until it has received the
|    response status for the previous request.

Not applicable to apt.


| 19.6.2 Compatibility with HTTP/1.0 Persistent Connections
| 
|    Some clients and servers might wish to be compatible with some
|    previous implementations of persistent connections in HTTP/1.0
|    clients and servers. Persistent connections in HTTP/1.0 are
|    explicitly negotiated as they are not the default behavior.

## Non-compliance ##

apt assumes that persistent connections are the default to HTTP/1.0 servers.
There is no way that apt will consider a connection to be non-persistent.


Summary
-------

When apt is talking to a server it needs to determine if the server can support
persistent connections.  This should be done by looking at the header of the
reply.

Header line          Persistent?

HTTP/1.1                yes
HTTP/1.0                no
Connection: close       no

Currently apt assumes that all servers will support persistent connections.
This is true if the server supports HTTP/1.1 but the default for HTTP/1.0
servers should be that the connection is not persistent and that pipelining
cannot be performed.  If a persistent connection is required to an HTTP/1.0
server then RFC2068 should be consulted and the appropriate header used.

---------------------------------------
Received: (at 64766-done) by bugs.debian.org; 28 May 2000 04:29:12 +0000
>From jgg@ualberta.ca Sat May 27 23:29:12 2000
Return-path: <jgg@ualberta.ca>
Received: from hawkmoth.tac.net (telusvelocity.net) [205.233.109.54] 
	by master.debian.org with smtp (Exim 3.12 2 (Debian))
	id 12vuhM-0002Nm-00; Sat, 27 May 2000 23:29:12 -0500
Received: (qmail 11469 invoked from network); 28 May 2000 04:29:08 -0000
Received: from unknown (HELO wakko.deltatee.com) (209.115.196.25)
  by hawkmoth.tac.net with SMTP; 28 May 2000 04:29:08 -0000
Received: from localhost (wakko.deltatee.com) [127.0.0.1] (jgg)
	by wakko.deltatee.com with smtp (Exim 2.11 #1)
	id 12vuhY-00011D-00 (Debian); Sat, 27 May 2000 22:29:24 -0600
Date: Sat, 27 May 2000 22:29:23 -0600 (MDT)
From: Jason Gunthorpe <jgg@ualberta.ca>
X-Sender: jgg@wakko.deltatee.com
Reply-To: Jason Gunthorpe <jgg@ualberta.ca>
To: amb@gedanken.demon.co.uk, 64766-done@bugs.debian.org
cc: APT Development Team <deity@lists.debian.org>
Subject: Re: Bug#64766: apt: The apt program does not obey the HTTP/1.1 RFC
In-Reply-To: <[🔎] E12vNME-0002oI-00@g2.gedanken>
Message-ID: <[🔎] Pine.LNX.3.96.1000527221119.26509H-100000@wakko.deltatee.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Delivered-To: 64766-done@bugs.debian.org


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: