Add bounds checking to StaticSocketDataProvider, to make tests more reliable

when they fail.

TEST=net_unittests
BUG=27567

Review URL: http://codereview.chromium.org/582020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38453 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 77 insertions, 43 deletions

diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index 283ae35..1ea1ec9 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -284,11 +284,8 @@ int MockSSLClientSocket::Write(net::IOBuffer* buf, int buf_len,
 }
 
 MockRead StaticSocketDataProvider::GetNextRead() {
-  MockRead rv = reads_[read_index_];
-  if (reads_[read_index_].result != OK ||
-      reads_[read_index_].data_len != 0)
-    read_index_++;  // Don't advance past an EOF.
-  return rv;
+  DCHECK(!at_read_eof());
+  return reads_[read_index_++];
 }
 
 MockWriteResult StaticSocketDataProvider::OnWrite(const std::string& data) {
@@ -297,6 +294,8 @@ MockWriteResult StaticSocketDataProvider::OnWrite(const std::string& data) {
     return MockWriteResult(false, data.length());
   }
 
+  DCHECK(!at_write_eof());
+
   // Check that what we are writing matches the expectation.
   // Then give the mocked return value.
   net::MockWrite* w = &writes_[write_index_++];
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index 90b4019..5e1fe65 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -121,28 +121,33 @@ class SocketDataProvider {
 // writes.
 class StaticSocketDataProvider : public SocketDataProvider {
  public:
-  StaticSocketDataProvider() : reads_(NULL), read_index_(0),
-      writes_(NULL), write_index_(0) {}
-  StaticSocketDataProvider(MockRead* r, MockWrite* w) : reads_(r),
-      read_index_(0), writes_(w), write_index_(0) {}
+  StaticSocketDataProvider() : reads_(NULL), read_index_(0), read_count_(0),
+      writes_(NULL), write_index_(0), write_count_(0) {}
+  StaticSocketDataProvider(MockRead* reads, size_t reads_count,
+                           MockWrite* writes, size_t writes_count)
+      : reads_(reads),
+        read_index_(0),
+        read_count_(reads_count),
+        writes_(writes),
+        write_index_(0),
+        write_count_(writes_count) {
+  }
 
   // SocketDataProvider methods:
   virtual MockRead GetNextRead();
   virtual MockWriteResult OnWrite(const std::string& data);
   virtual void Reset();
 
-  // If the test wishes to verify that all data is consumed, it can include
-  // a EOF MockRead or MockWrite, which is a zero-length Read or Write.
-  // The test can then call at_read_eof() or at_write_eof() to verify that
-  // all data has been consumed.
-  bool at_read_eof() const { return reads_[read_index_].data_len == 0; }
-  bool at_write_eof() const { return writes_[write_index_].data_len == 0; }
+  bool at_read_eof() const { return read_index_ >= read_count_; }
+  bool at_write_eof() const { return write_index_ >= write_count_; }
 
  private:
   MockRead* reads_;
-  int read_index_;
+  size_t read_index_;
+  size_t read_count_;
   MockWrite* writes_;
-  int write_index_;
+  size_t write_index_;
+  size_t write_count_;
 
   DISALLOW_COPY_AND_ASSIGN(StaticSocketDataProvider);
 };
diff --git a/net/socket/socks5_client_socket_unittest.cc b/net/socket/socks5_client_socket_unittest.cc
index 4b87a14..75d0619 100644
--- a/net/socket/socks5_client_socket_unittest.cc
+++ b/net/socket/socks5_client_socket_unittest.cc
@@ -32,7 +32,9 @@ class SOCKS5ClientSocketTest : public PlatformTest {
   SOCKS5ClientSocketTest();
   // Create a SOCKSClientSocket on top of a MockSocket.
   SOCKS5ClientSocket* BuildMockSocket(MockRead reads[],
+                                      size_t reads_count,
                                       MockWrite writes[],
+                                      size_t writes_count,
                                       const std::string& hostname,
                                       int port);
 
@@ -67,11 +69,14 @@ void SOCKS5ClientSocketTest::SetUp() {
 
 SOCKS5ClientSocket* SOCKS5ClientSocketTest::BuildMockSocket(
     MockRead reads[],
+    size_t reads_count,
     MockWrite writes[],
+    size_t writes_count,
     const std::string& hostname,
     int port) {
   TestCompletionCallback callback;
-  data_.reset(new StaticSocketDataProvider(reads, writes));
+  data_.reset(new StaticSocketDataProvider(reads, reads_count,
+                                           writes, writes_count));
   tcp_sock_ = new MockTCPClientSocket(address_list_, data_.get());
 
   int rv = tcp_sock_->Connect(&callback, NULL);
@@ -115,7 +120,9 @@ TEST_F(SOCKS5ClientSocketTest, CompleteHandshake) {
       MockRead(true, kSOCKS5OkResponse, arraysize(kSOCKS5OkResponse)),
       MockRead(true, payload_read.data(), payload_read.size()) };
 
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes, "localhost", 80));
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
+                                   "localhost", 80));
 
   // At this state the TCP connection is completed but not the SOCKS handshake.
   EXPECT_TRUE(tcp_sock_->IsConnected());
@@ -177,7 +184,9 @@ TEST_F(SOCKS5ClientSocketTest, ConnectAndDisconnectTwice) {
         MockRead(false, kSOCKS5OkResponse, arraysize(kSOCKS5OkResponse))
     };
 
-    user_sock_.reset(BuildMockSocket(data_reads, data_writes, hostname, 80));
+    user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                     data_writes, arraysize(data_writes),
+                                     hostname, 80));
 
     int rv = user_sock_->Connect(&callback_, NULL);
     EXPECT_EQ(OK, rv);
