commit 24e7ae8eb8ac4978abcab717a49084685b728eb4
author dschinazi <dschinazi@google.com> Thu Jul 11 13:07:41 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Jul 11 15:48:42 2019 -0700
tree d4bc6be691d48db6c7a92db13696cc68e681e075
parent 0e9980b4043b54ec5b9eef48161662eb0d2975c2

Fix QUIC version used when computing ALPN

The ALPN should be based on the QUIC version that is currently in use, not the first one we tried prior to version negotiation. Running the newly added version negotiation end-to-end test with TLS enabled is what found the issue.

gfe-relnote: fix ALPN, protected by flag quic_supports_tls_handshake
PiperOrigin-RevId: 257666341
Change-Id: I4f1619cf4923fae01443ed4c89450df0be78cd39

quic/core/http/end_to_end_test.cc

diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 1f8d6cf..e05852b 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -598,6 +598,12 @@
             client_->client()->GetNumSentClientHellos());
 }
 
+TEST_P(EndToEndTestWithTls, SimpleRequestResponse) {
+  ASSERT_TRUE(Initialize());
+  EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
+  EXPECT_EQ("200", client_->response_headers()->find(":status")->second);
+}
+
 TEST_P(EndToEndTest, SimpleRequestResponseForcedVersionNegotiation) {
   client_supported_versions_.insert(client_supported_versions_.begin(),
                                     QuicVersionReservedForNegotiation());
@@ -610,6 +616,16 @@
   EXPECT_EQ(3, client_->client()->GetNumSentClientHellos());
 }
 
+TEST_P(EndToEndTestWithTls, ForcedVersionNegotiation) {
+  client_supported_versions_.insert(client_supported_versions_.begin(),
+                                    QuicVersionReservedForNegotiation());
+  ASSERT_TRUE(Initialize());
+  ASSERT_TRUE(ServerSendsVersionNegotiation());
+
+  EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
+  EXPECT_EQ("200", client_->response_headers()->find(":status")->second);
+}
+
 TEST_P(EndToEndTest, SimpleRequestResponseZeroConnectionID) {
   QuicConnectionId connection_id = QuicUtils::CreateZeroConnectionId(
       GetParam().negotiated_version.transport_version);

quic/core/tls_client_handshaker.cc

diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index 7024ff8..20d9ff6 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -84,8 +84,7 @@
     return false;
   }
 
-  std::string alpn_string =
-      AlpnForVersion(session()->supported_versions().front());
+  std::string alpn_string = AlpnForVersion(session()->connection()->version());
   if (alpn_string.length() > std::numeric_limits<uint8_t>::max()) {
     QUIC_BUG << "ALPN too long: '" << alpn_string << "'";
     CloseConnection(QUIC_HANDSHAKE_FAILED, "ALPN too long");
@@ -273,7 +272,7 @@
     std::string received_alpn_string(reinterpret_cast<const char*>(alpn_data),
                                      alpn_length);
     std::string sent_alpn_string =
-        AlpnForVersion(session()->supported_versions().front());
+        AlpnForVersion(session()->connection()->version());
     if (received_alpn_string != sent_alpn_string) {
       QUIC_LOG(ERROR) << "Client: received mismatched ALPN '"
                       << received_alpn_string << "', expected '"