Fix a QUIC bug where previously undecryptable packets were not decrypted

net/quic/quic_connection.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 "net/quic/quic_connection.h"
 
 #include <string.h>
 #include <sys/types.h>
 #include <algorithm>
 #include <iterator>
@@ skipping 149 matching lines @@
       helper_(helper),
       writer_(writer),
       encryption_level_(ENCRYPTION_NONE),
       clock_(helper->GetClock()),
       random_generator_(helper->GetRandomGenerator()),
       guid_(guid),
       peer_address_(address),
       largest_seen_packet_with_ack_(0),
       pending_version_negotiation_packet_(false),
       received_packet_manager_(kTCP),
+      ack_queued_(false),
       ack_alarm_(helper->CreateAlarm(new AckAlarm(this))),
       retransmission_alarm_(helper->CreateAlarm(new RetransmissionAlarm(this))),
       send_alarm_(helper->CreateAlarm(new SendAlarm(this))),
       resume_writes_alarm_(helper->CreateAlarm(new SendAlarm(this))),
       timeout_alarm_(helper->CreateAlarm(new TimeoutAlarm(this))),
       debug_visitor_(NULL),
       packet_creator_(guid_, &framer_, random_generator_, is_server),
       packet_generator_(this, NULL, &packet_creator_),
       idle_network_timeout_(
           QuicTime::Delta::FromSeconds(kDefaultInitialTimeoutSecs)),
@@ skipping 449 matching lines @@
   DVLOG(1) << ENDPOINT << (last_packet_revived_ ? "Revived" : "Got")
              << " packet " << last_header_.packet_sequence_number
              << " with " << last_ack_frames_.size() << " acks, "
              << last_congestion_frames_.size() << " congestions, "
              << last_goaway_frames_.size() << " goaways, "
              << last_rst_frames_.size() << " rsts, "
              << last_close_frames_.size() << " closes, "
              << last_stream_frames_.size()
              << " stream frames for " << last_header_.public_header.guid;
 
-  // Must called before ack processing, because processing acks removes entries
-  // from unacket_packets_, increasing the least_unacked.
-  const bool last_packet_should_instigate_ack = ShouldLastPacketInstigateAck();
-
-  // If the incoming packet was missing, send an ack immediately.
-  bool send_ack_immediately = received_packet_manager_.IsMissing(
-      last_header_.packet_sequence_number);
+  MaybeQueueAck();
 
   // Discard the packet if the visitor fails to process the stream frames.
   if (!last_stream_frames_.empty() &&
       !visitor_->OnStreamFrames(last_stream_frames_)) {
     return;
   }
 
   received_packet_manager_.RecordPacketReceived(last_size_,
                                                 last_header_,
                                                 time_of_last_received_packet_,
@@ skipping 17 matching lines @@
     sent_packet_manager_.OnIncomingQuicCongestionFeedbackFrame(
         last_congestion_frames_[i], time_of_last_received_packet_);
   }
   if (!last_close_frames_.empty()) {
     CloseConnection(last_close_frames_[0].error_code, true);
     DCHECK(!connected_);
   }
 
   // If there are new missing packets to report, send an ack immediately.
   if (received_packet_manager_.HasNewMissingPackets()) {
-    send_ack_immediately = true;
+    ack_queued_ = true;
+    ack_alarm_->Cancel();
   }
 
-  MaybeSendInResponseToPacket(send_ack_immediately,
-                              last_packet_should_instigate_ack);
+  ClearLastFrames();
+}
 
-  ClearLastFrames();
+void QuicConnection::MaybeQueueAck() {
+  // If the incoming packet was missing, send an ack immediately.
+  ack_queued_ = received_packet_manager_.IsMissing(
+      last_header_.packet_sequence_number);
+
+  // ShouldLastPacketInstigateAck must called before ack processing, because
+  // processing acks removes entries from unacket_packets_, increasing the
+  // least_unacked.
+  if (!ack_queued_ && ShouldLastPacketInstigateAck()) {
+    if (ack_alarm_->IsSet()) {
+      ack_queued_ = true;
+    } else {
+      ack_alarm_->Set(clock_->ApproximateNow().Add(
+          sent_packet_manager_.DelayedAckTime()));
+      DVLOG(1) << "Ack timer set; next packet or timer will trigger ACK.";
+    }
+  }
+
+  if (ack_queued_) {
+    ack_alarm_->Cancel();
+  }
 }
 
 void QuicConnection::ClearLastFrames() {
   last_stream_frames_.clear();
   last_goaway_frames_.clear();
   last_rst_frames_.clear();
   last_ack_frames_.clear();
   last_congestion_frames_.clear();
 }
 
