Defines SSLServerSocket and implements SSLServerSocketNSS

net/socket/ssl_server_socket_unittest.cc

Index: net/socket/ssl_server_socket_unittest.cc
diff --git a/net/socket/ssl_server_socket_unittest.cc b/net/socket/ssl_server_socket_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..10a541109a5ab9425c00efc6ed08e169325c79aa
--- /dev/null
+++ b/net/socket/ssl_server_socket_unittest.cc
@@ -0,0 +1,331 @@
+// 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.
+
+// 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. FakeClientSocket
+//    Connects SSL socket to FakeDataChannel. This class is just a stub.
+//
+// 2. FakeDataChannel
+//    Implements the actual exchange of data between two FakeClientSockets.
+//
+// Implementations of these two classes are included in this file.
+
+#include <queue>
+
+#include "base/crypto/rsa_private_key.h"
+#include "net/base/address_list.h"
+#include "net/base/host_port_pair.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/net_log.h"
+#include "net/base/ssl_config_service.h"
+#include "net/base/x509_certificate.h"
+#include "net/socket/client_socket.h"
+#include "net/socket/client_socket_factory.h"
+#include "net/socket/socket_test_util.h"
+#include "net/socket/ssl_client_socket.h"
+#include "net/socket/ssl_server_socket.h"

[wtc, 2010/12/17 00:16:26]

Nit: list "net/socket/ssl_server_socket.h" first.
This is suggested by an example in the Style Guide:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Names_and_Orde...

[Alpha Left Google, 2010/12/17 08:30:43]

Done.

