Preserve transport errors for OpenSSL sockets.

net/socket/ssl_client_socket_unittest.cc

Index: net/socket/ssl_client_socket_unittest.cc
diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc
index 2639870e81f36f4fefef5ad4626ced9bc629da13..1147d246b1c39c51d4224d3e5064d2d33b224fc8 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -1136,6 +1136,40 @@ TEST_F(SSLClientSocketTest, Read) {
   }
 }
 
+TEST_F(SSLClientSocketTest, Connect_WithSynchronousError) {
+  SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS,
+                                SpawnedTestServer::kLocalhost,
+                                base::FilePath());
+  ASSERT_TRUE(test_server.Start());
+
+  AddressList addr;
+  ASSERT_TRUE(test_server.GetAddressList(&addr));
+
+  TestCompletionCallback callback;
+  scoped_ptr<StreamSocket> real_transport(
+      new TCPClientSocket(addr, NULL, NetLog::Source()));
+  scoped_ptr<SynchronousErrorStreamSocket> transport(
+      new SynchronousErrorStreamSocket(real_transport.Pass()));
+  int rv = callback.GetResult(transport->Connect(callback.callback()));
+  EXPECT_EQ(OK, rv);
+
+  // Disable TLS False Start to avoid handshake non-determinism.
+  SSLConfig ssl_config;
+  ssl_config.false_start_enabled = false;
+
+  SynchronousErrorStreamSocket* raw_transport = transport.get();
+  scoped_ptr<SSLClientSocket> sock(
+      CreateSSLClientSocket(transport.PassAs<StreamSocket>(),
+                            test_server.host_port_pair(),
+                            ssl_config));
+
+  raw_transport->SetNextWriteError(ERR_CONNECTION_RESET);
+
+  rv = callback.GetResult(sock->Connect(callback.callback()));
+  EXPECT_EQ(ERR_CONNECTION_RESET, rv);
+  EXPECT_FALSE(sock->IsConnected());
+}
+
 // Tests that the SSLClientSocket properly handles when the underlying transport
 // synchronously returns an error code - such as if an intermediary terminates
 // the socket connection uncleanly.
@@ -1191,13 +1225,7 @@ TEST_F(SSLClientSocketTest, Read_WithSynchronousError) {
   // result when using a dedicated task runner for NSS.
   rv = callback.GetResult(sock->Read(buf.get(), 4096, callback.callback()));
 
-#if !defined(USE_OPENSSL)
-  // SSLClientSocketNSS records the error exactly
   EXPECT_EQ(ERR_CONNECTION_RESET, rv);
-#else
-  // SSLClientSocketOpenSSL treats any errors as a simple EOF.
-  EXPECT_EQ(0, rv);
-#endif
 }
 
 // Tests that the SSLClientSocket properly handles when the underlying transport
@@ -1272,13 +1300,7 @@ TEST_F(SSLClientSocketTest, Write_WithSynchronousError) {
   // is a legitimate result when using a dedicated task runner for NSS.
   rv = callback.GetResult(rv);
 
-#if !defined(USE_OPENSSL)
-  // SSLClientSocketNSS records the error exactly
   EXPECT_EQ(ERR_CONNECTION_RESET, rv);
-#else
-  // SSLClientSocketOpenSSL treats any errors as a simple EOF.
-  EXPECT_EQ(0, rv);
-#endif
 }
 
 // Test the full duplex mode, with Read and Write pending at the same time.
@@ -1453,13 +1475,7 @@ TEST_F(SSLClientSocketTest, Read_DeleteWhilePendingFullDuplex) {
 
   rv = read_callback.WaitForResult();
 
-#if !defined(USE_OPENSSL)
-  // NSS records the error exactly.
   EXPECT_EQ(ERR_CONNECTION_RESET, rv);
-#else
-  // OpenSSL treats any errors as a simple EOF.
-  EXPECT_EQ(0, rv);
-#endif
 
   // The Write callback should not have been called.
   EXPECT_FALSE(callback.have_result());
@@ -1554,21 +1570,21 @@ TEST_F(SSLClientSocketTest, Read_WithWriteError) {
     }
   } while (rv > 0);
 
-#if !defined(USE_OPENSSL)
-  // NSS records the error exactly.
   EXPECT_EQ(ERR_CONNECTION_RESET, rv);
-#else
-  // OpenSSL treats the reset as a generic protocol error.
-  EXPECT_EQ(ERR_SSL_PROTOCOL_ERROR, rv);
-#endif
 
-  // Release the read. Some bytes should go through.
+  // Release the read.
   raw_transport->UnblockReadResult();
   rv = read_callback.WaitForResult();
 
-  // Per the fix for http://crbug.com/249848, write failures currently break
-  // reads. Change this assertion if they're changed to not collide.
+#if defined(USE_OPENSSL)
+  // Should still read bytes despite the write error.
+  EXPECT_LT(0, rv);
+#else
+  // NSS attempts to flush the write buffer in PR_Read on an SSL socket before
+  // pumping the read state machine, unless configured with SSL_ENABLE_FDX, so
+  // the write error bleeds into the read.

[davidben, 2014/06/06 23:31:51]

This comment is referring to this block:

https://code.google.com/p/chromium/codesearch#chromium/src/net/third_party/ns...

While after this CL, both nss_memio and SSLClientSocketOpenSSL's BIO both have
the same "almost full-duplex except for the write error hack" behavior, we do
not use NSS internally in a full-duplex mode.

   EXPECT_EQ(ERR_CONNECTION_RESET, rv);
+#endif
 }
 
 TEST_F(SSLClientSocketTest, Read_SmallChunks) {