On 08/12/2010 03:37 AM, Amos Jeffries wrote:
------------------------------------------------------------
revno: 10728
committer: Amos Jeffries<[email protected]>
branch nick: trunk
timestamp: Thu 2010-08-12 21:37:14 +1200
message:
Author: Stephen Thorne<[email protected]>
Bug 2936: NTLM-Authenticate 407 and Proxy-Connection: Close in same response.
Squid default from the days of HTTP/1.0 was to close connections unless
keep-alive was explicitly known. This changes the default to send
keep-alive unless we have a good reason to close.
modified:
src/client_side_reply.cc
=== modified file 'src/client_side_reply.cc'
--- a/src/client_side_reply.cc 2010-07-13 14:27:25 +0000
+++ b/src/client_side_reply.cc 2010-08-12 09:37:14 +0000
@@ -1383,6 +1383,9 @@
} else if (fdUsageHigh()&& !request->flags.must_keepalive) {
debugs(88, 3, "clientBuildReplyHeader: Not many unused FDs, can't
keep-alive");
request->flags.proxy_keepalive = 0;
+ } else if (request->http_ver.major == 1 && request->http_ver.minor == 1) {
+ debugs(88, 3, "clientBuildReplyHeader: Client is HTTP/1.1, send keep-alive,
no overriding reasons not to");
+ request->flags.proxy_keepalive = 1;
}
Persistent connections have been semi-broken since 3.0, but was the
above fix discussed somewhere? I think it contradicts the overall flow
of the persistency handling code in general and clientSetKeepaliveFlag
intent/documentation in particular. I do not know whether it introduces
more bugs, but I would not be surprised if it does because the
if-statements above the new code do not enumerate "all overriding reasons"!
To add insult to the injury, the commit message is also misleading
because, bugs notwithstanding, Squid did "send keep-alive unless we had
a good reason to close" even before this change.
Can we revert the above change, please?
You may want to test the attached fix instead. I do not know whether it
helps with Bug 2936 specifically, but it does fix a bug that smells
related to those issues because Bug 2936 test script uses HTTP/1.0 messages.
Thank you,
Alex.
Supply httpMsgIsPersistent() with request version it needs to
determine client connection persistency rather than some
irrelevant information.
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2010-08-07 14:22:54 +0000
+++ src/client_side.cc 2010-08-14 01:43:44 +0000
@@ -742,10 +742,7 @@
debugs(33, 3, "clientSetKeepaliveFlag: method = " <<
RequestMethodStr(request->method));
- /* We are HTTP/1.1 facing clients now*/
- HttpVersion http_ver(1,1);
-
- if (httpMsgIsPersistent(http_ver, req_hdr))
+ if (httpMsgIsPersistent(request->http_ver, req_hdr))
request->flags.proxy_keepalive = 1;
}
