diff options
author | mbelshe@chromium.org <mbelshe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-08-25 06:23:00 +0000 |
---|---|---|
committer | mbelshe@chromium.org <mbelshe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-08-25 06:23:00 +0000 |
commit | 76a505b0776481b4bb2ec41969c8c9dcefd6e2a2 (patch) | |
tree | e7dd53d4e8f5897815df364e9f1248a87646eccf /net/socket | |
parent | 2b6ed3dc5fc23bdadbd5052f896648d2d62b9ca2 (diff) | |
download | chromium_src-76a505b0776481b4bb2ec41969c8c9dcefd6e2a2.zip chromium_src-76a505b0776481b4bb2ec41969c8c9dcefd6e2a2.tar.gz chromium_src-76a505b0776481b4bb2ec41969c8c9dcefd6e2a2.tar.bz2 |
Fix a crash where we are checking IsConnected(). If you look into the
implementation of IsConnected() on windows, you'll find that it can
synchronously check if the socket is connected. This makes it so that
a CHECK (or DCHECK) on this attribute is dependent on the peer. The peer
could send us a TCP-FIN at any time - even 1 nanosecond before our call
to IsConnected(). We can only use CHECK(!IsConnected()) to check before
we connect, we simply can't use it at runtime.
I searched the code and found 4 other instances of this broken check. Most
were DCHECKs, which is good, but I've removed them just the same.
Turns out our MockSockets have a mechanism for simulating this error, but it
wasn't tested nor plumbed through the MockSSLClientSocket. In the process
I added a couple of basic proxy test cases, which may be slightly redundant
with other test cases that are more advanced. But they seem like a good
idea to cover anyway. Added 6 tests in total.
BUG=53099
TEST=ProxyTunnelGetHangup, RecycleDeadSSLSocket
Review URL: http://codereview.chromium.org/3107036
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57295 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/socket')
-rw-r--r-- | net/socket/socket_test_util.cc | 4 | ||||
-rw-r--r-- | net/socket/socket_test_util.h | 1 | ||||
-rw-r--r-- | net/socket/socks5_client_socket.cc | 1 | ||||
-rw-r--r-- | net/socket/socks_client_socket.cc | 1 | ||||
-rw-r--r-- | net/socket/ssl_client_socket_pool.cc | 1 |
5 files changed, 5 insertions, 3 deletions
diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc index 069c293..5ffd1d4 100644 --- a/net/socket/socket_test_util.cc +++ b/net/socket/socket_test_util.cc @@ -493,6 +493,10 @@ void MockSSLClientSocket::Disconnect() { transport_->socket()->Disconnect(); } +bool MockSSLClientSocket::IsConnected() const { + return transport_->socket()->IsConnected(); +} + int MockSSLClientSocket::Read(net::IOBuffer* buf, int buf_len, net::CompletionCallback* callback) { return transport_->socket()->Read(buf, buf_len, callback); diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h index ad30353..533a18d 100644 --- a/net/socket/socket_test_util.h +++ b/net/socket/socket_test_util.h @@ -636,6 +636,7 @@ class MockSSLClientSocket : public MockClientSocket { // ClientSocket methods: virtual int Connect(net::CompletionCallback* callback); virtual void Disconnect(); + virtual bool IsConnected() const; // Socket methods: virtual int Read(net::IOBuffer* buf, int buf_len, diff --git a/net/socket/socks5_client_socket.cc b/net/socket/socks5_client_socket.cc index 997fb25..9c9ecbe 100644 --- a/net/socket/socks5_client_socket.cc +++ b/net/socket/socks5_client_socket.cc @@ -67,7 +67,6 @@ SOCKS5ClientSocket::~SOCKS5ClientSocket() { int SOCKS5ClientSocket::Connect(CompletionCallback* callback) { DCHECK(transport_.get()); DCHECK(transport_->socket()); - DCHECK(transport_->socket()->IsConnected()); DCHECK_EQ(STATE_NONE, next_state_); DCHECK(!user_callback_); diff --git a/net/socket/socks_client_socket.cc b/net/socket/socks_client_socket.cc index 32c4e3b..2000754 100644 --- a/net/socket/socks_client_socket.cc +++ b/net/socket/socks_client_socket.cc @@ -102,7 +102,6 @@ SOCKSClientSocket::~SOCKSClientSocket() { int SOCKSClientSocket::Connect(CompletionCallback* callback) { DCHECK(transport_.get()); DCHECK(transport_->socket()); - DCHECK(transport_->socket()->IsConnected()); DCHECK_EQ(STATE_NONE, next_state_); DCHECK(!user_callback_); diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc index 059354e..541792f 100644 --- a/net/socket/ssl_client_socket_pool.cc +++ b/net/socket/ssl_client_socket_pool.cc @@ -240,7 +240,6 @@ int SSLConnectJob::DoTunnelConnectComplete(int result) { if (result < 0) return result; - DCHECK(tunnel_socket->IsConnected()); next_state_ = STATE_SSL_CONNECT; return result; } |