Introduce QuicStreamFactoryTest::DefaultProofVerifyDetails().

net/quic/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/quic_stream_factory.h"
 
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/cert_verifier.h"
@@ skipping 287 matching lines @@
     return port;
   }
 
   scoped_ptr<QuicEncryptedPacket> ConstructRstPacket() {
     QuicStreamId stream_id = kClientDataStreamId1;
     return maker_.MakeRstPacket(
         1, true, stream_id,
         AdjustErrorForVersion(QUIC_RST_ACKNOWLEDGEMENT, GetParam().version));
   }
 
+  // Note that MockCryptoClientStreamFactory::AddProofVerifyDetails() does not

[Ryan Hamilton, 2015/05/19 19:16:56]

I don't understand what this comment means.  What is a "named object"?

[Bence, 2015/05/19 19:45:33]

I'm landing without the comment as you suggested, but I'd like to add this in
the next CL.  Please help me with the wording here.  I'm trying to say that
crypto_client_stream_factory_.AddProofVerifyDetails(DefaultProofVerifyDetails())
would not work, because the argument is a temporary that gets destroyed right
after the call finished.  Instead, the test need to have an object in local
scope that remains valid while the test runs.

Or is it better to proactively change the implementation to prevent
use-after-free error?

[Ryan Hamilton, 2015/05/19 19:55:34]

Will that actually compile? AddProofVerifyDetails takes a ProofVerifyDetails*
but DefaultProofVerifyDetails() returns a ProofVerifyDetails (pointer vs object)

+  // take ownership of ProofVerifyDetailsChromium, therefore it has to be a
+  // named object so that it is valid until the end of the test.
+  static ProofVerifyDetailsChromium DefaultProofVerifyDetails() {
+    // Load a certificate that is valid for www.example.org, mail.example.org,
+    // and mail.example.com.
+    scoped_refptr<X509Certificate> test_cert(
+        ImportCertFromFile(GetTestCertsDirectory(), "spdy_pooling.pem"));
+    EXPECT_TRUE(test_cert.get());
+    ProofVerifyDetailsChromium verify_details;
+    verify_details.cert_verify_result.verified_cert = test_cert;
+    verify_details.cert_verify_result.is_issued_by_known_root = true;
+    return verify_details;
+  }
+
   MockQuicServerInfoFactory quic_server_info_factory_;
   MockHostResolver host_resolver_;
   DeterministicMockClientSocketFactory socket_factory_;
   MockCryptoClientStreamFactory crypto_client_stream_factory_;
   MockRandom random_generator_;
   MockClock* clock_;  // Owned by factory_.
   scoped_refptr<TestTaskRunner> runner_;
   QuicTestPacketMaker maker_;
   scoped_ptr<CertVerifier> cert_verifier_;
   scoped_ptr<ChannelIDService> channel_id_service_;
@@ skipping 327 matching lines @@
   MockRead reads[] = {
     MockRead(ASYNC, OK, 0)  // EOF
   };
   DeterministicSocketData socket_data(reads, arraysize(reads), nullptr, 0);
   socket_factory_.AddSocketDataProvider(&socket_data);
   socket_data.StopAfter(1);
 
   HostPortPair server1("www.example.org", 443);
   HostPortPair server2("mail.example.org", 443);
 
