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_;
}
