bug 87047. Handle keep-alives from http/1.1 servers properly. Patch by me

and darin, r=gagan, sr=dougt

netwerk/protocol/http/src/nsHttpChannel.cpp

@@ -168,7 +168,11 @@ nsHttpChannel::Init(nsIURI *uri,
     rv = mRequestHead.SetHeader(nsHttp::Host, hostLine);
     if (NS_FAILED(rv)) return rv;
 
-    rv = nsHttpHandler::get()->AddStandardRequestHeaders(&mRequestHead.Headers(), caps);
+    PRBool useProxy = (proxyHost && !PL_strcmp(proxyType, "http"));
+
+    rv = nsHttpHandler::get()->AddStandardRequestHeaders(&mRequestHead.Headers(),
+                                                         caps,
+                                                         useProxy);
     if (NS_FAILED(rv)) return rv;
 
     // check to see if authorization headers should be included

netwerk/protocol/http/src/nsHttpConnection.cpp

@@ -39,6 +39,9 @@
 
 static NS_DEFINE_CID(kSocketTransportServiceCID, NS_SOCKETTRANSPORTSERVICE_CID);
 
+// Default timeout in seconds, if the server doesn't give us one (eg http/1.1)
+#define HTTP_DEFAULT_TIMEOUT 10
+
 //-----------------------------------------------------------------------------
 // nsHttpConnection <public>
 //-----------------------------------------------------------------------------
@@ -146,32 +149,50 @@ nsHttpConnection::OnHeadersAvailable(nsHttpTransaction *trans, PRBool *reset)
     const char *val = trans->ResponseHead()->PeekHeader(nsHttp::Connection);
     if (!val)
         val = trans->ResponseHead()->PeekHeader(nsHttp::Proxy_Connection);
-    if (val) {
-        // be pesimistic
-        mKeepAlive = PR_FALSE;
 
-        if (PL_strcasecmp(val, "keep-alive") == 0) {
+    if ((trans->ResponseHead()->Version() < NS_HTTP_VERSION_1_1) ||
+        (nsHttpHandler::get()->DefaultVersion() < NS_HTTP_VERSION_1_1)) {
+        // HTTP/1.0 connections are by default NOT persistent
+        if (val && !PL_strcasecmp(val, "keep-alive"))
             mKeepAlive = PR_TRUE;
+        else
+            mKeepAlive = PR_FALSE;
+    }
+    else {
+        // HTTP/1.1 connections are by default persistent
+        if (val && !PL_strcasecmp(val, "close")) 
+            mKeepAlive = PR_FALSE;
+        else
+            mKeepAlive = PR_TRUE;
+    }
 
-            val = trans->ResponseHead()->PeekHeader(nsHttp::Keep_Alive);
+    // if this connection is persistent, then the server may send a "Keep-Alive"
+    // header specifying the maximum number of times the connection can be
+    // reused as well as the maximum amount of time the connection can be idle
+    // before the server will close it.  If this header is not present, then we
+    // pick a suitably large number. Technically, any number > 0 will do, since
+    // we reset this each time, and with HTTP/1.1 connections continue until we
+    // get a {Proxy-,}Connection: close header. Don't just use 1 though, because
+    // of pipelining
+    if (mKeepAlive) {
+        val = trans->ResponseHead()->PeekHeader(nsHttp::Keep_Alive);
 
-            LOG(("val = [%s]\n", val));
+        LOG(("val = [%s]\n", val));
 
-            const char *cp = PL_strcasestr(val, "max=");
-            if (cp)
-                mMaxReuseCount = (PRUint32) atoi(cp + 4);
-            else
-                mMaxReuseCount = 100;
+        const char *cp = PL_strcasestr(val, "max=");
+        if (cp)
+            mMaxReuseCount = (PRUint32) atoi(cp + 4);
+        else
+            mMaxReuseCount = 100;
 
-            cp = PL_strcasestr(val, "timeout=");
-            if (cp)
-                mIdleTimeout = (PRUint32) atoi(cp + 8);
-            else
-                mIdleTimeout = 10;
+        cp = PL_strcasestr(val, "timeout=");
+        if (cp)
+            mIdleTimeout = (PRUint32) atoi(cp + 8);
+        else
+            mIdleTimeout = HTTP_DEFAULT_TIMEOUT;
         
-            LOG(("Connection can be reused [this=%x max-reuse=%u "
-                 "keep-alive-timeout=%u\n", this, mMaxReuseCount, mIdleTimeout));
-        }
+        LOG(("Connection can be reused [this=%x max-reuse=%u "
+             "keep-alive-timeout=%u\n", this, mMaxReuseCount, mIdleTimeout));
     }
 
     // if we're doing an SSL proxy connect, then we need to check whether or not

netwerk/protocol/http/src/nsHttpHandler.cpp

@@ -24,6 +24,7 @@
  *   Christopher Blizzard <blizzard@mozilla.org>
  *   Adrian Havill <havill@redhat.com>
  *   Gervase Markham <gerv@gerv.net>
+ *   Bradley Baetz <bbaetz@netscape.com>
  */
 
 #include "nsHttp.h"
@@ -242,36 +243,44 @@ nsHttpHandler::Init()
 
 nsresult
 nsHttpHandler::AddStandardRequestHeaders(nsHttpHeaderArray *request,
-                                         PRUint32 caps)
+                                         PRUint32 caps,
+                                         PRBool useProxy)
 {
     nsresult rv;
 
     LOG(("nsHttpHandler::AddStandardRequestHeaders\n"));
 
-    // Add the User-Agent header:
+    // Add the "User-Agent" header
     rv = request->SetHeader(nsHttp::User_Agent, UserAgent());
     if (NS_FAILED(rv)) return rv;
 
     // MIME based content negotiation lives!
-    // Add the Accept header:
+    // Add the "Accept" header
     rv = request->SetHeader(nsHttp::Accept, mAccept.get());
     if (NS_FAILED(rv)) return rv;
 
-    // Add the Accept-Language header:
+    // Add the "Accept-Language" header
     rv = request->SetHeader(nsHttp::Accept_Language, mAcceptLanguages.get());
     if (NS_FAILED(rv)) return rv;
 
-    // Add the Accept-Encoding header:
+    // Add the "Accept-Encoding" header
     rv = request->SetHeader(nsHttp::Accept_Encoding, mAcceptEncodings.get());
     if (NS_FAILED(rv)) return rv;
 
-    // Add the Accept-Charset header:
+    // Add the "Accept-Charset" header
     rv = request->SetHeader(nsHttp::Accept_Charset, mAcceptCharsets.get());
     if (NS_FAILED(rv)) return rv;
 
-    // Add the Connection header:
-    const char *connectionType = "close";
-    if (caps && ALLOW_KEEPALIVE) {
+    // RFC2616 section 19.6.2 states that the "Connection: keep-alive"
+    // and "Keep-alive" request headers should not be sent by HTTP/1.1
+    // user-agents.  Otherwise, problems with proxy servers (especially
+    // transparent proxies) can result.
+    // However, we need to send something so that we can use keepalive
+    // with HTTP/1.0 servers/proxies. We use Proxy-Connection: when we're
+    // talking to an http proxy, and Connection: otherwise
+    
+    const char* connectionType = "close";
+    if (caps & ALLOW_KEEPALIVE) {
         char buf[32];
         
         PR_snprintf(buf, sizeof(buf), "%d", mIdleTimeout);
@@ -281,7 +290,9 @@ nsHttpHandler::AddStandardRequestHeaders(nsHttpHeaderArray *request,
         
         connectionType = "keep-alive";
     }
-    return request->SetHeader(nsHttp::Connection, connectionType);
+
+    const nsHttpAtom& connAtom = useProxy ? nsHttp::Proxy_Connection : nsHttp::Connection;
+    return request->SetHeader(connAtom, connectionType);
 }
 
 PRBool

netwerk/protocol/http/src/nsHttpHandler.h

@@ -81,7 +81,8 @@ public:
 
     nsresult Init();
     nsresult AddStandardRequestHeaders(nsHttpHeaderArray *,
-                                       PRUint32 capabilities);
+                                       PRUint32 capabilities,
+                                       PRBool useProxy);
     PRBool   IsAcceptableEncoding(const char *encoding);
 
     const char   *UserAgent();

netwerk/protocol/http/src/nsHttpResponseHead.cpp

@@ -390,6 +390,7 @@ nsHttpResponseHead::UpdateHeaders(nsHttpHeaderArray &headers)
 
         // Ignore any hop-by-hop headers...
         if (header == nsHttp::Connection          ||
+            header == nsHttp::Proxy_Connection    ||
             header == nsHttp::Keep_Alive          ||
             header == nsHttp::Proxy_Authenticate  ||
             header == nsHttp::Proxy_Authorization || // not a response header!