QUIC: mark QUIC handshake failed if connection is closed after CryptoConnect

net/quic/chromium/quic_stream_factory_test.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.
 
 #include "net/quic/chromium/quic_stream_factory.h"
 
 #include <memory>
 #include <ostream>
 #include <utility>
 
@@ skipping 298 matching lines @@
     migrate_sessions_on_network_change_ = true;
     migrate_sessions_early_ = true;
     Initialize();
   }
 
   bool HasActiveSession(const HostPortPair& host_port_pair) {
     QuicServerId server_id(host_port_pair, PRIVACY_MODE_DISABLED);
     return QuicStreamFactoryPeer::HasActiveSession(factory_.get(), server_id);
   }
 
+  bool HasActiveJob(const HostPortPair& host_port_pair,
+                    const PrivacyMode privacy_mode) {
+    QuicServerId server_id(host_port_pair, privacy_mode);
+    return QuicStreamFactoryPeer::HasActiveJob(factory_.get(), server_id);
+  }
+
   bool HasActiveCertVerifierJob(const QuicServerId& server_id) {
     return QuicStreamFactoryPeer::HasActiveCertVerifierJob(factory_.get(),
                                                            server_id);
   }
 
   QuicChromiumClientSession* GetActiveSession(
       const HostPortPair& host_port_pair) {
     QuicServerId server_id(host_port_pair, PRIVACY_MODE_DISABLED);
     return QuicStreamFactoryPeer::GetActiveSession(factory_.get(), server_id);
   }
@@ skipping 1467 matching lines @@
   EXPECT_THAT(callback_.WaitForResult(), IsOk());
   stream = request2.CreateStream();
   stream.reset();  // Will reset stream 3.
 
   EXPECT_TRUE(socket_data.AllReadDataConsumed());
   EXPECT_TRUE(socket_data.AllWriteDataConsumed());
   EXPECT_TRUE(socket_data2.AllReadDataConsumed());
   EXPECT_TRUE(socket_data2.AllWriteDataConsumed());
 }
 
+// Regression test for crbug.com/700617. Test a write error during the
+// crypto handshake will not hang QuicStreamFactory::Job and should
+// report QUIC_HANDSHAKE_FAILED to upper layers.
+TEST_P(QuicStreamFactoryTest, WriteErrorInCryptoConnect) {
+  Initialize();
+  crypto_client_stream_factory_.set_use_normal_crypto_stream();
+
+  MockQuicData socket_data;
+  socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
+  socket_data.AddWrite(SYNCHRONOUS, ERR_ADDRESS_UNREACHABLE);
+  socket_data.AddSocketDataToFactory(&socket_factory_);
+
+  // Create request and QuicHttpStream.
+  QuicStreamRequest request(factory_.get());
+  EXPECT_EQ(ERR_IO_PENDING,

[Ryan Hamilton, 2017/03/22 02:38:48]

I'm a bit surprised that this returns ERR_IO_PENDING instead of
QUIC_HANDSHAKE_FAILED. Is that because DNS resolution (in this test) in async?
If so, can you make another version of this test which does
host_resolver_.set_synchronous_mode(true); (look for other uses in this file)
and verify that it fails immediately?

[Zhongyi Shi, 2017/03/22 07:00:11]

Done.

+            request.Request(host_port_pair_, privacy_mode_,
+                            /*cert_verify_flags=*/0, url_, "GET", net_log_,
+                            callback_.callback()));
+  EXPECT_EQ(ERR_QUIC_HANDSHAKE_FAILED, callback_.WaitForResult());
+  EXPECT_FALSE(HasActiveSession(host_port_pair_));
+  EXPECT_FALSE(HasActiveJob(host_port_pair_, privacy_mode_));

[Ryan Hamilton, 2017/03/22 02:38:48]

If I understand the bug correctly, in the old code, if we had tried to make a
new request at this point, the request would have hung. So instead of checking
with the peer that there is no active job can you make another request, and this
time let it succeed?

[Zhongyi Shi, 2017/03/22 07:00:11]

Done. Good point! 

I have checked those regression test will fail w/o the fix.

[Ryan Hamilton, 2017/03/22 14:13:01]

It looks like this is still testing for an active not, instead of testing that a
new QuicStreamRequest will not hang.
Great!

[Zhongyi Shi, 2017/03/22 18:53:09]

Done.

+}
+
 TEST_P(QuicStreamFactoryTest, OnIPAddressChanged) {
   close_sessions_on_ip_change_ = true;
   Initialize();
   ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
 
   MockQuicData socket_data;
   socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
   socket_data.AddWrite(
@@ skipping 3865 matching lines @@
   // Clear all cached states.
   factory_->ClearCachedStatesInCryptoConfig(
       base::Callback<bool(const GURL&)>());
   EXPECT_TRUE(test_cases[0].state->certs().empty());
   EXPECT_TRUE(test_cases[1].state->certs().empty());
   EXPECT_TRUE(test_cases[2].state->certs().empty());
 }
 
 }  // namespace test
 }  // namespace net