@@ -197,7 +206,8 @@ TEST_F(SOCKS5ClientSocketTest, LargeHostNameFails) {
   // Create a SOCKS socket, with mock transport socket.
   MockWrite data_writes[] = {MockWrite()};
   MockRead data_reads[] = {MockRead()};
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes,
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
                                    large_host_name, 80));
 
   // Try to connect -- should fail (without having read/written anything to
@@ -232,7 +242,9 @@ TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) {
     MockRead data_reads[] = {
         MockRead(true, kSOCKS5GreetResponse, arraysize(kSOCKS5GreetResponse)),
         MockRead(true, kSOCKS5OkResponse, arraysize(kSOCKS5OkResponse)) };
-    user_sock_.reset(BuildMockSocket(data_reads, data_writes, hostname, 80));
+    user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                     data_writes, arraysize(data_writes),
+                                     hostname, 80));
     scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
     int rv = user_sock_->Connect(&callback_, log);
     EXPECT_EQ(ERR_IO_PENDING, rv);
@@ -254,7 +266,9 @@ TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) {
         MockRead(true, partial1, arraysize(partial1)),
         MockRead(true, partial2, arraysize(partial2)),
         MockRead(true, kSOCKS5OkResponse, arraysize(kSOCKS5OkResponse)) };
-    user_sock_.reset(BuildMockSocket(data_reads, data_writes, hostname, 80));
+    user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                     data_writes, arraysize(data_writes),
+                                     hostname, 80));
     scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
     int rv = user_sock_->Connect(&callback_, log);
     EXPECT_EQ(ERR_IO_PENDING, rv);
@@ -277,7 +291,9 @@ TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) {
     MockRead data_reads[] = {
         MockRead(true, kSOCKS5GreetResponse, arraysize(kSOCKS5GreetResponse)),
         MockRead(true, kSOCKS5OkResponse, arraysize(kSOCKS5OkResponse)) };
-    user_sock_.reset(BuildMockSocket(data_reads, data_writes, hostname, 80));
+    user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                     data_writes, arraysize(data_writes),
+                                     hostname, 80));
     scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
     int rv = user_sock_->Connect(&callback_, log);
     EXPECT_EQ(ERR_IO_PENDING, rv);
@@ -302,7 +318,9 @@ TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) {
                  arraysize(kSOCKS5OkResponse) - kSplitPoint)
     };
 
-    user_sock_.reset(BuildMockSocket(data_reads, data_writes, hostname, 80));
+    user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                     data_writes, arraysize(data_writes),
+                                     hostname, 80));
     scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
     int rv = user_sock_->Connect(&callback_, log);
     EXPECT_EQ(ERR_IO_PENDING, rv);
