Fixes bugs in packet size computation in the creator and in the framer

net/quic/quic_framer.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_framer.h"
 
 #include "base/containers/hash_tables.h"
 #include "base/stl_util.h"
 #include "net/quic/crypto/crypto_framer.h"
 #include "net/quic/crypto/crypto_handshake_message.h"
@@ skipping 180 matching lines @@
   return kQuicFrameTypeSize + GetStreamIdSize(stream_id) +
       GetStreamOffsetSize(offset) +
       (no_stream_frame_length ? 0 : kQuicStreamPayloadLengthSize);
 }
 
 // static
 size_t QuicFramer::GetMinAckFrameSize(
     QuicVersion version,
     QuicSequenceNumberLength sequence_number_length,
     QuicSequenceNumberLength largest_observed_length) {
-  return kQuicFrameTypeSize + kQuicEntropyHashSize +
-      sequence_number_length + kQuicEntropyHashSize +
+  size_t len = kQuicFrameTypeSize + kQuicEntropyHashSize +
       largest_observed_length + kQuicDeltaTimeLargestObservedSize;
+  if (version <= QUIC_VERSION_15) {
+    len += sequence_number_length + kQuicEntropyHashSize;
+  }
+  return len;
 }
 
 // static
 size_t QuicFramer::GetStopWaitingFrameSize(
     QuicSequenceNumberLength sequence_number_length) {
   return kQuicFrameTypeSize + kQuicEntropyHashSize +
       sequence_number_length;
 }
 
 // static
