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

net/url_request/url_request_unittest.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 "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shlobj.h>
 #endif
@@ skipping 5387 matching lines @@
       EXPECT_EQ("insert", parts[0]);
       if (i == 0) {
         session_id = parts[1];
       } else {
         EXPECT_NE(session_id, parts[1]);
       }
     }
   }
 }
 
+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)
       : ev_enabled_(ev_enabled),
         online_rev_checking_(online_rev_checking) {
   }
 
   // SSLConfigService:
   virtual void GetSSLConfig(SSLConfig* config) OVERRIDE {
     *config = SSLConfig();
@@ skipping 694 matching lines @@
 
     EXPECT_FALSE(r.is_pending());
     EXPECT_EQ(1, d->response_started_count());
     EXPECT_FALSE(d->received_data_before_response());
     EXPECT_EQ(d->bytes_received(), static_cast<int>(file_size));
   }
 }
 #endif  // !defined(DISABLE_FTP_SUPPORT)
 
 }  // namespace net