Avoid abandoned client connections after url_rewriter redirects CONNECT.

clientProcessRequest() assumes that a CONNECT request is always tunneled and
sets flags.readMore to false. However, if url_rewriter redirects the
CONNECTing user, Squid responds with a redirect message and does not tunnel.
In that case, we do want to read more. Otherwise, keepaliveNextRequest() will
declare the connection abandoned and the connection descriptor will "leak"
until the connection lifetime expires, even if the client disconnects
immediately after receiving the redirect response.

The fix delays resets flags.readMore back to true if we call end up calling
httpStart() instead of tunnelStart().

My earlier solution was to keep flags.readMore as true until tunnelStart(),
but that breaks SslBump because Squid starts reading the [encrypted] client
connection (trying to find the next HTTP request there) while obtaining server
certificate. This happens because ConnStateData::clientAfterReadingRequests()
calls readSomeData() if flags.readMore is true, even if
context->mayUseConnection() is also true.

The current solution is better because, besides working, it keeps
flags.readMore and context->mayUseConnection() consistent.

There should be no effect on CONNECT authentication (another case where
CONNECT is not tunneled) because the changes should not touch authentication
code paths, but that assertion has not been tested.

=== modified file 'src/client_side.cc'
--- src/client_side.cc	2013-01-18 05:57:22 +0000
+++ src/client_side.cc	2013-01-18 21:47:00 +0000
@@ -2762,40 +2762,45 @@
         const bool supportedExpect = (expect.caseCmp("100-continue") == 0);
         if (!supportedExpect) {
             clientStreamNode *node = context->getClientReplyContext();
             clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
             assert (repContext);
             conn->quitAfterError(request);
             repContext->setReplyToError(ERR_INVALID_REQ, HTTP_EXPECTATION_FAILED, request->method, http->uri,
                                         conn->clientConnection->remote, request, NULL, NULL);
             assert(context->http->out.offset == 0);
             context->pullData();
             goto finish;
         }
     }
 
     http->request = HTTPMSGLOCK(request);
     clientSetKeepaliveFlag(http);
 
     // Let tunneling code be fully responsible for CONNECT requests
     if (http->request->method == Http::METHOD_CONNECT) {
         context->mayUseConnection(true);
+        // "may" in "mayUse" is meaningful here: no tunnel if Squid redirects,
+        // needs to authenticate, or responds with an error. For now, pause so
+        // that we do not read and try to parse tunneled and/or encrypted data.
+        // We resume reading if we get to ClientHttpRequest::httpStart() or 
+        // ConnStateData::switchToHttps().
         conn->flags.readMore = false;
     }
 
 #if USE_SSL
     if (conn->switchedToHttps() && conn->serveDelayedError(context))
         goto finish;
 #endif
 
     /* Do we expect a request-body? */
     expectBody = chunked || request->content_length > 0;
     if (!context->mayUseConnection() && expectBody) {
         request->body_pipe = conn->expectRequestBody(
                                  chunked ? -1 : request->content_length);
 
         // consume header early so that body pipe gets just the body
         connNoteUseOfBuffer(conn, http->req_sz);
         notedUseOfBuffer = true;
 
         /* Is it too large? */
         if (!chunked && // if chunked, we will check as we accumulate
@@ -4280,40 +4285,41 @@
 
 bool
 ConnStateData::transparent() const
 {
     return clientConnection != NULL && (clientConnection->flags & (COMM_TRANSPARENT|COMM_INTERCEPTION));
 }
 
 bool
 ConnStateData::reading() const
 {
     return reader != NULL;
 }
 
 void
 ConnStateData::stopReading()
 {
     if (reading()) {
         comm_read_cancel(clientConnection->fd, reader);
         reader = NULL;
     }
+    flags.readMore = false;
 }
 
 BodyPipe::Pointer
 ConnStateData::expectRequestBody(int64_t size)
 {
     bodyPipe = new BodyPipe(this);
     if (size >= 0)
         bodyPipe->setBodySize(size);
     else
         startDechunkingRequest();
     return bodyPipe;
 }
 
 int64_t
 ConnStateData::mayNeedToReadMoreBody() const
 {
     if (!bodyPipe)
         return 0; // request without a body or read/produced all body bytes
 
     if (!bodyPipe->bodySizeKnown())

=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc	2012-12-28 15:23:21 +0000
+++ src/client_side_request.cc	2013-01-18 21:34:47 +0000
@@ -1432,40 +1432,43 @@
             sslBumpStart();
             return;
         }
 #endif
         logType = LOG_TCP_MISS;
         getConn()->stopReading(); // tunnels read for themselves
         tunnelStart(this, &out.size, &al->http.code);
         return;
     }
 
     httpStart();
 }
 
 void
 ClientHttpRequest::httpStart()
 {
     PROF_start(httpStart);
     logType = LOG_TAG_NONE;
     debugs(85, 4, "ClientHttpRequest::httpStart: " << Format::log_tags[logType] << " for '" << uri << "'");
 
+    // now we know that we are not tunneling or bumping CONNECT
+    getConn()->flags.readMore = true; // resume (or continue) reading
+
     /* no one should have touched this */
     assert(out.offset == 0);
     /* Use the Stream Luke */
     clientStreamNode *node = (clientStreamNode *)client_stream.tail->data;
     clientStreamRead(node, this, node->readBuffer);
     PROF_stop(httpStart);
 }
 
 #if USE_SSL
 
 void
 ClientHttpRequest::sslBumpNeed(Ssl::BumpMode mode)
 {
     debugs(83, 3, HERE << "sslBump required: "<< Ssl::bumpMode(mode));
     sslBumpNeed_ = mode;
 }
 
 // called when comm_write has completed
 static void
 SslBumpEstablish(const Comm::ConnectionPointer &, char *, size_t, comm_err_t errflag, int, void *data)


