Add `payload_length` argument to SpdyFramerVisitorInterface::OnHeaders(). Also add some comments to analogous SpdyFramerVisitorInterface::OnDataFrameHeader(), which also has a length field. The goal is to use the new `payload_length` argument in recording the header bytes for Dapper annotations. PiperOrigin-RevId: 449842231
diff --git a/quiche/http2/adapter/event_forwarder.cc b/quiche/http2/adapter/event_forwarder.cc index b5c0389..a18a2cd 100644 --- a/quiche/http2/adapter/event_forwarder.cc +++ b/quiche/http2/adapter/event_forwarder.cc
@@ -116,12 +116,13 @@ return false; } -void EventForwarder::OnHeaders(spdy::SpdyStreamId stream_id, bool has_priority, +void EventForwarder::OnHeaders(spdy::SpdyStreamId stream_id, + size_t payload_length, bool has_priority, int weight, spdy::SpdyStreamId parent_stream_id, bool exclusive, bool fin, bool end) { if (can_forward_()) { - receiver_.OnHeaders(stream_id, has_priority, weight, parent_stream_id, - exclusive, fin, end); + receiver_.OnHeaders(stream_id, payload_length, has_priority, weight, + parent_stream_id, exclusive, fin, end); } }
diff --git a/quiche/http2/adapter/event_forwarder.h b/quiche/http2/adapter/event_forwarder.h index 1682263..5f93db0 100644 --- a/quiche/http2/adapter/event_forwarder.h +++ b/quiche/http2/adapter/event_forwarder.h
@@ -46,7 +46,8 @@ void OnGoAway(spdy::SpdyStreamId last_accepted_stream_id, spdy::SpdyErrorCode error_code) override; bool OnGoAwayFrameData(const char* goaway_data, size_t len) override; - void OnHeaders(spdy::SpdyStreamId stream_id, bool has_priority, int weight, + void OnHeaders(spdy::SpdyStreamId stream_id, size_t payload_length, + bool has_priority, int weight, spdy::SpdyStreamId parent_stream_id, bool exclusive, bool fin, bool end) override; void OnWindowUpdate(spdy::SpdyStreamId stream_id,
diff --git a/quiche/http2/adapter/event_forwarder_test.cc b/quiche/http2/adapter/event_forwarder_test.cc index eab6516..c3680c8 100644 --- a/quiche/http2/adapter/event_forwarder_test.cc +++ b/quiche/http2/adapter/event_forwarder_test.cc
@@ -89,11 +89,12 @@ EXPECT_CALL(receiver, OnGoAwayFrameData(some_data.data(), some_data.size())); event_forwarder.OnGoAwayFrameData(some_data.data(), some_data.size()); - EXPECT_CALL( - receiver, - OnHeaders(stream_id, /*has_priority=*/false, /*weight=*/42, stream_id + 2, - /*exclusive=*/false, /*fin=*/true, /*end=*/true)); - event_forwarder.OnHeaders(stream_id, /*has_priority=*/false, /*weight=*/42, + EXPECT_CALL(receiver, + OnHeaders(stream_id, /*payload_length=*/1234, + /*has_priority=*/false, /*weight=*/42, stream_id + 2, + /*exclusive=*/false, /*fin=*/true, /*end=*/true)); + event_forwarder.OnHeaders(stream_id, /*payload_length=*/1234, + /*has_priority=*/false, /*weight=*/42, stream_id + 2, /*exclusive=*/false, /*fin=*/true, /*end=*/true); @@ -186,7 +187,8 @@ event_forwarder.OnGoAwayFrameData(some_data.data(), some_data.size()); EXPECT_CALL(receiver, OnHeaders).Times(0); - event_forwarder.OnHeaders(stream_id, /*has_priority=*/false, /*weight=*/42, + event_forwarder.OnHeaders(stream_id, /*payload_length=*/1234, + /*has_priority=*/false, /*weight=*/42, stream_id + 2, /*exclusive=*/false, /*fin=*/true, /*end=*/true);
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc index c27cee2..77fff36 100644 --- a/quiche/http2/adapter/oghttp2_session.cc +++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -1338,7 +1338,8 @@ } void OgHttp2Session::OnHeaders(spdy::SpdyStreamId stream_id, - bool /*has_priority*/, int /*weight*/, + size_t /*payload_length*/, bool /*has_priority*/, + int /*weight*/, spdy::SpdyStreamId /*parent_stream_id*/, bool /*exclusive*/, bool fin, bool /*end*/) { if (stream_id % 2 == 0) {
diff --git a/quiche/http2/adapter/oghttp2_session.h b/quiche/http2/adapter/oghttp2_session.h index 458d0da..5bc854b 100644 --- a/quiche/http2/adapter/oghttp2_session.h +++ b/quiche/http2/adapter/oghttp2_session.h
@@ -180,7 +180,8 @@ void OnGoAway(spdy::SpdyStreamId last_accepted_stream_id, spdy::SpdyErrorCode error_code) override; bool OnGoAwayFrameData(const char* goaway_data, size_t len) override; - void OnHeaders(spdy::SpdyStreamId stream_id, bool has_priority, int weight, + void OnHeaders(spdy::SpdyStreamId stream_id, size_t payload_length, + bool has_priority, int weight, spdy::SpdyStreamId parent_stream_id, bool exclusive, bool fin, bool end) override; void OnWindowUpdate(spdy::SpdyStreamId stream_id,
diff --git a/quiche/http2/core/http2_trace_logging.cc b/quiche/http2/core/http2_trace_logging.cc index f6eef16..10ff163 100644 --- a/quiche/http2/core/http2_trace_logging.cc +++ b/quiche/http2/core/http2_trace_logging.cc
@@ -257,16 +257,17 @@ return wrapped_->OnGoAwayFrameData(goaway_data, len); } -void Http2TraceLogger::OnHeaders(SpdyStreamId stream_id, bool has_priority, - int weight, SpdyStreamId parent_stream_id, - bool exclusive, bool fin, bool end) { +void Http2TraceLogger::OnHeaders(SpdyStreamId stream_id, size_t payload_length, + bool has_priority, int weight, + SpdyStreamId parent_stream_id, bool exclusive, + bool fin, bool end) { HTTP2_TRACE_LOG(perspective_, is_enabled_) << "OnHeaders:" << FORMAT_ARG(connection_id_) << FORMAT_ARG(stream_id) - << FORMAT_ARG(has_priority) << FORMAT_INT_ARG(weight) - << FORMAT_ARG(parent_stream_id) << FORMAT_ARG(exclusive) - << FORMAT_ARG(fin) << FORMAT_ARG(end); - wrapped_->OnHeaders(stream_id, has_priority, weight, parent_stream_id, - exclusive, fin, end); + << FORMAT_ARG(payload_length) << FORMAT_ARG(has_priority) + << FORMAT_INT_ARG(weight) << FORMAT_ARG(parent_stream_id) + << FORMAT_ARG(exclusive) << FORMAT_ARG(fin) << FORMAT_ARG(end); + wrapped_->OnHeaders(stream_id, payload_length, has_priority, weight, + parent_stream_id, exclusive, fin, end); } void Http2TraceLogger::OnWindowUpdate(SpdyStreamId stream_id,
diff --git a/quiche/http2/core/http2_trace_logging.h b/quiche/http2/core/http2_trace_logging.h index 6f982dc..1bde7ee 100644 --- a/quiche/http2/core/http2_trace_logging.h +++ b/quiche/http2/core/http2_trace_logging.h
@@ -71,9 +71,9 @@ void OnGoAway(SpdyStreamId last_accepted_stream_id, SpdyErrorCode error_code) override; bool OnGoAwayFrameData(const char* goaway_data, size_t len) override; - void OnHeaders(SpdyStreamId stream_id, bool has_priority, int weight, - SpdyStreamId parent_stream_id, bool exclusive, bool fin, - bool end) override; + void OnHeaders(SpdyStreamId stream_id, size_t payload_length, + bool has_priority, int weight, SpdyStreamId parent_stream_id, + bool exclusive, bool fin, bool end) override; void OnWindowUpdate(SpdyStreamId stream_id, int delta_window_size) override; void OnPushPromise(SpdyStreamId stream_id, SpdyStreamId promised_stream_id, bool end) override;
diff --git a/quiche/quic/core/http/quic_headers_stream_test.cc b/quiche/quic/core/http/quic_headers_stream_test.cc index f82a0dd..9b8ffb4 100644 --- a/quiche/quic/core/http/quic_headers_stream_test.cc +++ b/quiche/quic/core/http/quic_headers_stream_test.cc
@@ -101,9 +101,9 @@ (SpdyStreamId last_accepted_stream_id, SpdyErrorCode error_code), (override)); MOCK_METHOD(void, OnHeaders, - (SpdyStreamId stream_id, bool has_priority, int weight, - SpdyStreamId parent_stream_id, bool exclusive, bool fin, - bool end), + (SpdyStreamId stream_id, size_t payload_length, bool has_priority, + int weight, SpdyStreamId parent_stream_id, bool exclusive, + bool fin, bool end), (override)); MOCK_METHOD(void, OnWindowUpdate, (SpdyStreamId stream_id, int delta_window_size), (override)); @@ -281,17 +281,20 @@ // Parse the outgoing data and check that it matches was was written. if (is_request) { - EXPECT_CALL(visitor_, - OnHeaders(stream_id, kHasPriority, - Spdy3PriorityToHttp2Weight(priority), - /*parent_stream_id=*/0, - /*exclusive=*/false, fin, kFrameComplete)); + EXPECT_CALL( + visitor_, + OnHeaders(stream_id, saved_data_.length() - spdy::kFrameHeaderSize, + kHasPriority, Spdy3PriorityToHttp2Weight(priority), + /*parent_stream_id=*/0, + /*exclusive=*/false, fin, kFrameComplete)); } else { - EXPECT_CALL(visitor_, - OnHeaders(stream_id, !kHasPriority, - /*weight=*/0, - /*parent_stream_id=*/0, - /*exclusive=*/false, fin, kFrameComplete)); + EXPECT_CALL( + visitor_, + OnHeaders(stream_id, saved_data_.length() - spdy::kFrameHeaderSize, + !kHasPriority, + /*weight=*/0, + /*parent_stream_id=*/0, + /*exclusive=*/false, fin, kFrameComplete)); } headers_handler_ = std::make_unique<RecordingHeadersHandler>(); EXPECT_CALL(visitor_, OnHeaderFrameStart(stream_id))
diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc index af40f36..f0aa6d5 100644 --- a/quiche/quic/core/http/quic_spdy_session.cc +++ b/quiche/quic/core/http/quic_spdy_session.cc
@@ -318,8 +318,9 @@ QUIC_INVALID_HEADERS_STREAM_DATA); } - void OnHeaders(SpdyStreamId stream_id, bool has_priority, int weight, - SpdyStreamId /* parent_stream_id */, bool /* exclusive */, + void OnHeaders(SpdyStreamId stream_id, size_t /*payload_length*/, + bool has_priority, int weight, + SpdyStreamId /*parent_stream_id*/, bool /*exclusive*/, bool fin, bool /*end*/) override { if (!session_->IsConnected()) { return;
diff --git a/quiche/spdy/core/http2_frame_decoder_adapter.cc b/quiche/spdy/core/http2_frame_decoder_adapter.cc index c783a5d..780342e 100644 --- a/quiche/spdy/core/http2_frame_decoder_adapter.cc +++ b/quiche/spdy/core/http2_frame_decoder_adapter.cc
@@ -436,7 +436,8 @@ } on_headers_called_ = true; ReportReceiveCompressedFrame(header); - visitor()->OnHeaders(header.stream_id, kNotHasPriorityFields, + visitor()->OnHeaders(header.stream_id, header.payload_length, + kNotHasPriorityFields, 0, // priority 0, // parent_stream_id false, // exclusive @@ -460,10 +461,10 @@ << " priority:" << priority << " frame_header:" << frame_header_; return; } - visitor()->OnHeaders(frame_header_.stream_id, kHasPriorityFields, - priority.weight, priority.stream_dependency, - priority.is_exclusive, frame_header_.IsEndStream(), - frame_header_.IsEndHeaders()); + visitor()->OnHeaders( + frame_header_.stream_id, frame_header_.payload_length, kHasPriorityFields, + priority.weight, priority.stream_dependency, priority.is_exclusive, + frame_header_.IsEndStream(), frame_header_.IsEndHeaders()); CommonStartHpackBlock(); }
diff --git a/quiche/spdy/core/http2_frame_decoder_adapter.h b/quiche/spdy/core/http2_frame_decoder_adapter.h index cc71433..fbdfd5b 100644 --- a/quiche/spdy/core/http2_frame_decoder_adapter.h +++ b/quiche/spdy/core/http2_frame_decoder_adapter.h
@@ -376,9 +376,12 @@ virtual void OnCommonHeader(SpdyStreamId /*stream_id*/, size_t /*length*/, uint8_t /*type*/, uint8_t /*flags*/) {} - // Called when a data frame header is received. The frame's data - // payload will be provided via subsequent calls to - // OnStreamFrameData(). + // Called when a data frame header is received. The frame's data payload will + // be provided via subsequent calls to OnStreamFrameData(). + // |stream_id| The stream receiving data. + // |length| The length of the payload in this DATA frame. Includes the length + // of the data itself and potential padding. + // |fin| Whether the END_STREAM flag is set in the frame header. virtual void OnDataFrameHeader(SpdyStreamId stream_id, size_t length, bool fin) = 0; @@ -444,6 +447,8 @@ // Called when a HEADERS frame is received. // Note that header block data is not included. See OnHeaderFrameStart(). // |stream_id| The stream receiving the header. + // |payload_length| The length of the payload in this HEADERS frame. Includes + // the length of the encoded header block and potential padding. // |has_priority| Whether or not the headers frame included a priority value, // and stream dependency info. // |weight| If |has_priority| is true, then weight (in the range [1, 256]) @@ -452,10 +457,11 @@ // receiving stream, else 0. // |exclusive| If |has_priority| is true the exclusivity of dependence on the // parent stream, else false. - // |fin| Whether FIN flag is set in frame headers. + // |fin| Whether the END_STREAM flag is set in the frame header. // |end| False if HEADERs frame is to be followed by a CONTINUATION frame, // or true if not. - virtual void OnHeaders(SpdyStreamId stream_id, bool has_priority, int weight, + virtual void OnHeaders(SpdyStreamId stream_id, size_t payload_length, + bool has_priority, int weight, SpdyStreamId parent_stream_id, bool exclusive, bool fin, bool end) = 0;
diff --git a/quiche/spdy/core/spdy_framer_test.cc b/quiche/spdy/core/spdy_framer_test.cc index bdea70c..1b3c421 100644 --- a/quiche/spdy/core/spdy_framer_test.cc +++ b/quiche/spdy/core/spdy_framer_test.cc
@@ -353,12 +353,13 @@ ++goaway_count_; } - void OnHeaders(SpdyStreamId stream_id, bool has_priority, int weight, - SpdyStreamId parent_stream_id, bool exclusive, bool fin, - bool end) override { - QUICHE_VLOG(1) << "OnHeaders(" << stream_id << ", " << has_priority << ", " - << weight << ", " << parent_stream_id << ", " << exclusive - << ", " << fin << ", " << end << ")"; + void OnHeaders(SpdyStreamId stream_id, size_t payload_length, + bool has_priority, int weight, SpdyStreamId parent_stream_id, + bool exclusive, bool fin, bool end) override { + QUICHE_VLOG(1) << "OnHeaders(" << stream_id << ", " << payload_length + << ", " << has_priority << ", " << weight << ", " + << parent_stream_id << ", " << exclusive << ", " << fin + << ", " << end << ")"; ++headers_frame_count_; InitHeaderStreaming(SpdyFrameType::HEADERS, stream_id); if (fin) { @@ -852,7 +853,7 @@ sizeof(kH2FrameData), false); EXPECT_CALL(visitor, OnCommonHeader(1, 5, 0x1, 0x8)); - EXPECT_CALL(visitor, OnHeaders(1, false, 0, 0, false, false, false)); + EXPECT_CALL(visitor, OnHeaders(1, 5, false, 0, 0, false, false, false)); EXPECT_CALL(visitor, OnHeaderFrameStart(1)).Times(1); EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_PADDING, _)); EXPECT_EQ(frame.size(), deframer_->ProcessInput(frame.data(), frame.size())); @@ -883,7 +884,7 @@ SpdySerializedFrame frame(kH2FrameData, sizeof(kH2FrameData), false); EXPECT_CALL(visitor, OnCommonHeader(1, 5, 0x1, 0x8)); - EXPECT_CALL(visitor, OnHeaders(1, false, 0, 0, false, false, false)); + EXPECT_CALL(visitor, OnHeaders(1, 5, false, 0, 0, false, false, false)); EXPECT_CALL(visitor, OnHeaderFrameStart(1)).Times(1); EXPECT_EQ(frame.size(), deframer_->ProcessInput(frame.data(), frame.size())); @@ -1316,14 +1317,14 @@ testing::InSequence s; EXPECT_CALL(visitor, OnCommonHeader(1, 1, 0x1, 0x4)); - EXPECT_CALL(visitor, OnHeaders(1, false, 0, 0, false, false, true)); + EXPECT_CALL(visitor, OnHeaders(1, 1, false, 0, 0, false, false, true)); EXPECT_CALL(visitor, OnHeaderFrameStart(1)); EXPECT_CALL(visitor, OnHeaderFrameEnd(1)); EXPECT_CALL(visitor, OnCommonHeader(1, 12, 0x0, 0x0)); EXPECT_CALL(visitor, OnDataFrameHeader(1, 12, false)); EXPECT_CALL(visitor, OnStreamFrameData(1, _, 12)); EXPECT_CALL(visitor, OnCommonHeader(3, 6, 0x1, 0x24)); - EXPECT_CALL(visitor, OnHeaders(3, true, 131, 0, false, false, true)); + EXPECT_CALL(visitor, OnHeaders(3, 6, true, 131, 0, false, false, true)); EXPECT_CALL(visitor, OnHeaderFrameStart(3)); EXPECT_CALL(visitor, OnHeaderFrameEnd(3)); EXPECT_CALL(visitor, OnCommonHeader(3, 8, 0x0, 0x0)); @@ -4196,7 +4197,7 @@ exclusive = true; } EXPECT_CALL(visitor, OnCommonHeader(stream_id, _, 0x1, set_flags)); - EXPECT_CALL(visitor, OnHeaders(stream_id, has_priority, weight, + EXPECT_CALL(visitor, OnHeaders(stream_id, _, has_priority, weight, parent_stream_id, exclusive, fin, end)); EXPECT_CALL(visitor, OnHeaderFrameStart(57)).Times(1); if (end) { @@ -4338,7 +4339,7 @@ EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(42, SpdyFrameType::HEADERS, _)); EXPECT_CALL(visitor, OnCommonHeader(42, _, 0x1, 0)); - EXPECT_CALL(visitor, OnHeaders(42, false, 0, 0, false, false, false)); + EXPECT_CALL(visitor, OnHeaders(42, _, false, 0, 0, false, false, false)); EXPECT_CALL(visitor, OnHeaderFrameStart(42)).Times(1); SpdyHeadersIR headers_ir(/* stream_id = */ 42);
diff --git a/quiche/spdy/core/spdy_no_op_visitor.h b/quiche/spdy/core/spdy_no_op_visitor.h index b0fa009..3bb47be 100644 --- a/quiche/spdy/core/spdy_no_op_visitor.h +++ b/quiche/spdy/core/spdy_no_op_visitor.h
@@ -46,9 +46,10 @@ void OnSettingsAck() override {} void OnGoAway(SpdyStreamId /*last_accepted_stream_id*/, SpdyErrorCode /*error_code*/) override {} - void OnHeaders(SpdyStreamId /*stream_id*/, bool /*has_priority*/, - int /*weight*/, SpdyStreamId /*parent_stream_id*/, - bool /*exclusive*/, bool /*fin*/, bool /*end*/) override {} + void OnHeaders(SpdyStreamId /*stream_id*/, size_t /*payload_length*/, + bool /*has_priority*/, int /*weight*/, + SpdyStreamId /*parent_stream_id*/, bool /*exclusive*/, + bool /*fin*/, bool /*end*/) override {} void OnWindowUpdate(SpdyStreamId /*stream_id*/, int /*delta_window_size*/) override {} void OnPushPromise(SpdyStreamId /*stream_id*/,
diff --git a/quiche/spdy/test_tools/mock_spdy_framer_visitor.h b/quiche/spdy/test_tools/mock_spdy_framer_visitor.h index 3575d67..5d721c6 100644 --- a/quiche/spdy/test_tools/mock_spdy_framer_visitor.h +++ b/quiche/spdy/test_tools/mock_spdy_framer_visitor.h
@@ -59,9 +59,9 @@ MOCK_METHOD(bool, OnGoAwayFrameData, (const char* goaway_data, size_t len), (override)); MOCK_METHOD(void, OnHeaders, - (SpdyStreamId stream_id, bool has_priority, int weight, - SpdyStreamId parent_stream_id, bool exclusive, bool fin, - bool end), + (SpdyStreamId stream_id, size_t payload_length, bool has_priority, + int weight, SpdyStreamId parent_stream_id, bool exclusive, + bool fin, bool end), (override)); MOCK_METHOD(void, OnWindowUpdate, (SpdyStreamId stream_id, int delta_window_size), (override));