Allow a non-200 (or non-407) response for a CONNECT request from an HTTPS pro...

net/http/http_proxy_client_socket.cc

Index: net/http/http_proxy_client_socket.cc
===================================================================
--- net/http/http_proxy_client_socket.cc	(revision 68026)
+++ net/http/http_proxy_client_socket.cc	(working copy)
@@ -12,6 +12,7 @@
 #include "net/base/io_buffer.h"
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
+#include "net/http/http_basic_stream.h"
 #include "net/http/http_net_log_params.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_proxy_utils.h"
@@ -31,7 +32,8 @@
     HttpAuthCache* http_auth_cache,
     HttpAuthHandlerFactory* http_auth_handler_factory,
     bool tunnel,
-    bool using_spdy)
+    bool using_spdy,
+    bool is_https_proxy)
     : ALLOW_THIS_IN_INITIALIZER_LIST(
           io_callback_(this, &HttpProxyClientSocket::OnIOComplete)),
       next_state_(STATE_NONE),
@@ -46,6 +48,7 @@
           : NULL),
       tunnel_(tunnel),
       using_spdy_(using_spdy),
+      is_https_proxy_(is_https_proxy),
       net_log_(transport_socket->socket()->NetLog()) {
   // Synthesize the bits of a request that we actually use.
   request_.url = request_url;
@@ -59,6 +62,12 @@
   Disconnect();
 }
 
+HttpStream* HttpProxyClientSocket::GetConnectResponseStream() {
+  return new HttpBasicStream(http_stream_parser_.release(),
+                             transport_.release());
+}
+
+
 int HttpProxyClientSocket::Connect(CompletionCallback* callback) {
   DCHECK(transport_.get());
   DCHECK(transport_->socket());
@@ -145,7 +154,8 @@
 }
 
 void HttpProxyClientSocket::Disconnect() {
-  transport_->socket()->Disconnect();
+  if (transport_.get())
+    transport_->socket()->Disconnect();
 
   // Reset other states to make sure they aren't mistakenly used later.
   // These are the states initialized by Connect().
@@ -405,6 +415,8 @@
       return HandleAuthChallenge();
 
     default:
+      if (is_https_proxy_)
+        return ERR_HTTPS_PROXY_TUNNEL_CONNECTION_RESPONSE;

[vandebo (ex-Chrome), 2010/12/14 00:30:23]

Should you call response_.headers.RemoveHeader("KeepAlive") here to ensure that
the socket is not reused?

[Ryan Hamilton, 2010/12/15 20:14:17]

I'm not sure I follow.  Are you concerned about the SSL & TCP sockets (to the
proxy) being reused, or the Http|SpdyProxyClientSockets?  In the case of the
former, we send a valid HTTP request (CONNECT) to the proxy, and received a
valid (though non 200) HTTP response.  As such, it seems perfectly fine to reuse
the connection to the proxy.  Do you agree?

In the case of the Spdy|HttpProxyClientSocket, you're right that we don't want
them to be reused.  Of course, since the have released their transport sockets,
the're definitely not reusable :>  Does that sound right?

[vandebo (ex-Chrome), 2010/12/15 22:52:38]

You're right about this.  I wasn't thinking that this was a recoverable
condition, but of course it is.  The proxy connection can be reused for a
different CONNECT request.  The SSL socket is never set up, so there's no way
for it to get reused.

       // For all other status codes, we conservatively fail the CONNECT
       // request.
       // We lose something by doing this.  We have seen proxy 403, 404, and