+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+namespace net {
+
+namespace {
+
+class FakeDataChannel {
+ public:
+  FakeDataChannel() : read_callback_(NULL), read_buf_len_(0) {
+  }
+
+  virtual int Read(IOBuffer* buf, int buf_len,
+                   CompletionCallback* callback) {
+    if (data_.empty()) {
+      read_callback_ = callback;
+      read_buf_ = buf;
+      read_buf_len_ = buf_len;
+      return net::ERR_IO_PENDING;
+    }
+    return PropogateData(buf, buf_len);
+  }
+
+  virtual int Write(IOBuffer* buf, int buf_len,
+                    CompletionCallback* callback) {
+    data_.push(new net::DrainableIOBuffer(buf, buf_len));
+    DoReadCallback();
+    return buf_len;
+  }
+
+ private:
+  void DoReadCallback() {
+    if (!read_callback_)
+      return;
+
+    int copied = PropogateData(read_buf_, read_buf_len_);
+    net::CompletionCallback* callback = read_callback_;
+    read_callback_ = NULL;
+    read_buf_ = NULL;
+    read_buf_len_ = 0;
+    callback->Run(copied);
+  }
+
+  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;
+  }
+
+  net::CompletionCallback* read_callback_;
+  scoped_refptr<net::IOBuffer> read_buf_;
+  int read_buf_len_;
+
+  std::queue<scoped_refptr<net::DrainableIOBuffer> > data_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeDataChannel);
+};
+
+class FakeClientSocket : public ClientSocket {
+ public:
+  FakeClientSocket(FakeDataChannel* incoming_channel,
+                   FakeDataChannel* outgoing_channel)
+      : incoming_(incoming_channel),
+        outgoing_(outgoing_channel) {
+  }
+
+  virtual ~FakeClientSocket() {
+
+  }
+
+  virtual int Read(IOBuffer* buf, int buf_len,
+                   CompletionCallback* callback) {
+    return incoming_->Read(buf, buf_len, callback);
+  }
+
+  virtual int Write(IOBuffer* buf, int buf_len,
+                    CompletionCallback* callback) {
+    return outgoing_->Write(buf, buf_len, callback);
+  }
+
+  virtual bool SetReceiveBufferSize(int32 size) {
+    return true;
+  }
+
+  virtual bool SetSendBufferSize(int32 size) {
+    return true;
+  }
+
+  virtual int Connect(CompletionCallback* callback) {
+    return net::OK;
+  }
+
+  virtual void Disconnect() {}
+
+  virtual bool IsConnected() const {
+    return true;
+  }
+
+  virtual bool IsConnectedAndIdle() const {
+    return false;

[wtc, 2010/12/17 00:16:26]

This probably should return true.

[Alpha Left Google, 2010/12/17 08:30:43]

Done.

+  }
+
+  virtual int GetPeerAddress(AddressList* address) const {
+    net::IPAddressNumber ip_address(4);
+    *address = net::AddressList(ip_address, 0, false);
+    return net::OK;
+  }
+
+  virtual const BoundNetLog& NetLog() const {
+    return net_log_;
+  }
+
+  virtual void SetSubresourceSpeculation() {}
+  virtual void SetOmniboxSpeculation() {}
+
+  virtual bool WasEverUsed() const {
+    return true;
+  }
+
+  virtual bool UsingTCPFastOpen() const {
+    return false;
+  }
+
+ private:
+  net::BoundNetLog net_log_;
+  FakeDataChannel* incoming_;
+  FakeDataChannel* outgoing_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeClientSocket);
+};
+
+}  // namespace
+
+// Verify the correctness of the test helper classes first.
+TEST(FakeClientSocketTest, DataTransfer) {
+  // Establish connections between two sockets.

[wtc, 2010/12/17 00:16:26]

Nit: connections => channels

[Alpha Left Google, 2010/12/17 08:30:43]

Done.

+  FakeDataChannel channel_1;
+  FakeDataChannel channel_2;
+  FakeClientSocket client(&channel_1, &channel_2);
+  FakeClientSocket server(&channel_2, &channel_1);
+
+  const char kTestData[] = "testing123";
+  const int kTestDataSize = strlen(kTestData);
+  const int kReadBufSize = 1024;
+  scoped_refptr<net::IOBuffer> write_buf = new net::StringIOBuffer(kTestData);
+  scoped_refptr<net::IOBuffer> read_buf = new net::IOBuffer(kReadBufSize);
+
+  // Write then read.
+  EXPECT_EQ(kTestDataSize, server.Write(write_buf, kTestDataSize, NULL));
+  EXPECT_EQ(kTestDataSize, client.Read(read_buf, kReadBufSize, NULL));
+  EXPECT_EQ(0, memcmp(kTestData, read_buf->data(), kTestDataSize));
+
+  // Read then write.
+  TestCompletionCallback callback;
+  EXPECT_EQ(net::ERR_IO_PENDING,
+            server.Read(read_buf, kReadBufSize, &callback));
+  EXPECT_EQ(kTestDataSize, client.Write(write_buf, kTestDataSize, NULL));
+  EXPECT_EQ(0, memcmp(kTestData, read_buf->data(), kTestDataSize));
+  EXPECT_EQ(kTestDataSize, callback.WaitForResult());

[wtc, 2010/12/17 00:16:26]

BUG: these two statements should be reversed.  You need to
first wait for Read completion and then do the memcmp.

[Alpha Left Google, 2010/12/17 08:30:43]

nice catch, done.

+}
+
+class SSLServerSocketTest : public PlatformTest {
+ public:
+  SSLServerSocketTest()
+      : socket_factory_(net::ClientSocketFactory::GetDefaultFactory()) {
+  }
+
+ protected:
+  void Initialize() {
+    net::ClientSocket* fake_client_socket = new FakeClientSocket(&channel_1_,
+                                                                 &channel_2_);
+    net::ClientSocket* fake_server_socket = new FakeClientSocket(&channel_2_,

[wtc, 2010/12/17 00:16:26]

It is confusing to see fake_server_socket is a ClientSocket.
I hope it's possible to clean this up.  Perhaps define
both FakeSocket and FakeClientSocket?

[Alpha Left Google, 2010/12/17 08:30:43]

Renamed FakeClientSocket to FakeSocket. And instead of net::ClientSocket*, have
the variables be type FakeSocket*.

+                                                                 &channel_1_);
+
+    // Generate private key and certificate.
+    scoped_ptr<base::RSAPrivateKey> private_key(
+        base::RSAPrivateKey::Create(1024));
+
+    std::string subject = "CN=net-unittest";
+    scoped_refptr<net::X509Certificate> cert =
+        net::X509Certificate::CreateSelfSigned(
+            private_key.get(), subject, 0, base::TimeDelta::FromDays(1));
+
+    net::SSLConfig ssl_config;
+    ssl_config.false_start_enabled = false;
+    ssl_config.snap_start_enabled = false;
+    ssl_config.ssl3_enabled = true;
+    ssl_config.tls1_enabled = true;
+    ssl_config.no_cache_enabled = true;
+
+    // Certificate provided by the host doesn't need authority.
+    net::SSLConfig::CertAndStatus cert_and_status;
+    cert_and_status.cert_status = net::ERR_CERT_AUTHORITY_INVALID;
+    cert_and_status.cert = cert;
+    ssl_config.allowed_bad_certs.push_back(cert_and_status);
+
+    net::HostPortPair host_and_pair("net-unittest", 0);
+    client_socket_.reset(
+        net::ClientSocketFactory::GetDefaultFactory()->CreateSSLClientSocket(

[wtc, 2010/12/17 00:16:26]

You should replace net::ClientSocketFactory::GetDefaultFactory()
by socket_factory_, otherwise the socket_factory_ member
is not being used.  Or you can just remove the socket_factory_
member.

[Alpha Left Google, 2010/12/17 08:30:43]

Done.

+            fake_client_socket, host_and_pair, ssl_config, NULL));
+    server_socket_.reset(net::CreateSSLServerSocket(fake_server_socket,
+                                                    cert, private_key.get()));
+  }
+
+  FakeDataChannel channel_1_;
+  FakeDataChannel channel_2_;
+  scoped_ptr<net::SSLClientSocket> client_socket_;
+  scoped_ptr<net::SSLServerSocket> server_socket_;
+  net::ClientSocketFactory* socket_factory_;
+};
+
+// SSLServerSocket is only implemented using NSS.
+#if defined(USE_NSS)
+TEST_F(SSLServerSocketTest, Initialize) {

[wtc, 2010/12/17 00:16:26]

What's the purpose of this test?  To check that Initialize()
doesn't crash and has no valgrind errors?

[Alpha Left Google, 2010/12/17 08:30:43]

To make sure setting up the sockets is successful. And to have minimal scope for
valgrind. I'll document that.

+  Initialize();
+}
+
+TEST_F(SSLServerSocketTest, Connect) {

[wtc, 2010/12/17 00:16:26]

Nit: it may be better to name this test Handshake since
it tests not only Connect but also Accept.

[Alpha Left Google, 2010/12/17 08:30:43]

Done.

+  Initialize();
+
+  TestCompletionCallback connect_callback;
+  TestCompletionCallback accept_callback;
+
+  int client_ret = client_socket_->Connect(&connect_callback);
+  EXPECT_TRUE(client_ret == net::OK || client_ret == net::ERR_IO_PENDING);
+
+  int server_ret = server_socket_->Accept(&accept_callback);
+  EXPECT_TRUE(server_ret == net::OK || server_ret == net::ERR_IO_PENDING);
+
+  if (client_ret == net::ERR_IO_PENDING) {

[wtc, 2010/12/17 00:16:26]

Nit: in the 'net' module we usually omit curly braces around
one-liner "if" bodies.

[Alpha Left Google, 2010/12/17 08:30:43]

Yup, I tend to omit {} for a single line, but this is the exception where the
content is a macro.

[wtc, 2010/12/17 16:38:32]

Someone else also told me about this exception.  Does
this exception comes from the Style Guide?

[Alpha Left Google, 2010/12/17 20:09:57]

Don't remember if this is in the Style Guide. But it is dangerous to have
if(...) DCHECK(); and the best is to enclose all marcos by {}.

+    EXPECT_EQ(net::OK, connect_callback.WaitForResult());
+  }
+  if (server_ret == net::ERR_IO_PENDING) {
+    EXPECT_EQ(net::OK, accept_callback.WaitForResult());
+  }
+}
+
+TEST_F(SSLServerSocketTest, DataTransfer) {
+  Initialize();
+
+  TestCompletionCallback connect_callback;
+  TestCompletionCallback accept_callback;
+
+  // Establish connection.
+  int client_ret = client_socket_->Connect(&connect_callback);
+  EXPECT_TRUE(client_ret == net::OK || client_ret == net::ERR_IO_PENDING);
+
+  int server_ret = server_socket_->Accept(&accept_callback);
+  EXPECT_TRUE(server_ret == net::OK || server_ret == net::ERR_IO_PENDING);
+
+  if (client_ret == net::ERR_IO_PENDING) {
+    EXPECT_EQ(net::OK, connect_callback.WaitForResult());
+  }
+  if (server_ret == net::ERR_IO_PENDING) {
+    EXPECT_EQ(net::OK, accept_callback.WaitForResult());
+  }
+
+  const char kTestData[] = "testing123";
+  const int kTestDataSize = strlen(kTestData);
+  const int kReadBufSize = 1024;
+  scoped_refptr<net::IOBuffer> write_buf = new net::StringIOBuffer(kTestData);
+  scoped_refptr<net::IOBuffer> read_buf = new net::IOBuffer(kReadBufSize);
+
+  // Write then read.
+  TestCompletionCallback write_callback;
+  TestCompletionCallback read_callback;
+  server_ret = server_socket_->Write(write_buf, kTestDataSize, &write_callback);
+  EXPECT_TRUE(server_ret > 0 || server_ret == net::ERR_IO_PENDING);
+  client_ret = client_socket_->Read(read_buf, kReadBufSize, &read_callback);
+  EXPECT_TRUE(client_ret > 0 || client_ret == net::ERR_IO_PENDING);
+
+  if (server_ret == net::ERR_IO_PENDING) {
+    EXPECT_GT(write_callback.WaitForResult(), 0);
+  }
+  if (client_ret == net::ERR_IO_PENDING) {
+    EXPECT_GT(read_callback.WaitForResult(), 0);
+  }
+  EXPECT_EQ(0, memcmp(kTestData, read_buf->data(), kTestDataSize));
+
+  // Read then write.

[wtc, 2010/12/17 00:16:26]

Since you reuse the read buffer for the "Read then write"
part of this unit test, it'd be better to have a second
test data, so that you can be sure the second Read call
actually reads in new data.

[Alpha Left Google, 2010/12/17 08:30:43]

Done.

+  server_ret = server_socket_->Read(read_buf, kReadBufSize, &read_callback);
+  EXPECT_TRUE(server_ret > 0 || server_ret == net::ERR_IO_PENDING);
+  client_ret = client_socket_->Write(write_buf, kTestDataSize, &write_callback);
+  EXPECT_TRUE(client_ret > 0 || client_ret == net::ERR_IO_PENDING);
+
+  if (server_ret == net::ERR_IO_PENDING) {
+    EXPECT_GT(read_callback.WaitForResult(), 0);
+  }
+  if (client_ret == net::ERR_IO_PENDING) {
+    EXPECT_GT(write_callback.WaitForResult(), 0);
+  }
+  EXPECT_EQ(0, memcmp(kTestData, read_buf->data(), kTestDataSize));
+}
+#endif
+
+}  // namespace net