@@ skipping 10 matching lines @@
   return new QuicCongestionFeedbackFrame(outgoing_congestion_feedback_);
 }
 
 bool QuicConnection::ShouldLastPacketInstigateAck() {
   if (!last_stream_frames_.empty() ||
       !last_goaway_frames_.empty() ||
       !last_rst_frames_.empty()) {
     return true;
   }
 
-  // If the peer is still waiting for a packet that we are no
-  // longer planning to send, we should send an ack to raise
-  // the high water mark.
+  // If the peer is still waiting for a packet that we are no longer planning to
+  // send, send an ack to raise the high water mark.
   if (!last_ack_frames_.empty() &&
       !last_ack_frames_.back().received_info.missing_packets.empty()) {
     return sent_packet_manager_.GetLeastUnackedSentPacket() >
         *last_ack_frames_.back().received_info.missing_packets.begin();
   }
   return false;
 }
 
-void QuicConnection::MaybeSendInResponseToPacket(
-    bool send_ack_immediately,
-    bool last_packet_should_instigate_ack) {
-  // |include_ack| is false since we decide about ack bundling below.
+void QuicConnection::MaybeSendInResponseToPacket() {
+  if (!connected_) {
+    return;
+  }
   ScopedPacketBundler bundler(this, false);
-
-  if (last_packet_should_instigate_ack) {
-    // In general, we ack every second packet.  When we don't ack the first
-    // packet, we set the delayed ack alarm.  Thus, if the ack alarm is set
-    // then we know this is the second packet, and we should send an ack.
-    if (send_ack_immediately || ack_alarm_->IsSet()) {
-      SendAck();
-      DCHECK(!ack_alarm_->IsSet());
-    } else {
-      ack_alarm_->Set(clock_->ApproximateNow().Add(
-          sent_packet_manager_.DelayedAckTime()));
-      DVLOG(1) << "Ack timer set; next packet or timer will trigger ACK.";
-    }
+  if (ack_queued_) {
+    SendAck();
   }
 
-  if (!last_ack_frames_.empty()) {
-    // Now the we have received an ack, we might be able to send packets which
-    // are queued locally, or drain streams which are blocked.
-    QuicTime::Delta delay = sent_packet_manager_.TimeUntilSend(
-        time_of_last_received_packet_, NOT_RETRANSMISSION,
-        HAS_RETRANSMITTABLE_DATA, NOT_HANDSHAKE);
-    if (delay.IsZero()) {
-      send_alarm_->Cancel();
-      WriteIfNotBlocked();
-    } else if (!delay.IsInfinite()) {
-      send_alarm_->Cancel();
-      send_alarm_->Set(time_of_last_received_packet_.Add(delay));
-    }
+  // Now that we have received an ack, we might be able to send packets which
+  // are queued locally, or drain streams which are blocked.
+  QuicTime::Delta delay = sent_packet_manager_.TimeUntilSend(
+      time_of_last_received_packet_, NOT_RETRANSMISSION,
+      HAS_RETRANSMITTABLE_DATA, NOT_HANDSHAKE);
+  if (delay.IsZero()) {
+    send_alarm_->Cancel();
+    WriteIfNotBlocked();
+  } else if (!delay.IsInfinite()) {
+    send_alarm_->Cancel();
+    send_alarm_->Set(time_of_last_received_packet_.Add(delay));
   }
 }
 
 void QuicConnection::SendVersionNegotiationPacket() {
   scoped_ptr<QuicEncryptedPacket> version_packet(
       packet_creator_.SerializeVersionNegotiationPacket(
           framer_.supported_versions()));
   // TODO(satyamshekhar): implement zero server state negotiation.
   WriteResult result =
       writer_->WritePacket(version_packet->data(), version_packet->length(),
@@ skipping 92 matching lines @@
     // because the CHLO or SHLO packet was lost.
     if (encryption_level_ != ENCRYPTION_FORWARD_SECURE &&
         framer_.error() == QUIC_DECRYPTION_FAILURE &&
         undecryptable_packets_.size() < kMaxUndecryptablePackets) {
       QueueUndecryptablePacket(packet);
     }
     DVLOG(1) << ENDPOINT << "Unable to process packet.  Last packet processed: "
              << last_header_.packet_sequence_number;
     return;
   }
+
   MaybeProcessUndecryptablePackets();
   MaybeProcessRevivedPacket();
+  MaybeSendInResponseToPacket();
 }
 
 bool QuicConnection::OnCanWrite() {
   DCHECK(!writer_->IsWriteBlocked());
 
   WriteQueuedPackets();
   WritePendingRetransmissions();
 
   IsHandshake pending_handshake = visitor_->HasPendingHandshake() ?
       IS_HANDSHAKE : NOT_HANDSHAKE;
@@ skipping 773 matching lines @@
   // If we changed the generator's batch state, restore original batch state.
   if (!already_in_batch_mode_) {
     DVLOG(1) << "Leaving Batch Mode.";
     connection_->packet_generator_.FinishBatchOperations();
   }
   DCHECK_EQ(already_in_batch_mode_,
             connection_->packet_generator_.InBatchMode());
 }
 
 }  // namespace net