doesn't honour the "Connection: Close" HTTP client header #3010

Closed
michaelrsweet opened this Issue Nov 16, 2008 · 3 comments

Comments

Projects
None yet
2 participants
Collaborator

michaelrsweet commented Nov 16, 2008

Version: 1.3.9
CUPS.org User: martin.pitt.canonical

From http://bugs.debian.org/504727:

Below is a wireshark capture of a client (nagios) doing an HTTP
request to cups. The client uses HTTP 1.1 and includes the
"Connection: close" header meaning that he wants that request to
be the last one. cups replies with a "Connection: keep-alive"
header and doesn't close its end of the connection, which is in
violation of the HTTP 1.1 RFC.

In the case of nagios, it has the effect of nagios timing out
after 10 seconds waiting for the connection to close, and thus
declares the cups service dead.

This is on the loopback interface, so no proxy comes into play.

Request from nagios:

[...]
Frame 4 (180 bytes on wire, 180 bytes captured)
Arrival Time: Nov 6, 2008 16:25:37.127285000
[...]
Hypertext Transfer Protocol
GET / HTTP/1.1\r\n
Request Method: GET
Request URI: /
Request Version: HTTP/1.1
Host: 127.0.0.1:631\r\n
User-Agent: check_http/v1991 (nagios-plugins 1.4.12)\r\n
Connection: close\r\n
\r\n
[...]

answer from cups:

Frame 6 (323 bytes on wire, 323 bytes captured)
Arrival Time: Nov 6, 2008 16:25:37.127445000
[Time delta from previous captured frame: 0.000151000 seconds]
[Time delta from previous displayed frame: 0.000151000 seconds]
[Time since reference or first frame: 0.000405000 seconds]
[...]
Hypertext Transfer Protocol
HTTP/1.1 200 OK\r\n
Request Version: HTTP/1.1
Response Code: 200
Date: Thu, 06 Nov 2008 16:25:37 GMT\r\n
Server: CUPS/1.3\r\n
Connection: Keep-Alive\r\n
Keep-Alive: timeout=60\r\n
Content-Language: en_GB\r\n
Content-Type: text/html; charset=utf-8\r\n
Last-Modified: Sat, 11 Oct 2008 10:58:14 GMT\r\n
Content-Length: 5242
\r\n

[...]

After 10 seconds, nagios times out.

Frame 10 (68 bytes on wire, 68 bytes captured)
Arrival Time: Nov 6, 2008 16:25:47.127182000
[Time delta from previous captured frame: 9.999571000 seconds]
[Time delta from previous displayed frame: 9.999571000 seconds]
[Time since reference or first frame: 10.000142000 seconds]
[...]
Transmission Control Protocol, Src Port: 41750 (41750), Dst Port: ipp (631), Seq: 113, Ack: 5498, Len: 0
Source port: 41750 (41750)
Destination port: ipp (631)
Sequence number: 113 (relative sequence number)
Acknowledgement number: 5498 (relative ack number)
Header length: 32 bytes
Flags: 0x11 (FIN, ACK)

[...]

nagios log:

CURRENT SERVICE STATE: localhost;CUPS;CRITICAL;HARD;4;CRITICAL -
Socket timeout after 10 seconds

The "CUPS" nagios service does a:
/usr/lib/nagios/plugins/check_http -H localhost -p 631

Which does:

socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 3
connect(3, {sa_family=AF_INET, sin_port=htons(631),
sin_addr=inet_addr("127.0.0.1")}, 16) = 0
send(3, "GET / HTTP/1.1\r\nHost: localhost:6"..., 112, 0) = 112
read(3, "HTTP/1.1 200 OK\r\nDate: Thu, 06 No"..., 8191) = 5497
read(3,

That second read is waiting for end of file that never comes
(well would come after cups keep-alive timeout: 1 minute) and
so times out.

Collaborator

michaelrsweet commented Nov 17, 2008

CUPS.org User: mike

Fixed in Subversion repository.

Collaborator

michaelrsweet commented Nov 17, 2008

"str3010.patch":

Index: scheduler/client.c
===================================================================
--- scheduler/client.c  (revision 8133)
+++ scheduler/client.c  (working copy)
@@ -1136,9 +1136,11 @@

     cupsdAuthorize(con);

-    if (!strncmp(con->http.fields[HTTP_FIELD_CONNECTION], "Keep-Alive", 10) &&
+    if (!strncasecmp(con->http.fields[HTTP_FIELD_CONNECTION], "Keep-Alive", 10) &&
         KeepAlive)
       con->http.keep_alive = HTTP_KEEPALIVE_ON;
+    else if (!strncasecmp(con->http.fields[HTTP_FIELD_CONNECTION], "close", 5))
+      con->http.keep_alive = HTTP_KEEPALIVE_OFF;

     if (!con->http.fields[HTTP_FIELD_HOST][0] &&
         con->http.version >= HTTP_1_1)

michaelrsweet added this to the Stable milestone Mar 17, 2016

jsmeix commented Apr 14, 2016

@michaelrsweet
many thanks for making the embedded diff in #3010 (comment) readable!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment