commit 7ffadb836dade7685f6ba20bf804e708ceb7441e
author bnc <bnc@google.com> Tue Apr 07 10:01:30 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Apr 07 10:02:01 2020 -0700
tree 485364239d953fb8d3a3a9b9d13cd4eb136307e3
parent c1521a02d2ff680812b71992816a77ca63cfe700

gfe-relnote: Do not include PUSH_PROMISE header block length when calling QuicSpdyStream::OnStreamHeaderList().  Not protected.

Before this CL, PUSH_PROMISE header block size is incorrectly added to
headers_payload_length_ or trailers_payload_length_, which is then passed to
QuicSpdyStream::OnStreamHeaderList().  This CL instead saves the payload_length
argument of OnHeadersFrameStart().

The |frame_len| argument of OnStreamHeaderList() is only used in a VLOG statement in QuicEventManagerStream::OnInitialHeadersComplete() and for incrementing |header_bytes_read_| in QuicSpdyClientStream::OnInitialHeadersComplete().  QuicSpdyClientStream::header_bytes_read() is only called from QuicTestClient.  Therefore production code is not affected by this change.

PiperOrigin-RevId: 305281258
Change-Id: I65be9886574f849bf25fe0f1e5c05ee2c6a453a9

quic/core/http/quic_spdy_stream.cc

diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index c376d01..06962b0 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -192,7 +192,6 @@
       headers_decompressed_(false),
       header_list_size_limit_exceeded_(false),
       headers_payload_length_(0),
-      trailers_payload_length_(0),
       trailers_decompressed_(false),
       trailers_consumed_(false),
       http_decoder_visitor_(std::make_unique<HttpDecoderVisitor>(this)),
@@ -229,7 +228,6 @@
       headers_decompressed_(false),
       header_list_size_limit_exceeded_(false),
       headers_payload_length_(0),
-      trailers_payload_length_(0),
       trailers_decompressed_(false),
       trailers_consumed_(false),
       http_decoder_visitor_(std::make_unique<HttpDecoderVisitor>(this)),
@@ -569,10 +567,7 @@
       debug_visitor->OnHeadersDecoded(id(), headers);
     }
 
-    const QuicByteCount frame_length = headers_decompressed_
-                                           ? trailers_payload_length_
-                                           : headers_payload_length_;
-    OnStreamHeaderList(/* fin = */ false, frame_length, headers);
+    OnStreamHeaderList(/* fin = */ false, headers_payload_length_, headers);
   } else {
     if (debug_visitor) {
       debug_visitor->OnPushPromiseDecoded(id(), promised_stream_id, headers);
@@ -962,6 +957,8 @@
                                                            payload_length);
   }
 
+  headers_payload_length_ = payload_length;
+
   if (trailers_decompressed_) {
     stream_delegate()->OnStreamError(
         QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
@@ -983,15 +980,6 @@
   DCHECK(VersionUsesHttp3(transport_version()));
   DCHECK(qpack_decoded_headers_accumulator_);
 
-  // TODO(b/152518220): Save |payload_length| argument of OnHeadersFrameStart()
-  // instead of accumulating payload length in |headers_payload_length_| or
-  // |trailers_payload_length_|.
-  if (headers_decompressed_) {
-    trailers_payload_length_ += payload.length();
-  } else {
-    headers_payload_length_ += payload.length();
-  }
-
   qpack_decoded_headers_accumulator_->Decode(payload);
 
   // |qpack_decoded_headers_accumulator_| is reset if an error is detected.
@@ -1040,7 +1028,7 @@
         id(), push_id, header_block_length);
   }
 
-  // TODO(renjietang): Check max push id and handle errors.
+  // TODO(b/151749109): Check max push id and handle errors.
   spdy_session_->OnPushPromise(id(), push_id);
   sequencer()->MarkConsumed(body_manager_.OnNonBody(push_id_length));