-  // Load a cert that is valid for:
-  //   www.example.org (server1)
-  //   mail.example.org (server2)
-  //   www.example.com
-  base::FilePath certs_dir = GetTestCertsDirectory();
-  scoped_refptr<X509Certificate> test_cert(
-      ImportCertFromFile(certs_dir, "spdy_pooling.pem"));
-  ASSERT_NE(static_cast<X509Certificate*>(nullptr), test_cert.get());
-  ProofVerifyDetailsChromium verify_details;
-  verify_details.cert_verify_result.verified_cert = test_cert;
-  verify_details.cert_verify_result.is_issued_by_known_root = true;
+  ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
 
   host_resolver_.set_synchronous_mode(true);
   host_resolver_.rules()->AddIPLiteralRule(server1.host(), "192.168.0.1", "");
   host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");
 
   QuicStreamRequest request(&factory_);
   is_https_ = true;
   EXPECT_EQ(OK,
             request.Request(server1, is_https_, privacy_mode_, server1.host(),
@@ skipping 25 matching lines @@
   DeterministicSocketData socket_data1(reads, arraysize(reads), nullptr, 0);
   DeterministicSocketData socket_data2(reads, arraysize(reads), nullptr, 0);
   socket_factory_.AddSocketDataProvider(&socket_data1);
   socket_factory_.AddSocketDataProvider(&socket_data2);
   socket_data1.StopAfter(1);
   socket_data2.StopAfter(1);
 
   HostPortPair server1("www.example.org", 443);
   HostPortPair server2("mail.example.org", 443);
 
-  // Load a cert that is valid for:
-  //   www.example.org (server1)
-  //   mail.example.org (server2)
-  //   www.example.com
-  base::FilePath certs_dir = GetTestCertsDirectory();
-  scoped_refptr<X509Certificate> test_cert(
-      ImportCertFromFile(certs_dir, "spdy_pooling.pem"));
-  ASSERT_NE(static_cast<X509Certificate*>(nullptr), test_cert.get());
-  ProofVerifyDetailsChromium verify_details;
-  verify_details.cert_verify_result.verified_cert = test_cert;
-  verify_details.cert_verify_result.is_issued_by_known_root = true;
+  ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
 
   host_resolver_.set_synchronous_mode(true);
   host_resolver_.rules()->AddIPLiteralRule(server1.host(), "192.168.0.1", "");
   host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");
 
   // Disable connection pooling.
   QuicStreamFactoryPeer::DisableConnectionPooling(&factory_);
 
   QuicStreamRequest request(&factory_);
@@ skipping 30 matching lines @@
   DeterministicSocketData socket_data1(reads, arraysize(reads), nullptr, 0);
   DeterministicSocketData socket_data2(reads, arraysize(reads), nullptr, 0);
   socket_factory_.AddSocketDataProvider(&socket_data1);
   socket_factory_.AddSocketDataProvider(&socket_data2);
   socket_data1.StopAfter(1);
   socket_data2.StopAfter(1);
 
   HostPortPair server1("www.example.org", 443);
   HostPortPair server2("mail.google.com", 443);
 
-  // Load a cert that is valid for:
-  //   www.example.org (server1)
-  //   mail.example.org
-  //   www.example.com
-  // But is not valid for mail.google.com (server2).
-  base::FilePath certs_dir = GetTestCertsDirectory();
-  scoped_refptr<X509Certificate> test_cert(
-      ImportCertFromFile(certs_dir, "spdy_pooling.pem"));
-  ASSERT_NE(static_cast<X509Certificate*>(nullptr), test_cert.get());
-  ProofVerifyDetailsChromium verify_details;
-  verify_details.cert_verify_result.verified_cert = test_cert;
+  ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
 
   host_resolver_.set_synchronous_mode(true);
   host_resolver_.rules()->AddIPLiteralRule(server1.host(), "192.168.0.1", "");
   host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");
 
   QuicStreamRequest request(&factory_);
   is_https_ = true;
   EXPECT_EQ(OK,
             request.Request(server1, is_https_, privacy_mode_, server1.host(),
@@ skipping 28 matching lines @@
   socket_factory_.AddSocketDataProvider(&socket_data);
   socket_data.StopAfter(1);
 
   HostPortPair server1("www.example.org", 443);
   HostPortPair server2("mail.example.org", 443);
   uint8 primary_pin = 1;
   uint8 backup_pin = 2;
   test::AddPin(&transport_security_state_, "mail.example.org", primary_pin,
                backup_pin);
 
-  // Load a cert that is valid for:
-  //   www.example.org (server1)
-  //   mail.example.org (server2)
-  base::FilePath certs_dir = GetTestCertsDirectory();
-  scoped_refptr<X509Certificate> test_cert(
-      ImportCertFromFile(certs_dir, "spdy_pooling.pem"));
-  ASSERT_NE(static_cast<X509Certificate*>(nullptr), test_cert.get());
-  ProofVerifyDetailsChromium verify_details;
-  verify_details.cert_verify_result.verified_cert = test_cert;
-  verify_details.cert_verify_result.is_issued_by_known_root = true;
+  ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
   verify_details.cert_verify_result.public_key_hashes.push_back(
       test::GetTestHashValue(primary_pin));
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
 
   host_resolver_.set_synchronous_mode(true);
   host_resolver_.rules()->AddIPLiteralRule(server1.host(), "192.168.0.1", "");
   host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");
 
   QuicStreamRequest request(&factory_);
   is_https_ = true;
@@ skipping 31 matching lines @@
   socket_data1.StopAfter(1);
   socket_data2.StopAfter(1);
 
   HostPortPair server1("www.example.org", 443);
   HostPortPair server2("mail.example.org", 443);
   uint8 primary_pin = 1;
   uint8 backup_pin = 2;
   test::AddPin(&transport_security_state_, "mail.example.org", primary_pin,
                backup_pin);
 
-  // Load a cert that is valid for:
-  //   www.example.org (server1)
-  //   mail.example.org (server2)
-  base::FilePath certs_dir = GetTestCertsDirectory();
-  scoped_refptr<X509Certificate> test_cert(
-      ImportCertFromFile(certs_dir, "spdy_pooling.pem"));
-  ASSERT_NE(static_cast<X509Certificate*>(nullptr), test_cert.get());
-  ProofVerifyDetailsChromium verify_details;
-  verify_details.cert_verify_result.verified_cert = test_cert;
-  verify_details.cert_verify_result.is_issued_by_known_root = true;
+  ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
   verify_details.cert_verify_result.public_key_hashes.push_back(
       test::GetTestHashValue(primary_pin));
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
 
   host_resolver_.set_synchronous_mode(true);
   host_resolver_.rules()->AddIPLiteralRule(server1.host(), "192.168.0.1", "");
   host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");
 
   // Disable connection pooling.
   QuicStreamFactoryPeer::DisableConnectionPooling(&factory_);
@@ skipping 37 matching lines @@
   socket_data2.StopAfter(1);
 
   HostPortPair server1("www.example.org", 443);
   HostPortPair server2("mail.example.org", 443);
   uint8 primary_pin = 1;
   uint8 backup_pin = 2;
   uint8 bad_pin = 3;
   test::AddPin(&transport_security_state_, "mail.example.org", primary_pin,
                backup_pin);
 
-  // Load a cert that is valid for:
-  //   www.example.org (server1)
-  //   mail.example.org (server2)
-  base::FilePath certs_dir = GetTestCertsDirectory();
-  scoped_refptr<X509Certificate> test_cert(
-      ImportCertFromFile(certs_dir, "spdy_pooling.pem"));
-  ASSERT_NE(static_cast<X509Certificate*>(nullptr), test_cert.get());
-
-  ProofVerifyDetailsChromium verify_details1;
-  verify_details1.cert_verify_result.verified_cert = test_cert;
-  verify_details1.cert_verify_result.is_issued_by_known_root = true;
+  ProofVerifyDetailsChromium verify_details1 = DefaultProofVerifyDetails();
   verify_details1.cert_verify_result.public_key_hashes.push_back(
       test::GetTestHashValue(bad_pin));
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details1);
 
-  ProofVerifyDetailsChromium verify_details2;
-  verify_details2.cert_verify_result.verified_cert = test_cert;
-  verify_details2.cert_verify_result.is_issued_by_known_root = true;
+  ProofVerifyDetailsChromium verify_details2 = DefaultProofVerifyDetails();
   verify_details2.cert_verify_result.public_key_hashes.push_back(
       test::GetTestHashValue(primary_pin));
   crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details2);
 
   host_resolver_.set_synchronous_mode(true);
   host_resolver_.rules()->AddIPLiteralRule(server1.host(), "192.168.0.1", "");
   host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");
 
   QuicStreamRequest request(&factory_);
   is_https_ = true;
@@ skipping 740 matching lines @@
   EXPECT_TRUE(socket_data.AllReadDataConsumed());
   EXPECT_TRUE(socket_data.AllWriteDataConsumed());
   EXPECT_TRUE(socket_data2.AllReadDataConsumed());
   EXPECT_TRUE(socket_data2.AllWriteDataConsumed());
   EXPECT_TRUE(socket_data3.AllReadDataConsumed());
   EXPECT_TRUE(socket_data3.AllWriteDataConsumed());
 }
 
 }  // namespace test
 }  // namespace net