Migrate OgHttp2Session to using OnUnknownFrameStart() and OnUnknownFramePayload().
This CL migrates OgHttp2Session from implementing ExtensionVisitorInterface to
implementing the corresponding SpdyFramerVisitorInterface OnUnknownFrameStart()
and OnUnknownFramePayload() overrides.
The migration mostly involves a trivial moving of code, along with:
- Adding a bool process_metadata_ field to drop uninteresting unknown frames.
- Removing the StreamId metadata_stream_id_ field, as the stream ID is an
argument in OnUnknownFramePayload().
This CL is expected to be a functional no-op.
PiperOrigin-RevId: 461946550
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc
index f0800f7..d3d4f79 100644
--- a/quiche/http2/adapter/oghttp2_session.cc
+++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -357,7 +357,6 @@
options.should_window_update_fn,
/*update_window_on_notify=*/false) {
decoder_.set_visitor(&receive_logger_);
- decoder_.set_extension_visitor(this);
if (options_.max_header_list_bytes) {
// Limit buffering of encoded HPACK data to 2x the decoded limit.
decoder_.GetHpackDecoder()->set_max_decode_buffer_size_bytes(
@@ -1491,12 +1490,52 @@
return true;
}
-void OgHttp2Session::OnUnknownFrameStart(spdy::SpdyStreamId /*stream_id*/,
- size_t /*length*/, uint8_t /*type*/,
- uint8_t /*flags*/) {}
+void OgHttp2Session::OnUnknownFrameStart(spdy::SpdyStreamId stream_id,
+ size_t length, uint8_t type,
+ uint8_t flags) {
+ process_metadata_ = false;
+ if (streams_reset_.contains(stream_id)) {
+ return;
+ }
+ if (type == kMetadataFrameType) {
+ QUICHE_DCHECK_EQ(metadata_length_, 0u);
+ visitor_.OnBeginMetadataForStream(stream_id, length);
+ metadata_length_ = length;
+ process_metadata_ = true;
+ end_metadata_ = flags & kMetadataEndFlag;
-void OgHttp2Session::OnUnknownFramePayload(spdy::SpdyStreamId /*stream_id*/,
- absl::string_view /*payload*/) {}
+ // Empty metadata payloads will not trigger OnUnknownFramePayload(), so
+ // handle that possibility here.
+ MaybeHandleMetadataEndForStream(stream_id);
+ } else {
+ QUICHE_DLOG(INFO) << "Received unexpected frame type "
+ << static_cast<int>(type);
+ }
+}
+
+void OgHttp2Session::OnUnknownFramePayload(spdy::SpdyStreamId stream_id,
+ absl::string_view payload) {
+ if (!process_metadata_) {
+ return;
+ }
+ if (streams_reset_.contains(stream_id)) {
+ return;
+ }
+ if (metadata_length_ > 0) {
+ QUICHE_DCHECK_LE(payload.size(), metadata_length_);
+ const bool payload_success =
+ visitor_.OnMetadataForStream(stream_id, payload);
+ if (payload_success) {
+ metadata_length_ -= payload.size();
+ MaybeHandleMetadataEndForStream(stream_id);
+ } else {
+ fatal_visitor_callback_failure_ = true;
+ decoder_.StopProcessing();
+ }
+ } else {
+ QUICHE_DLOG(INFO) << "Unexpected metadata payload for stream " << stream_id;
+ }
+}
void OgHttp2Session::OnHeaderStatus(
Http2StreamId stream_id, Http2VisitorInterface::OnHeaderResult result) {
@@ -1542,51 +1581,6 @@
}
}
-bool OgHttp2Session::OnFrameHeader(spdy::SpdyStreamId stream_id, size_t length,
- uint8_t type, uint8_t flags) {
- if (streams_reset_.contains(stream_id)) {
- return false;
- }
- if (type == kMetadataFrameType) {
- QUICHE_DCHECK_EQ(metadata_length_, 0u);
- visitor_.OnBeginMetadataForStream(stream_id, length);
- metadata_stream_id_ = stream_id;
- metadata_length_ = length;
- end_metadata_ = flags & kMetadataEndFlag;
-
- // Empty metadata payloads will not trigger OnFramePayload(), so handle
- // that possibility here.
- MaybeHandleMetadataEndForStream(metadata_stream_id_);
-
- return true;
- } else {
- QUICHE_DLOG(INFO) << "Unexpected frame type " << static_cast<int>(type)
- << " received by the extension visitor.";
- return false;
- }
-}
-
-void OgHttp2Session::OnFramePayload(const char* data, size_t len) {
- if (streams_reset_.contains(metadata_stream_id_)) {
- return;
- }
- if (metadata_length_ > 0) {
- QUICHE_DCHECK_LE(len, metadata_length_);
- const bool payload_success = visitor_.OnMetadataForStream(
- metadata_stream_id_, absl::string_view(data, len));
- if (payload_success) {
- metadata_length_ -= len;
- MaybeHandleMetadataEndForStream(metadata_stream_id_);
- } else {
- fatal_visitor_callback_failure_ = true;
- decoder_.StopProcessing();
- }
- } else {
- QUICHE_DLOG(INFO) << "Unexpected metadata payload for stream "
- << metadata_stream_id_;
- }
-}
-
void OgHttp2Session::MaybeSetupPreface(bool sending_outbound_settings) {
if (!queued_preface_) {
queued_preface_ = true;
@@ -1894,7 +1888,7 @@
fatal_visitor_callback_failure_ = true;
decoder_.StopProcessing();
}
- metadata_stream_id_ = 0;
+ process_metadata_ = false;
end_metadata_ = false;
}
}
diff --git a/quiche/http2/adapter/oghttp2_session.h b/quiche/http2/adapter/oghttp2_session.h
index 5f28217..1d896a3 100644
--- a/quiche/http2/adapter/oghttp2_session.h
+++ b/quiche/http2/adapter/oghttp2_session.h
@@ -36,8 +36,7 @@
// This class manages state associated with a single multiplexed HTTP/2 session.
class QUICHE_EXPORT_PRIVATE OgHttp2Session
: public Http2Session,
- public spdy::SpdyFramerVisitorInterface,
- public spdy::ExtensionVisitorInterface {
+ public spdy::SpdyFramerVisitorInterface {
public:
struct QUICHE_EXPORT_PRIVATE Options {
// Returns whether to send a WINDOW_UPDATE based on the window limit, window
@@ -213,13 +212,6 @@
void OnHeaderStatus(Http2StreamId stream_id,
Http2VisitorInterface::OnHeaderResult result);
- // Returns true if a recognized extension frame is received.
- bool OnFrameHeader(spdy::SpdyStreamId stream_id, size_t length, uint8_t type,
- uint8_t flags) override;
-
- // Handles the payload for a recognized extension frame.
- void OnFramePayload(const char* data, size_t len) override;
-
private:
struct QUICHE_EXPORT_PRIVATE StreamState {
StreamState(int32_t stream_receive_window, int32_t stream_send_window,
@@ -507,7 +499,6 @@
// which this endpoint created a stream in the stream map.
Http2StreamId highest_received_stream_id_ = 0;
Http2StreamId highest_processed_stream_id_ = 0;
- Http2StreamId metadata_stream_id_ = 0;
Http2StreamId received_goaway_stream_id_ = 0;
size_t metadata_length_ = 0;
int32_t connection_send_window_ = kInitialFlowControlWindowSize;
@@ -537,6 +528,7 @@
bool queued_preface_ = false;
bool peer_supports_metadata_ = false;
bool end_metadata_ = false;
+ bool process_metadata_ = false;
bool sent_non_ack_settings_ = false;
// Recursion guard for ProcessBytes().
