Interaction between keep-alive, http 1.0 client and 1.1 server [rt.cpan.org #104430]

Migrated from rt.cpan.org#104430 (status was 'new')

Requestors:

• pjb1008@cam.ac.uk

From pjb1008@cam.ac.uk on 2015-05-14 13:34:02:

Hi,

I've got a problem getting keep-alives to work with an XML-RPC client that 
I believe to be due to a bug in Net::HTTP. I'm using version 6.06, but I 
can't see any significant differences in this area between that version 
and the latest, 6.08_002.

What's happening is this:

1st request
-----------
Client make a connection to the server.
Client sends request in HTTP/1.0 with Keep-Alive: 300 and Connection: Keep-Alive.
Server sends response in HTTP/1.1 with Connection: Keep-Alive.

2nd request
-----------
Client sends request in HTTP/1.0 with no Keep-Alive: or Connection: headers.
Server sends response in HTTP/1.1 with no Connection: Keep-Alive.

3rd request
-----------
Client discovers that the socket is marked readable (I think the server 
has closed the connection already) so it closes the connection.
Client makes a new connection to the server.
Client sends request in HTTP/1.0 with Keep-Alive: 300 and Connection: Keep-Alive.
Server sends response in HTTP/1.1 with Connection: Keep-Alive.
etc.

4th request
-----------
Client sends request in HTTP/1.0 with no Keep-Alive: or Connection: headers.
Server sends response in HTTP/1.1 with no Connection: Keep-Alive.

The behaviour is that the client alternates between using, and not using, 
keep-alives.

The server's first response is 1.1 and without Connection: close so the 
socket is legitimately considered for re-use. However, the client doesn't 
explicitly request on the next request Keep-Alive because the server is 
1.1. The server's second response results in the connection being closed 
because the client is 1.0 and it didn't request keep-alive. I think the 
server is behaving correctly, and that is was an error not to emit 
Connection: keep-alive when making a 1.0 request.

The relevant section of code is this: (Net/HTTP/Methods.pm)

     unless (grep lc($_) eq "close", @connection) {
         if ($self->keep_alive) {
             if ($peer_ver eq "1.0") {
                 # from looking at Netscape's headers
                 push(@h2, "Keep-Alive: 300");
                 unshift(@connection, "Keep-Alive");
             }
         }
         else {
             push(@connection, "close") if $ver ge "1.1";
         }
     }

If I've understood LWP correctly, the fact that the socket was deposited 
into the connection cache at all means that the socket is eligible for 
re-use. It doesn't need further tests because it was put there by 
LWP::Protocol::http.pm on the basis of the http version and connection 
header in the response.

In Net/HTTP/Methods.pm, I believe that it is only necessary to focus on how 
to advertise that we would like the next response to keep the 
connection open, so the test, 'if ($peer_ver eq "1.0")', should actually 
be looking at the http version _we_ are using, not the version that the 
peer is using. In other words, the code above should be along the lines of:

     if ($self->keep_alive && $ver eq '1.0') {
         push(@h2, "Keep-Alive: 300");
         unshift(@connection, "Keep-Alive");
     }

Peter

Start in Net/HTTP/Methods.pm, focusing on the keep-alive header logic and how the local HTTP version is represented versus the peer version. Reproduce or inspect the HTTP/1.0 client and HTTP/1.1 server sequence described in the issue. Done means the client consistently advertises keep-alive when appropriate and no longer alternates between reusable and closed connections.