Prevent the infinite loop inside SSLClientSocketNSS::OnSendComplete.

net/socket/ssl_server_socket_unittest.cc

 // Copyright (c) 2012 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.
 
 // This test suite uses SSLClientSocket to test the implementation of
 // SSLServerSocket. In order to establish connections between the sockets
 // we need two additional classes:
 // 1. FakeSocket
 //    Connects SSL socket to FakeDataChannel. This class is just a stub.
 //
@@ skipping 36 matching lines @@
 #include "testing/platform_test.h"
 
 namespace net {
 
 namespace {
 
 class FakeDataChannel {
  public:
   FakeDataChannel()
       : read_buf_len_(0),
-        ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
+        ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
+        closed_(false),
+        write_called_after_close_(false) {
   }
 
   int Read(IOBuffer* buf, int buf_len, const CompletionCallback& callback) {
+    if (closed_)
+      return 0;
     if (data_.empty()) {
       read_callback_ = callback;
       read_buf_ = buf;
       read_buf_len_ = buf_len;
       return net::ERR_IO_PENDING;
     }
     return PropogateData(buf, buf_len);
   }
 
   int Write(IOBuffer* buf, int buf_len, const CompletionCallback& callback) {
+    if (closed_) {
+      if (write_called_after_close_)
+        return net::ERR_CONNECTION_RESET;
+      write_called_after_close_ = true;
+      write_callback_ = callback;
+      MessageLoop::current()->PostTask(
+          FROM_HERE, base::Bind(&FakeDataChannel::DoWriteCallback,
+                                weak_factory_.GetWeakPtr()));
+      return net::ERR_IO_PENDING;
+    }
     data_.push(new net::DrainableIOBuffer(buf, buf_len));
     MessageLoop::current()->PostTask(
         FROM_HERE, base::Bind(&FakeDataChannel::DoReadCallback,
                               weak_factory_.GetWeakPtr()));
     return buf_len;
   }
 
+  // Closes the FakeDataChannel. After Close() is called, Read() returns 0,
+  // indicating EOF, and Write() fails with ERR_CONNECTION_RESET. Note that
+  // after the FakeDataChannel is closed, the first Write() call completes
+  // asynchronously, which is necessary to reproduce bug 127822.
+  void Close() {
+    closed_ = true;
+  }
+
  private:
   void DoReadCallback() {
     if (read_callback_.is_null() || data_.empty())
       return;
 
     int copied = PropogateData(read_buf_, read_buf_len_);
     CompletionCallback callback = read_callback_;
     read_callback_.Reset();
     read_buf_ = NULL;
     read_buf_len_ = 0;
     callback.Run(copied);
   }
 
+  void DoWriteCallback() {
+    if (write_callback_.is_null())
+      return;
+
+    CompletionCallback callback = write_callback_;
+    write_callback_.Reset();
+    callback.Run(net::ERR_CONNECTION_RESET);
+  }
+
   int PropogateData(scoped_refptr<net::IOBuffer> read_buf, int read_buf_len) {
     scoped_refptr<net::DrainableIOBuffer> buf = data_.front();
     int copied = std::min(buf->BytesRemaining(), read_buf_len);
     memcpy(read_buf->data(), buf->data(), copied);
     buf->DidConsume(copied);
 
     if (!buf->BytesRemaining())
       data_.pop();
     return copied;
   }
 
   CompletionCallback read_callback_;
   scoped_refptr<net::IOBuffer> read_buf_;
   int read_buf_len_;
 
+  CompletionCallback write_callback_;
+
   std::queue<scoped_refptr<net::DrainableIOBuffer> > data_;
 
   base::WeakPtrFactory<FakeDataChannel> weak_factory_;
 
+  // True if Close() has been called.
+  bool closed_;
+
+  // Controls the completion of Write() after the FakeDataChannel is closed.
+  // After the FakeDataChannel is closed, the first Write() call completes
+  // asynchronously.
+  bool write_called_after_close_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeDataChannel);
 };
 
 class FakeSocket : public StreamSocket {
  public:
   FakeSocket(FakeDataChannel* incoming_channel,
              FakeDataChannel* outgoing_channel)
       : incoming_(incoming_channel),
         outgoing_(outgoing_channel) {
   }
@@ skipping 20 matching lines @@
   }
 
   virtual bool SetSendBufferSize(int32 size) OVERRIDE {
     return true;
   }
 
   virtual int Connect(const CompletionCallback& callback) OVERRIDE {
     return net::OK;
   }
 
-  virtual void Disconnect() OVERRIDE {}
+  virtual void Disconnect() OVERRIDE {
+    incoming_->Close();
+    outgoing_->Close();
+  }
 
   virtual bool IsConnected() const OVERRIDE {
     return true;
   }
 
   virtual bool IsConnectedAndIdle() const OVERRIDE {
     return true;
   }
 
   virtual int GetPeerAddress(AddressList* address) const OVERRIDE {
@@ skipping 261 matching lines @@
     EXPECT_TRUE(server_ret > 0 || server_ret == net::ERR_IO_PENDING);
     server_ret = read_callback.GetResult(server_ret);
     ASSERT_GT(server_ret, 0);
     read_buf->DidConsume(server_ret);
   }
   EXPECT_EQ(write_buf->size(), read_buf->BytesConsumed());
   read_buf->SetOffset(0);
   EXPECT_EQ(0, memcmp(write_buf->data(), read_buf->data(), write_buf->size()));
 }
 