diff --git a/net/socket/socks_client_socket_unittest.cc b/net/socket/socks_client_socket_unittest.cc
index b3bb11a..fb1ad82 100644
--- a/net/socket/socks_client_socket_unittest.cc
+++ b/net/socket/socks_client_socket_unittest.cc
@@ -29,7 +29,8 @@ class SOCKSClientSocketTest : public PlatformTest {
  public:
   SOCKSClientSocketTest();
   // Create a SOCKSClientSocket on top of a MockSocket.
-  SOCKSClientSocket* BuildMockSocket(MockRead reads[], MockWrite writes[],
+  SOCKSClientSocket* BuildMockSocket(MockRead reads[], size_t reads_count,
+                                     MockWrite writes[], size_t writes_count,
                                      HostResolver* host_resolver,
                                      const std::string& hostname, int port);
   virtual void SetUp();
@@ -57,13 +58,16 @@ void SOCKSClientSocketTest::SetUp() {
 
 SOCKSClientSocket* SOCKSClientSocketTest::BuildMockSocket(
     MockRead reads[],
+    size_t reads_count,
     MockWrite writes[],
+    size_t writes_count,
     HostResolver* host_resolver,
     const std::string& hostname,
     int port) {
 
   TestCompletionCallback callback;
-  data_.reset(new StaticSocketDataProvider(reads, writes));
+  data_.reset(new StaticSocketDataProvider(reads, reads_count,
+                                           writes, writes_count));
   tcp_sock_ = new MockTCPClientSocket(address_list_, data_.get());
 
   int rv = tcp_sock_->Connect(&callback, NULL);
@@ -126,8 +130,9 @@ TEST_F(SOCKSClientSocketTest, CompleteHandshake) {
       MockRead(true, kSOCKSOkReply, arraysize(kSOCKSOkReply)),
       MockRead(true, payload_read.data(), payload_read.size()) };
 
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes, host_resolver_,
-                                   "localhost", 80));
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
+                                   host_resolver_, "localhost", 80));
 
   // At this state the TCP connection is completed but not the SOCKS handshake.
   EXPECT_TRUE(tcp_sock_->IsConnected());
@@ -192,8 +197,9 @@ TEST_F(SOCKSClientSocketTest, HandshakeFailures) {
     MockRead data_reads[] = {
         MockRead(false, tests[i].fail_reply, arraysize(tests[i].fail_reply)) };
 
-    user_sock_.reset(BuildMockSocket(data_reads, data_writes, host_resolver_,
-                                     "localhost", 80));
+    user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                     data_writes, arraysize(data_writes),
+                                     host_resolver_, "localhost", 80));
     scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
 
     int rv = user_sock_->Connect(&callback_, log);
@@ -219,8 +225,9 @@ TEST_F(SOCKSClientSocketTest, PartialServerReads) {
       MockRead(true, kSOCKSPartialReply1, arraysize(kSOCKSPartialReply1)),
       MockRead(true, kSOCKSPartialReply2, arraysize(kSOCKSPartialReply2)) };
 
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes, host_resolver_,
-                                   "localhost", 80));
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
+                                   host_resolver_, "localhost", 80));
   scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
 
   int rv = user_sock_->Connect(&callback_, log);
@@ -249,8 +256,9 @@ TEST_F(SOCKSClientSocketTest, PartialClientWrites) {
   MockRead data_reads[] = {
       MockRead(true, kSOCKSOkReply, arraysize(kSOCKSOkReply)) };
 
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes, host_resolver_,
-                                   "localhost", 80));
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
+                                   host_resolver_, "localhost", 80));
   scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
 
   int rv = user_sock_->Connect(&callback_, log);
@@ -272,8 +280,9 @@ TEST_F(SOCKSClientSocketTest, FailedSocketRead) {
       // close connection unexpectedly
       MockRead(false, 0) };
 
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes, host_resolver_,
-                                   "localhost", 80));
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
+                                   host_resolver_, "localhost", 80));
   scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
 
   int rv = user_sock_->Connect(&callback_, log);
@@ -300,8 +309,9 @@ TEST_F(SOCKSClientSocketTest, SOCKS4AFailedDNS) {
   MockRead data_reads[] = {
       MockRead(false, kSOCKSOkReply, arraysize(kSOCKSOkReply)) };
 
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes, host_resolver_,
-                                   hostname, 80));
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
+                                   host_resolver_, hostname, 80));
   scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
 
   int rv = user_sock_->Connect(&callback_, log);
@@ -330,8 +340,9 @@ TEST_F(SOCKSClientSocketTest, SOCKS4AIfDomainInIPv6) {
   MockRead data_reads[] = {
       MockRead(false, kSOCKSOkReply, arraysize(kSOCKSOkReply)) };
 
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes, host_resolver_,
-                                   hostname, 80));
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
+                                   host_resolver_, hostname, 80));
   scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
 
   int rv = user_sock_->Connect(&callback_, log);
@@ -354,8 +365,9 @@ TEST_F(SOCKSClientSocketTest, DisconnectWhileHostResolveInProgress) {
   MockWrite data_writes[] = { MockWrite(false, "", 0) };
   MockRead data_reads[] = { MockRead(false, "", 0) };
 
-  user_sock_.reset(BuildMockSocket(data_reads, data_writes, hanging_resolver,
-                                   "foo", 80));
+  user_sock_.reset(BuildMockSocket(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes),
+                                   hanging_resolver, "foo", 80));
 
   // Start connecting (will get stuck waiting for the host to resolve).
   int rv = user_sock_->Connect(&callback_, NULL);