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

net/http/http_proxy_client_socket_pool.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_proxy_client_socket_pool.h"
 
 #include <algorithm>
 
 #include "base/time.h"
 #include "base/values.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_proxy_client_socket.h"
+#include "net/http/http_response_headers.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/client_socket_pool_base.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/ssl_client_socket_pool.h"
 #include "net/socket/tcp_client_socket_pool.h"
+#include "net/spdy/spdy_http_stream.h"
 #include "net/spdy/spdy_proxy_client_socket.h"
 #include "net/spdy/spdy_session.h"
 #include "net/spdy/spdy_session_pool.h"
 #include "net/spdy/spdy_settings_storage.h"
 #include "net/spdy/spdy_stream.h"
 
 namespace net {
 
 HttpProxySocketParams::HttpProxySocketParams(
     const scoped_refptr<TCPSocketParams>& tcp_params,
@@ skipping 213 matching lines @@
     next_state_ = STATE_SPDY_PROXY_CREATE_STREAM;
   else
     next_state_ = STATE_HTTP_PROXY_CONNECT;
   return result;
 }
 
 void HttpProxyConnectJob::GetAdditionalErrorState(ClientSocketHandle * handle) {
   if (error_response_info_.cert_request_info) {
     handle->set_ssl_error_response_info(error_response_info_);
     handle->set_is_ssl_error(true);
+  } else if (error_response_info_.headers) {
+    handle->set_ssl_error_response_info(error_response_info_);
+    handle->set_pending_https_proxy_response_socket(
+        pending_https_proxy_response_socket_.release());
   }
 }
 
 int HttpProxyConnectJob::DoSpdyProxyCreateStream() {
   DCHECK(using_spdy_);
   DCHECK(params_->tunnel());
 
   HostPortProxyPair pair(params_->destination().host_port_pair(),
                          ProxyServer::Direct());
   SpdySessionPool* spdy_pool = params_->spdy_session_pool();
@@ skipping 45 matching lines @@
   // Add a HttpProxy connection on top of the tcp socket.
   transport_socket_.reset(
       new HttpProxyClientSocket(transport_socket_handle_.release(),
                                 params_->request_url(),
                                 params_->user_agent(),
                                 params_->endpoint(),
                                 proxy_server,
                                 params_->http_auth_cache(),
                                 params_->http_auth_handler_factory(),
                                 params_->tunnel(),
-                                using_spdy_));
+                                using_spdy_,
+                                params_->ssl_params() != NULL));
   return transport_socket_->Connect(&callback_);
 }
 
 int HttpProxyConnectJob::DoHttpProxyConnectComplete(int result) {
-  if (result == OK || result == ERR_PROXY_AUTH_REQUESTED)
+  if (result == OK || result == ERR_PROXY_AUTH_REQUESTED) {
       set_socket(transport_socket_.release());
+  } else if (result == ERR_HTTPS_PROXY_TUNNEL_CONNECTION_RESPONSE) {
+    error_response_info_ = *transport_socket_->GetConnectResponseInfo();

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

Urg - it just occurred to me this connection will not be accounted for in the
socket limits.  In order to do that, you'd need to make this a recoverable error
and then in the SSlClientSocketPool, push it into a ClientSocketHandle with the
appropriate pool so that when it was released the right pool would get it back.

At least in the non-spdy case, that means you can't create the stream until this
point.  I don't have a good enough model of the spdy pool to think about it in
this context.

Would "trafficing" in ClientSocketHandles from this point up make this workable?

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

I'm still not sure that I understand the technical details here.  I think that
perhaps you're saying that if a socket is created as part of
ClientSocketHandle.Init(), it either needs to be destroyed in the process, or
must be saved via the Handle's set_socket method?  Is that right?  If a socket
(ssl, or proxy) is created on top of an existing socket, I guess the idea is
that it is created on top of an existing SocketHandle (not just Socket) so that
the Handle can manage the interaction with the pool.

Does thats sound right?  If so, I think you might be right... Trafficing in
Handles might be the way to go.  I'll take a look...

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

I think you've got this right.  A ClientSocketHandle has a reference to a
particular pool and a slot for a socket.  Init tries to fill that slot.  Sockets
that layer will internally have a ClientSocketHandle (usually called
transport_socket_) to manage that underlying pool allocation.  If a pool were to
return (by any means) a ClientSocket without putting it into a
ClientSocketHandle, that socket wouldn't be counted against the connection limit
and there would be no link to the pool that created it.

+    pending_https_proxy_response_socket_.reset(transport_socket_.release());
+    DCHECK(!error_response_info_.cert_request_info.get());
+  }
 
   return result;
 }
 
 HttpProxyClientSocketPool::
 HttpProxyConnectJobFactory::HttpProxyConnectJobFactory(
     TCPClientSocketPool* tcp_pool,
     SSLClientSocketPool* ssl_pool,
     HostResolver* host_resolver,
     NetLog* net_log)
@@ skipping 110 matching lines @@
       list->Append(ssl_pool_->GetInfoAsValue("ssl_socket_pool",
                                              "ssl_socket_pool",
                                              true));
     }
     dict->Set("nested_pools", list);
   }
   return dict;
 }
 
 }  // namespace net