commit 334f02301fe0adb1a9656931205bf1ffa0df79ad
author dschinazi <dschinazi@google.com> Wed May 29 16:08:53 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Fri May 31 15:28:24 2019 -0700
tree 066792e1a45a410c750a93cbf85e068de9ff6c43
parent baea59c31905f28eee449772b6ab4347d47ca64e

Fix expected_server_connection_id_length on client

When should_update_expected_server_connection_id_length is true, ProcessAndValidateIetfConnectionIdLength, expected_server_connection_id_length will update expected_server_connection_id_length. Unfortunately that code was wrong on the client as it only looked at the destination connection ID length instead of the source on the client. This CL fixes that error and adds a test. This wasn't an issue in production because we've only ever used should_update_expected_server_connection_id_length on the server.

gfe-relnote: client-only change, not flag protected
PiperOrigin-RevId: 250586674
Change-Id: Iccb6776bc9aa41350f1e80ecddea76ba8aa4eb30

quic/core/quic_framer.cc

diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 7e34b3b..328e453 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -2624,6 +2624,7 @@
 bool QuicFramer::ProcessAndValidateIetfConnectionIdLength(
     QuicDataReader* reader,
     ParsedQuicVersion version,
+    Perspective perspective,
     bool should_update_expected_server_connection_id_length,
     uint8_t* expected_server_connection_id_length,
     uint8_t* destination_connection_id_length,
@@ -2639,17 +2640,20 @@
   if (dcil != 0) {
     dcil += kConnectionIdLengthAdjustment;
   }
-  if (should_update_expected_server_connection_id_length &&
-      *expected_server_connection_id_length != dcil) {
-    QUIC_DVLOG(1) << "Updating expected_server_connection_id_length: "
-                  << static_cast<int>(*expected_server_connection_id_length)
-                  << " -> " << static_cast<int>(dcil);
-    *expected_server_connection_id_length = dcil;
-  }
   uint8_t scil = connection_id_lengths_byte & kSourceConnectionIdLengthMask;
   if (scil != 0) {
     scil += kConnectionIdLengthAdjustment;
   }
+  if (should_update_expected_server_connection_id_length) {
+    uint8_t server_connection_id_length =
+        perspective == Perspective::IS_SERVER ? dcil : scil;
+    if (*expected_server_connection_id_length != server_connection_id_length) {
+      QUIC_DVLOG(1) << "Updating expected_server_connection_id_length: "
+                    << static_cast<int>(*expected_server_connection_id_length)
+                    << " -> " << static_cast<int>(server_connection_id_length);
+      *expected_server_connection_id_length = server_connection_id_length;
+    }
+  }
   if (!should_update_expected_server_connection_id_length &&
       (dcil != *destination_connection_id_length ||
        scil != *source_connection_id_length) &&
@@ -2683,7 +2687,7 @@
           : 0;
   if (header->form == IETF_QUIC_LONG_HEADER_PACKET) {
     if (!ProcessAndValidateIetfConnectionIdLength(
-            reader, header->version,
+            reader, header->version, perspective_,
             should_update_expected_server_connection_id_length_,
             &expected_server_connection_id_length_,
             &destination_connection_id_length, &source_connection_id_length,
@@ -6099,6 +6103,7 @@
     uint8_t unused_expected_server_connection_id_length = 0;
     if (!ProcessAndValidateIetfConnectionIdLength(
             &reader, ParseQuicVersionLabel(*version_label),
+            Perspective::IS_SERVER,
             /*should_update_expected_server_connection_id_length=*/true,
             &unused_expected_server_connection_id_length,
             destination_connection_id_length,
@@ -6247,7 +6252,7 @@
   uint8_t expected_server_connection_id_length = 0,
           destination_connection_id_length = 0, source_connection_id_length = 0;
   if (!ProcessAndValidateIetfConnectionIdLength(
-          &reader, UnsupportedQuicVersion(),
+          &reader, UnsupportedQuicVersion(), Perspective::IS_CLIENT,
           /*should_update_expected_server_connection_id_length=*/true,
           &expected_server_connection_id_length,
           &destination_connection_id_length, &source_connection_id_length,