QUIC - use QuicSessionKey tuple (host, port, is_https) instead of server_hostname

net/http/disk_cache_based_quic_server_info_unittest.cc

Index: net/http/disk_cache_based_quic_server_info_unittest.cc
diff --git a/net/http/disk_cache_based_quic_server_info_unittest.cc b/net/http/disk_cache_based_quic_server_info_unittest.cc
index 4e4591f8da373a08a4d033c64e49373fb58c542d..2efa1690b7d4173c17148ee41f945e58da1a7962 100644
--- a/net/http/disk_cache_based_quic_server_info_unittest.cc
+++ b/net/http/disk_cache_based_quic_server_info_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/message_loop/message_loop.h"
+#include "net/base/host_port_pair.h"
 #include "net/base/net_errors.h"
 #include "net/http/mock_http_cache.h"
 #include "net/quic/crypto/quic_server_info.h"
@@ -16,8 +17,23 @@
 namespace {
 
 // This is an empty transaction, needed to register the URL and the test mode.
-const MockTransaction kHostInfoTransaction = {
-  "quicserverinfo:https://www.google.com",
+const MockTransaction kHostInfoTransaction1 = {
+  "quicserverinfo:https://www.google.com:443",
+  "",
+  base::Time(),
+  "",
+  net::LOAD_NORMAL,
+  "",
+  "",
+  base::Time(),
+  "",
+  TEST_MODE_NORMAL,
+  NULL,
+  0
+};
+
+const MockTransaction kHostInfoTransaction2 = {
+  "quicserverinfo:https://www.google.com:80",
   "",
   base::Time(),
   "",
@@ -38,8 +54,9 @@ TEST(DiskCacheBasedQuicServerInfo, DeleteInCallback) {
   // of quic_server_info->WaitForDataReady(), so that the callback will run.
   MockBlockingBackendFactory* factory = new MockBlockingBackendFactory();
   MockHttpCache cache(factory);
+  net::HostPortPair host_port_pair("https://www.verisign.com", 443);

[wtc, 2014/03/11 01:41:39]

IMPORTANT: the HostPortPair constructor you are using is:

  // If |in_host| represents an IPv6 address, it should not bracket the address.
  HostPortPair(const std::string& in_host, uint16 in_port);

So, I believe you incorrectly include the scheme ("https://") in the |in_host|
argument.

[ramant (doing other things), 2014/03/13 01:50:12]

Done.

   scoped_ptr<net::QuicServerInfo> quic_server_info(
-      new net::DiskCacheBasedQuicServerInfo("https://www.verisign.com",
+      new net::DiskCacheBasedQuicServerInfo(host_port_pair,
                                             cache.http_cache()));
   quic_server_info->Start();
   net::TestCompletionCallback callback;
@@ -53,11 +70,12 @@ TEST(DiskCacheBasedQuicServerInfo, DeleteInCallback) {
 // Tests the basic logic of storing, retrieving and updating data.
 TEST(DiskCacheBasedQuicServerInfo, Update) {
   MockHttpCache cache;
-  AddMockTransaction(&kHostInfoTransaction);
+  AddMockTransaction(&kHostInfoTransaction1);
   net::TestCompletionCallback callback;
 
+  net::HostPortPair host_port_pair("https://www.google.com", 443);
   scoped_ptr<net::QuicServerInfo> quic_server_info(
-      new net::DiskCacheBasedQuicServerInfo("https://www.google.com",
+      new net::DiskCacheBasedQuicServerInfo(host_port_pair,
                                             cache.http_cache()));
   quic_server_info->Start();
   int rv = quic_server_info->WaitForDataReady(callback.callback());
@@ -82,7 +100,7 @@ TEST(DiskCacheBasedQuicServerInfo, Update) {
 
   // Open the stored QuicServerInfo.
   quic_server_info.reset(
-      new net::DiskCacheBasedQuicServerInfo("https://www.google.com",
+      new net::DiskCacheBasedQuicServerInfo(host_port_pair,
                                             cache.http_cache()));
   quic_server_info->Start();
   rv = quic_server_info->WaitForDataReady(callback.callback());
@@ -99,7 +117,7 @@ TEST(DiskCacheBasedQuicServerInfo, Update) {
 
   // Verify that the state was updated.
   quic_server_info.reset(
-      new net::DiskCacheBasedQuicServerInfo("https://www.google.com",
+      new net::DiskCacheBasedQuicServerInfo(host_port_pair,
                                             cache.http_cache()));
   quic_server_info->Start();
   rv = quic_server_info->WaitForDataReady(callback.callback());
@@ -114,7 +132,100 @@ TEST(DiskCacheBasedQuicServerInfo, Update) {
   EXPECT_EQ(cert_a, state1.certs[0]);
   EXPECT_EQ(cert_b, state1.certs[1]);
 
-  RemoveMockTransaction(&kHostInfoTransaction);
+  RemoveMockTransaction(&kHostInfoTransaction1);
+}
+
+// Test that demonstrates different info is returned when the ports differ.
+TEST(DiskCacheBasedQuicServerInfo, UpdateDifferentPorts) {
+  MockHttpCache cache;
+  AddMockTransaction(&kHostInfoTransaction1);
+  AddMockTransaction(&kHostInfoTransaction2);
+  net::TestCompletionCallback callback;
+
+  // Persist data for port 443.
+  net::HostPortPair host_port_pair1("https://www.google.com", 443);
+  scoped_ptr<net::QuicServerInfo> quic_server_info1(
+      new net::DiskCacheBasedQuicServerInfo(host_port_pair1,
+                                            cache.http_cache()));
+  quic_server_info1->Start();
+  int rv = quic_server_info1->WaitForDataReady(callback.callback());
+  EXPECT_EQ(net::OK, callback.GetResult(rv));
+
+  net::QuicServerInfo::State* state1 = quic_server_info1->mutable_state();
+  EXPECT_TRUE(state1->certs.empty());
+  const string server_config_a = "server_config_a";
+  const string source_address_token_a = "source_address_token_a";
+  const string server_config_sig_a = "server_config_sig_a";
+  const string cert_a = "cert_a";
+
+  state1->server_config = server_config_a;
+  state1->source_address_token = source_address_token_a;
+  state1->server_config_sig = server_config_sig_a;
+  state1->certs.push_back(cert_a);
+  quic_server_info1->Persist();
+
+  // Wait until Persist() does the work.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Persist data for port 80.
+  net::HostPortPair host_port_pair2("https://www.google.com", 80);
+  scoped_ptr<net::QuicServerInfo> quic_server_info2(
+      new net::DiskCacheBasedQuicServerInfo(host_port_pair2,
+                                            cache.http_cache()));
+  quic_server_info2->Start();
+  rv = quic_server_info2->WaitForDataReady(callback.callback());
+  EXPECT_EQ(net::OK, callback.GetResult(rv));
+
+  net::QuicServerInfo::State* state2 = quic_server_info2->mutable_state();
+  EXPECT_TRUE(state2->certs.empty());
+  const string server_config_b = "server_config_b";
+  const string source_address_token_b = "source_address_token_b";
+  const string server_config_sig_b = "server_config_sig_b";
+  const string cert_b = "cert_b";
+
+  state2->server_config = server_config_b;
+  state2->source_address_token = source_address_token_b;
+  state2->server_config_sig = server_config_sig_b;
+  state2->certs.push_back(cert_b);
+  quic_server_info2->Persist();
+
+  // Wait until Persist() does the work.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Verify the stored QuicServerInfo for port 443.
+  scoped_ptr<net::QuicServerInfo> quic_server_info(
+      new net::DiskCacheBasedQuicServerInfo(host_port_pair1,
+                                            cache.http_cache()));
+  quic_server_info->Start();
+  rv = quic_server_info->WaitForDataReady(callback.callback());
+  EXPECT_EQ(net::OK, callback.GetResult(rv));
+
+  const net::QuicServerInfo::State& state_a = quic_server_info->state();
+  EXPECT_TRUE(quic_server_info->IsDataReady());
+  EXPECT_EQ(server_config_a, state_a.server_config);
+  EXPECT_EQ(source_address_token_a, state_a.source_address_token);
+  EXPECT_EQ(server_config_sig_a, state_a.server_config_sig);
+  EXPECT_EQ(1U, state_a.certs.size());
+  EXPECT_EQ(cert_a, state_a.certs[0]);
+
+  // Verify the stored QuicServerInfo for port 80.
+  quic_server_info.reset(
+      new net::DiskCacheBasedQuicServerInfo(host_port_pair2,
+                                            cache.http_cache()));
+  quic_server_info->Start();
+  rv = quic_server_info->WaitForDataReady(callback.callback());
+  EXPECT_EQ(net::OK, callback.GetResult(rv));
+
+  const net::QuicServerInfo::State& state_b = quic_server_info->state();
+  EXPECT_TRUE(quic_server_info->IsDataReady());
+  EXPECT_EQ(server_config_b, state_b.server_config);
+  EXPECT_EQ(source_address_token_b, state_b.source_address_token);
+  EXPECT_EQ(server_config_sig_b, state_b.server_config_sig);
+  EXPECT_EQ(1U, state_b.certs.size());
+  EXPECT_EQ(cert_b, state_b.certs[0]);
+
+  RemoveMockTransaction(&kHostInfoTransaction2);
+  RemoveMockTransaction(&kHostInfoTransaction1);
 }
 
 }  // namespace