+// A regression test for bug 127822 (http://crbug.com/127822).
+// If the server closes the connection after the handshake is finished,
+// the client's Write() call should not cause an infinite loop.
+TEST_F(SSLServerSocketTest, WriteAfterPeerClose) {

[Ryan Sleevi, 2012/05/16 02:16:46]

Just to make sure - this is really testing the behaviour of the SSLClientSocket,
but it's here as an SSLServerSocketTest because the ability to avoid network IO
(the FakeDataChannel) while still having packets successfully encrypt/decrypt
with SSL (which is why we can't use the SocketDataProvider)

I agree with the approach - avoiding the test server = great - but am just
concerned about the naming/documentation, since it's not really testing the
server impl.

[wtc, 2012/05/16 03:04:29]

Yes.  I added a comment to clarify this.

+  Initialize();
+
+  TestCompletionCallback connect_callback;
+  TestCompletionCallback handshake_callback;
+
+  // Establish connection.
+  int client_ret = client_socket_->Connect(connect_callback.callback());
+  ASSERT_TRUE(client_ret == net::OK || client_ret == net::ERR_IO_PENDING);
+
+  int server_ret = server_socket_->Handshake(handshake_callback.callback());
+  ASSERT_TRUE(server_ret == net::OK || server_ret == net::ERR_IO_PENDING);
+
+  client_ret = connect_callback.GetResult(client_ret);
+  ASSERT_EQ(net::OK, client_ret);
+  server_ret = handshake_callback.GetResult(server_ret);
+  ASSERT_EQ(net::OK, server_ret);
+
+  scoped_refptr<net::StringIOBuffer> write_buf =
+      new net::StringIOBuffer("testing123");
+
+  // The server closes the connection. The server needs to write some
+  // data first so that the client's Read() calls from the transport
+  // socket won't return ERR_IO_PENDING.  This ensures that the client
+  // will call Read() on the transport socket again.
+  TestCompletionCallback write_callback;
+
+  server_ret = server_socket_->Write(write_buf, write_buf->size(),
+                                     write_callback.callback());
+  EXPECT_TRUE(server_ret > 0 || server_ret == net::ERR_IO_PENDING);
+
+  server_ret = write_callback.GetResult(server_ret);
+  EXPECT_GT(server_ret, 0);
+
+  server_socket_->Disconnect();
+
+  // The client writes some data. This should not cause an infinite loop.
+  client_ret = client_socket_->Write(write_buf, write_buf->size(),
+                                     write_callback.callback());
+  EXPECT_TRUE(client_ret > 0 || client_ret == net::ERR_IO_PENDING);
+
+  client_ret = write_callback.GetResult(client_ret);
+  EXPECT_GT(client_ret, 0);
+
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE, MessageLoop::QuitClosure(),
+      base::TimeDelta::FromMilliseconds(10));
+  MessageLoop::current()->Run();
+}
+
 // This test executes ExportKeyingMaterial() on the client and server sockets,
 // after connecting them, and verifies that the results match.
 // This test will fail if False Start is enabled (see crbug.com/90208).
 TEST_F(SSLServerSocketTest, ExportKeyingMaterial) {
   Initialize();
 
   TestCompletionCallback connect_callback;
   TestCompletionCallback handshake_callback;
 
   int client_ret = client_socket_->Connect(connect_callback.callback());
@@ skipping 29 matching lines @@
   unsigned char client_bad[kKeyingMaterialSize];
   rv = client_socket_->ExportKeyingMaterial(kKeyingLabelBad,
                                             false, kKeyingContext,
                                             client_bad, sizeof(client_bad));
   ASSERT_EQ(rv, net::OK);
   EXPECT_NE(0, memcmp(server_out, client_bad, sizeof(server_out)));
 }
 #endif
 
 }  // namespace net