net: add a test to ensure that our TLS handshake doesn't get too large.

net/url_request/url_request_unittest.cc

Index: net/url_request/url_request_unittest.cc
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index d1b0cb43b46dcc6c84a1daef15deb23d2e1252da..fcc10df9ea4fa87be8bd3fca9c6654f0370684f1 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -5405,6 +5405,47 @@ TEST_F(HTTPSRequestTest, SSLSessionCacheShardTest) {
   }
 }
 
+TEST_F(HTTPSRequestTest, ClientHelloTest) {
+  // Check that our ClientHello doesn't get too large because that's known to
+  // cause issues.

[wtc, 2013/07/24 19:25:10]

Nit: we usually put this kind of comment before the test, like this:

// Check that our ClientHello doesn't get too large because that's known to
// cause issues.
TEST_F(HTTPSRequestTest, ClientHelloTest) {

Also, it would be nice to be specific about what is too large (the 256u constant
at the end of the test).

[agl, 2013/07/29 16:58:37]

Done.

+
+  SpawnedTestServer::SSLOptions ssl_options;
+  SpawnedTestServer test_server(
+      SpawnedTestServer::TYPE_HTTPS,
+      ssl_options,
+      base::FilePath(FILE_PATH_LITERAL("net/data/ssl")));
+  ASSERT_TRUE(test_server.Start());
+
+  // NPN/ALPN strings need to be enabled in order for the size of the handshake
+  // to be accurate as ALPN includes these strings in the handshake. This will
+  // be reset by NetTestSuite.
+  HttpStreamFactory::EnableNpnSpdy4a2();
+
+  TestDelegate d;
+  URLRequest r(
+      test_server.GetURL("client-hello-length"), &d, &default_context_);
+
+  r.Start();
+  EXPECT_TRUE(r.is_pending());
+
+  base::MessageLoop::current()->Run();
+
+  // The response will be a single, base 10 number which is the number of bytes
+  // in the ClientHello, including the handshake message type byte and 3 byte
+  // length at the beginning.
+
+  EXPECT_EQ(1, d.response_started_count());
+  unsigned client_hello_length;
+  ASSERT_TRUE(base::StringToUint(d.data_received(), &client_hello_length));
+  const unsigned sni_overhead = 2 /* extension type */ +
+                                2 /* extension length */ +
+                                4 /* lengths in the server_name extension */ +
+                                strlen("www.wellsfargo.com");

[wtc, 2013/07/24 19:25:10]

Why do we add the SNI overhead? Is it because this unit test is using an IP
address (127.0.0.1) in the URL?

[agl, 2013/07/29 16:58:37]

Yes, have noted so in comment.

+  static const unsigned session_id_overhead = 32;
+
+  EXPECT_LT(client_hello_length + sni_overhead + session_id_overhead, 256u);

[wtc, 2013/07/24 19:25:10]

Nit: it may be better to reverse this:
  EXPECT_GT(256u, client_hello_length + sni_overhead + session_id_overhead);

because the EXPECT_xxx arguments are (expect, actual).

+}
+
 class TestSSLConfigService : public SSLConfigService {
  public:
   TestSSLConfigService(bool ev_enabled, bool online_rev_checking)