Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(37)

Unified Diff: net/socket/ssl_client_socket_unittest.cc

Issue 280853002: Preserve transport errors for OpenSSL sockets. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Rephrase a lot of comments. Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 7748f07106b2c73f1eeab10bd6ede8e832b5a5dd..fc051e5f6d52cfceea607a5a0c21e8c6dda1e680 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -1174,6 +1174,43 @@ TEST_F(SSLClientSocketTest, Read) {
}
}
+// Tests that SSLClientSocket properly handle when the underlying transport
+// synchronously returns an error code on read failure. The error code should be
+// echoed back so SSLv3 fallback logic can condition on it.
+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.
@@ -1229,13 +1266,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
@@ -1310,13 +1341,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.
@@ -1491,13 +1516,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());
@@ -1592,21 +1611,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.
EXPECT_EQ(ERR_CONNECTION_RESET, rv);
+#endif
}
TEST_F(SSLClientSocketTest, Read_SmallChunks) {

Powered by Google App Engine
This is Rietveld 408576698