Refactor incoming SETTINGS logic to QuicSpdySession::OnSetting(). Move Google QUIC logic from QuicSpdySession::SpdyFramerVisitor::OnSettings(), and IETF QUIC logic from QuicReceiveControlStream::OnSettingsFrame() together into new method QuicSpdySession::OnSetting(). Consistently use spdy:: settings identifiers for former and quic:: settings identifiers for latter. (This reverts part of what I did at cr/260526616. My confusion about the two different kind of settings is in great part what motivates this CL so that all SETTINGS logic is in one place). Inline a number of QuicSpdySession methods now that they are not called from outside the class: * UpdateHeaderEncoderTableSize(), * UpdateEnableServerPush(), * set_server_push_enabled(), * set_max_outbound_header_list_size(). This change does not modify behavior. Note that in case of Google QUIC, settings identifiers (uint16_t) and values (uint32_t) are cast to uint64_t then back to their respective type, which is a no-op. gfe-relnote: n/a, no behavioral change. PiperOrigin-RevId: 260752115 Change-Id: I6b24c98542375e55d907027127ffa1149eaffea6
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc index 4408e59..8b26afc 100644 --- a/quic/core/http/quic_receive_control_stream.cc +++ b/quic/core/http/quic_receive_control_stream.cc
@@ -208,18 +208,8 @@ QUIC_DVLOG(1) << "Control Stream " << id() << " received settings frame: " << settings; QuicSpdySession* spdy_session = static_cast<QuicSpdySession*>(session()); - for (auto& it : settings.values) { - uint64_t setting_id = it.first; - switch (setting_id) { - case SETTINGS_MAX_HEADER_LIST_SIZE: - spdy_session->set_max_outbound_header_list_size(it.second); - break; - case SETTINGS_NUM_PLACEHOLDERS: - // TODO: Support placeholder setting - break; - default: - break; - } + for (const auto& setting : settings.values) { + spdy_session->OnSetting(setting.first, setting.second); } return true; }
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index 0e909ed..3c3f8e0 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -27,7 +27,6 @@ using spdy::HpackEntry; using spdy::HpackHeaderTable; using spdy::Http2WeightToSpdy3Priority; -using spdy::SETTINGS_ENABLE_PUSH; using spdy::Spdy3PriorityToHttp2Weight; using spdy::SpdyErrorCode; using spdy::SpdyFramer; @@ -152,37 +151,7 @@ } void OnSetting(SpdySettingsId id, uint32_t value) override { - switch (id) { - case SETTINGS_QPACK_MAX_TABLE_CAPACITY: - session_->UpdateHeaderEncoderTableSize(value); - break; - case SETTINGS_ENABLE_PUSH: - // TODO(b/138438479): Remove this setting. - if (session_->perspective() == Perspective::IS_SERVER) { - // See rfc7540, Section 6.5.2. - if (value > 1) { - CloseConnection( - QuicStrCat("Invalid value for SETTINGS_ENABLE_PUSH: ", value), - QUIC_INVALID_HEADERS_STREAM_DATA); - return; - } - session_->UpdateEnableServerPush(value > 0); - break; - } else { - CloseConnection( - QuicStrCat("Unsupported field of HTTP/2 SETTINGS frame: ", id), - QUIC_INVALID_HEADERS_STREAM_DATA); - } - break; - // TODO(fayang): Need to support SETTINGS_MAX_HEADER_LIST_SIZE when - // clients are actually sending it. - case SETTINGS_MAX_HEADER_LIST_SIZE: - break; - default: - CloseConnection( - QuicStrCat("Unsupported field of HTTP/2 SETTINGS frame: ", id), - QUIC_INVALID_HEADERS_STREAM_DATA); - } + session_->OnSetting(id, value); } void OnSettingsEnd() override {} @@ -649,6 +618,91 @@ ConnectionCloseBehavior::SILENT_CLOSE); } +void QuicSpdySession::OnSetting(uint64_t id, uint64_t value) { + if (VersionHasStreamType(connection()->transport_version())) { + // SETTINGS frame received on the control stream. + switch (id) { + case SETTINGS_QPACK_MAX_TABLE_CAPACITY: + QUIC_DVLOG(1) + << "SETTINGS_QPACK_MAX_TABLE_CAPACITY received with value " + << value; + // TODO(b/112770235): implement. + break; + case SETTINGS_MAX_HEADER_LIST_SIZE: + QUIC_DVLOG(1) << "SETTINGS_MAX_HEADER_LIST_SIZE received with value " + << value; + max_outbound_header_list_size_ = value; + break; + case SETTINGS_QPACK_BLOCKED_STREAMS: + QUIC_DVLOG(1) << "SETTINGS_QPACK_BLOCKED_STREAMS received with value " + << value; + // TODO(b/112770235): implement. + break; + case SETTINGS_NUM_PLACEHOLDERS: + QUIC_DVLOG(1) << "SETTINGS_NUM_PLACEHOLDERS received with value " + << value; + // TODO: Support placeholder setting. + break; + default: + QUIC_DVLOG(1) << "Unknown setting identifier " << id + << " received with value " << value; + // Ignore unknown settings. + break; + } + return; + } + + // SETTINGS frame received on the headers stream. + switch (id) { + case spdy::SETTINGS_HEADER_TABLE_SIZE: + QUIC_DVLOG(1) << "SETTINGS_HEADER_TABLE_SIZE received with value " + << value; + spdy_framer_.UpdateHeaderEncoderTableSize(value); + break; + case spdy::SETTINGS_ENABLE_PUSH: + if (perspective() == Perspective::IS_SERVER) { + // See rfc7540, Section 6.5.2. + if (value > 1) { + QUIC_DLOG(ERROR) << "Invalid value " << value + << " received for SETTINGS_ENABLE_PUSH."; + if (IsConnected()) { + CloseConnectionWithDetails( + QUIC_INVALID_HEADERS_STREAM_DATA, + QuicStrCat("Invalid value for SETTINGS_ENABLE_PUSH: ", value)); + } + return; + } + QUIC_DVLOG(1) << "SETTINGS_ENABLE_PUSH received with value " << value; + server_push_enabled_ = value; + break; + } else { + QUIC_DLOG(ERROR) + << "Invalid SETTINGS_ENABLE_PUSH received by client with value " + << value; + if (IsConnected()) { + CloseConnectionWithDetails( + QUIC_INVALID_HEADERS_STREAM_DATA, + QuicStrCat("Unsupported field of HTTP/2 SETTINGS frame: ", id)); + } + } + break; + // TODO(fayang): Need to support SETTINGS_MAX_HEADER_LIST_SIZE when + // clients are actually sending it. + case spdy::SETTINGS_MAX_HEADER_LIST_SIZE: + QUIC_DVLOG(1) << "SETTINGS_MAX_HEADER_LIST_SIZE received with value " + << value; + break; + default: + QUIC_DLOG(ERROR) << "Unknown setting identifier " << id + << " received with value " << value; + if (IsConnected()) { + CloseConnectionWithDetails( + QUIC_INVALID_HEADERS_STREAM_DATA, + QuicStrCat("Unsupported field of HTTP/2 SETTINGS frame: ", id)); + } + } +} + bool QuicSpdySession::ShouldReleaseHeadersStreamSequencerBuffer() { return false; } @@ -742,15 +796,6 @@ connection()->helper()->GetClock(), std::move(visitor))); } -void QuicSpdySession::UpdateHeaderEncoderTableSize(uint32_t value) { - // TODO(b/112770235): Update QpackEncoder. - spdy_framer_.UpdateHeaderEncoderTableSize(value); -} - -void QuicSpdySession::UpdateEnableServerPush(bool value) { - set_server_push_enabled(value); -} - void QuicSpdySession::CloseConnectionWithDetails(QuicErrorCode error, const std::string& details) { connection()->CloseConnection(
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h index 8d8bbda..1fa0be7 100644 --- a/quic/core/http/quic_spdy_session.h +++ b/quic/core/http/quic_spdy_session.h
@@ -143,9 +143,8 @@ bool server_push_enabled() const { return server_push_enabled_; } - // Called by |QuicHeadersStream::UpdateEnableServerPush()| with - // value from SETTINGS_ENABLE_PUSH. - void set_server_push_enabled(bool enable) { server_push_enabled_ = enable; } + // Called when a setting is parsed from an incoming SETTINGS frame. + void OnSetting(uint64_t id, uint64_t value); // Return true if this session wants to release headers stream's buffer // aggressively. @@ -160,10 +159,6 @@ max_inbound_header_list_size_ = max_inbound_header_list_size; } - void set_max_outbound_header_list_size(size_t max_outbound_header_list_size) { - max_outbound_header_list_size_ = max_outbound_header_list_size; - } - size_t max_outbound_header_list_size() const { return max_outbound_header_list_size_; }