@@ skipping 77 matching lines @@
     bool last_frame,
     InFecGroup is_in_fec_group,
     QuicSequenceNumberLength sequence_number_length) {
   if (frame.type == PADDING_FRAME) {
     // PADDING implies end of packet.
     return free_bytes;
   }
   size_t frame_len =
       ComputeFrameLength(frame, last_frame, is_in_fec_group,
                          sequence_number_length);
-  if (frame_len > free_bytes) {
-    // Only truncate the first frame in a packet, so if subsequent ones go
-    // over, stop including more frames.
-    if (!first_frame) {
-      return 0;
-    }
-    if (CanTruncate(quic_version_, frame, free_bytes)) {
-      // Truncate the frame so the packet will not exceed kMaxPacketSize.
-      // Note that we may not use every byte of the writer in this case.
-      DVLOG(1) << "Truncating large frame";
-      return free_bytes;
-    } else if (!FLAGS_quic_allow_oversized_packets_for_test) {
-      return 0;
-    }
+  if (frame_len <= free_bytes) {
+    // Frame fits within packet. Note that acks may be truncated.
+    return frame_len;
   }
+  // Only truncate the first frame in a packet, so if subsequent ones go
+  // over, stop including more frames.
+  if (!first_frame) {
+    return 0;
+  }
+  if (CanTruncate(quic_version_, frame, free_bytes)) {
+    // Truncate the frame so the packet will not exceed kMaxPacketSize.
+    // Note that we may not use every byte of the writer in this case.
+    DVLOG(1) << "Truncating large frame, free bytes: " << free_bytes;
+    return free_bytes;
+  }
+  if (!FLAGS_quic_allow_oversized_packets_for_test) {
+    return 0;
+  }
+  LOG(DFATAL) << "Packet size too small to fit frame.";
   return frame_len;
 }
 
 QuicFramer::AckFrameInfo::AckFrameInfo() : max_delta(0) { }
 
 QuicFramer::AckFrameInfo::~AckFrameInfo() { }
 
 QuicPacketEntropyHash QuicFramer::GetPacketEntropyHash(
     const QuicPacketHeader& header) const {
   return header.entropy_flag << (header.packet_sequence_number % 8);
@@ skipping 1485 matching lines @@
   AckFrameInfo ack_info = GetAckFrameInfo(ack);
   QuicSequenceNumberLength largest_observed_length =
       GetMinSequenceNumberLength(ack.received_info.largest_observed);
   QuicSequenceNumberLength missing_sequence_number_length =
       GetMinSequenceNumberLength(ack_info.max_delta);
 
   size_t ack_size = GetMinAckFrameSize(quic_version_,
                                        sequence_number_length,
                                        largest_observed_length);
   if (!ack_info.nack_ranges.empty()) {
-    ack_size += kNumberOfMissingPacketsSize  + kNumberOfRevivedPacketsSize;
-    ack_size += ack_info.nack_ranges.size() *
+    ack_size += kNumberOfNackRangesSize  + kNumberOfRevivedPacketsSize;
+    ack_size += min(ack_info.nack_ranges.size(), kMaxNackRanges) *
       (missing_sequence_number_length + PACKET_1BYTE_SEQUENCE_NUMBER);
-    ack_size +=
-        ack.received_info.revived_packets.size() * largest_observed_length;
+    ack_size += min(ack.received_info.revived_packets.size(),
+                    kMaxRevivedPackets) * largest_observed_length;
   }
   return ack_size;
 }
 
 size_t QuicFramer::ComputeFrameLength(
     const QuicFrame& frame,
     bool last_frame_in_packet,
     InFecGroup is_in_fec_group,
     QuicSequenceNumberLength sequence_number_length) {
   switch (frame.type) {
@@ skipping 185 matching lines @@
     QuicDataWriter* writer) {
   AckFrameInfo ack_info = GetAckFrameInfo(frame);
   QuicPacketSequenceNumber ack_largest_observed =
       frame.received_info.largest_observed;
   QuicSequenceNumberLength largest_observed_length =
       GetMinSequenceNumberLength(ack_largest_observed);
   QuicSequenceNumberLength missing_sequence_number_length =
       GetMinSequenceNumberLength(ack_info.max_delta);
   // Determine whether we need to truncate ranges.
   size_t available_range_bytes = writer->capacity() - writer->length() -
+      kNumberOfRevivedPacketsSize - kNumberOfNackRangesSize -
       GetMinAckFrameSize(quic_version_,
                          header.public_header.sequence_number_length,
-                         largest_observed_length) - kNumberOfRevivedPacketsSize;
+                         largest_observed_length);
   size_t max_num_ranges = available_range_bytes /
       (missing_sequence_number_length + PACKET_1BYTE_SEQUENCE_NUMBER);
-  max_num_ranges =
-      min(static_cast<size_t>(numeric_limits<uint8>::max()), max_num_ranges);
+  max_num_ranges = min(kMaxNackRanges, max_num_ranges);
   bool truncated = ack_info.nack_ranges.size() > max_num_ranges;
   DVLOG_IF(1, truncated) << "Truncating ack from "
                          << ack_info.nack_ranges.size() << " ranges to "
                          << max_num_ranges;
-
   // Write out the type byte by setting the low order bits and doing shifts
   // to make room for the next bit flags to be set.
   // Whether there are any nacks.
   uint8 type_byte = ack_info.nack_ranges.empty() ? 0 : kQuicHasNacksMask;
 
   // truncating bit.
   type_byte <<= kQuicAckTruncatedShift;
   type_byte |= truncated ? kQuicAckTruncatedMask : 0;
 
   // Largest observed sequence number length.
@@ skipping 81 matching lines @@
       return false;
     }
     // Subtract 1 so a missing_delta of 0 means an adjacent range.
     last_sequence_written = ack_iter->first - 1;
     ++num_ranges_written;
   }
   DCHECK_EQ(num_missing_ranges, num_ranges_written);
 
   // Append revived packets.
   // If not all the revived packets fit, only mention the ones that do.
-  uint8 num_revived_packets =
-      min(received_info.revived_packets.size(),
-          static_cast<size_t>(numeric_limits<uint8>::max()));
+  uint8 num_revived_packets = min(received_info.revived_packets.size(),
+                                  kMaxRevivedPackets);
   num_revived_packets = min(
       static_cast<size_t>(num_revived_packets),
       (writer->capacity() - writer->length()) / largest_observed_length);
   if (!writer->WriteBytes(&num_revived_packets, 1)) {
     return false;
   }
 
   SequenceNumberSet::const_iterator iter =
       received_info.revived_packets.begin();
   for (int i = 0; i < num_revived_packets; ++i, ++iter) {
@@ skipping 195 matching lines @@
 
 bool QuicFramer::RaiseError(QuicErrorCode error) {
   DVLOG(1) << "Error detail: " << detailed_error_;
   set_error(error);
   visitor_->OnError(this);
   reader_.reset(NULL);
   return false;
 }
 
 }  // namespace net