Validates more HTTP/2 SETTINGS values in oghttp2. Specifically, SETTINGS_ENABLE_PUSH and SETTINGS_ENABLE_CONNECT_PROTOCOL. PiperOrigin-RevId: 453484075
diff --git a/quiche/http2/adapter/nghttp2_adapter_test.cc b/quiche/http2/adapter/nghttp2_adapter_test.cc index 8647b02..8751f52 100644 --- a/quiche/http2/adapter/nghttp2_adapter_test.cc +++ b/quiche/http2/adapter/nghttp2_adapter_test.cc
@@ -5883,12 +5883,12 @@ EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::SETTINGS})); } -TEST(NgHttp2AdapterTest, InvalidMaxFrameSize) { +TEST(NgHttp2AdapterTest, InvalidMaxFrameSizeSetting) { DataSavingVisitor visitor; auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor); const std::string frames = - TestFrameSequence().ClientPreface({{MAX_FRAME_SIZE, 3}}).Serialize(); + TestFrameSequence().ClientPreface({{MAX_FRAME_SIZE, 3u}}).Serialize(); testing::InSequence s; // Client preface @@ -5912,6 +5912,90 @@ EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::GOAWAY})); } +TEST(OgHttp2AdapterTest, InvalidPushSetting) { + DataSavingVisitor visitor; + auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor); + + const std::string frames = + TestFrameSequence().ClientPreface({{ENABLE_PUSH, 3u}}).Serialize(); + testing::InSequence s; + + // Client preface + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(visitor, OnInvalidFrame(0, _)); + + const int64_t read_result = adapter->ProcessBytes(frames); + EXPECT_EQ(static_cast<size_t>(read_result), frames.size()); + + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0)); + EXPECT_CALL(visitor, + OnFrameSent(GOAWAY, 0, _, 0x0, + static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR))); + + int send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::GOAWAY})); +} + +TEST(NgHttp2AdapterTest, InvalidConnectProtocolSetting) { + DataSavingVisitor visitor; + auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor); + + const std::string frames = TestFrameSequence() + .ClientPreface({{ENABLE_CONNECT_PROTOCOL, 3u}}) + .Serialize(); + testing::InSequence s; + + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL( + visitor, + OnInvalidFrame(0, Http2VisitorInterface::InvalidFrameError::kProtocol)); + + int64_t read_result = adapter->ProcessBytes(frames); + EXPECT_EQ(static_cast<size_t>(read_result), frames.size()); + + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0)); + EXPECT_CALL(visitor, + OnFrameSent(GOAWAY, 0, _, 0x0, + static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR))); + + int send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::GOAWAY})); + + auto adapter2 = NgHttp2Adapter::CreateServerAdapter(visitor); + const std::string frames2 = TestFrameSequence() + .ClientPreface({{ENABLE_CONNECT_PROTOCOL, 1}}) + .Settings({{ENABLE_CONNECT_PROTOCOL, 0}}) + .Serialize(); + + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSetting(Http2Setting{ENABLE_CONNECT_PROTOCOL, 1u})); + EXPECT_CALL(visitor, OnSettingsEnd()); + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + // Surprisingly, nghttp2 allows this behavior, which is prohibited in RFC + // 8441. + EXPECT_CALL(visitor, OnSetting(Http2Setting{ENABLE_CONNECT_PROTOCOL, 0u})); + EXPECT_CALL(visitor, OnSettingsEnd()); + + read_result = adapter2->ProcessBytes(frames2); + EXPECT_EQ(static_cast<size_t>(read_result), frames2.size()); + + EXPECT_TRUE(adapter2->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); + adapter2->Send(); +} + TEST(NgHttp2AdapterTest, ServerForbidsProtocolPseudoheaderBeforeAck) { DataSavingVisitor visitor; auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc index ba195bc..ca6333f 100644 --- a/quiche/http2/adapter/oghttp2_adapter_test.cc +++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -347,14 +347,14 @@ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS})); } -TEST(OgHttp2AdapterTest, InvalidMaxFrameSize) { +TEST(OgHttp2AdapterTest, InvalidMaxFrameSizeSetting) { DataSavingVisitor visitor; OgHttp2Adapter::Options options; options.perspective = Perspective::kServer; auto adapter = OgHttp2Adapter::Create(visitor, options); const std::string frames = - TestFrameSequence().ClientPreface({{MAX_FRAME_SIZE, 3}}).Serialize(); + TestFrameSequence().ClientPreface({{MAX_FRAME_SIZE, 3u}}).Serialize(); testing::InSequence s; // Client preface @@ -383,6 +383,108 @@ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::GOAWAY})); } +TEST(OgHttp2AdapterTest, InvalidPushSetting) { + DataSavingVisitor visitor; + OgHttp2Adapter::Options options; + options.perspective = Perspective::kServer; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + const std::string frames = + TestFrameSequence().ClientPreface({{ENABLE_PUSH, 3u}}).Serialize(); + testing::InSequence s; + + // Client preface + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL( + visitor, + OnInvalidFrame(0, Http2VisitorInterface::InvalidFrameError::kProtocol)); + EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kInvalidSetting)); + + const int64_t read_result = adapter->ProcessBytes(frames); + EXPECT_EQ(static_cast<size_t>(read_result), frames.size()); + + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0)); + EXPECT_CALL(visitor, + OnFrameSent(GOAWAY, 0, _, 0x0, + static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR))); + + int send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + EXPECT_THAT(visitor.data(), + EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::GOAWAY})); +} + +TEST(OgHttp2AdapterTest, InvalidConnectProtocolSetting) { + DataSavingVisitor visitor; + OgHttp2Adapter::Options options; + options.perspective = Perspective::kServer; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + const std::string frames = TestFrameSequence() + .ClientPreface({{ENABLE_CONNECT_PROTOCOL, 3u}}) + .Serialize(); + testing::InSequence s; + + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL( + visitor, + OnInvalidFrame(0, Http2VisitorInterface::InvalidFrameError::kProtocol)); + EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kInvalidSetting)); + + int64_t read_result = adapter->ProcessBytes(frames); + EXPECT_EQ(static_cast<size_t>(read_result), frames.size()); + + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0)); + EXPECT_CALL(visitor, + OnFrameSent(GOAWAY, 0, _, 0x0, + static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR))); + + int send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + EXPECT_THAT(visitor.data(), + EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::GOAWAY})); + + auto adapter2 = OgHttp2Adapter::Create(visitor, options); + const std::string frames2 = TestFrameSequence() + .ClientPreface({{ENABLE_CONNECT_PROTOCOL, 1}}) + .Settings({{ENABLE_CONNECT_PROTOCOL, 0}}) + .Serialize(); + + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSetting(Http2Setting{ENABLE_CONNECT_PROTOCOL, 1u})); + EXPECT_CALL(visitor, OnSettingsEnd()); + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL( + visitor, + OnInvalidFrame(0, Http2VisitorInterface::InvalidFrameError::kProtocol)); + EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kInvalidSetting)); + + read_result = adapter2->ProcessBytes(frames2); + EXPECT_EQ(static_cast<size_t>(read_result), frames2.size()); + + EXPECT_TRUE(adapter2->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0)); + EXPECT_CALL(visitor, + OnFrameSent(GOAWAY, 0, _, 0x0, + static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR))); + adapter2->Send(); +} + TEST(OgHttp2AdapterTest, ClientHandles100Headers) { DataSavingVisitor visitor; OgHttp2Adapter::Options options;
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc index 9f48090..89fc1df 100644 --- a/quiche/http2/adapter/oghttp2_session.cc +++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -1233,6 +1233,18 @@ encoder_header_table_capacity_when_acking_ = value; } break; + case ENABLE_PUSH: + if (value > 1u) { + visitor_.OnInvalidFrame( + 0, Http2VisitorInterface::InvalidFrameError::kProtocol); + // The specification says this is a connection-level protocol error. + LatchErrorAndNotify( + Http2ErrorCode::PROTOCOL_ERROR, + Http2VisitorInterface::ConnectionError::kInvalidSetting); + return; + } + // Aside from validation, this setting is ignored. + break; case MAX_CONCURRENT_STREAMS: max_outbound_concurrent_streams_ = value; if (!IsServerSession()) { @@ -1266,6 +1278,17 @@ } max_frame_payload_ = value; break; + case ENABLE_CONNECT_PROTOCOL: + if (value > 1u || (value == 0 && peer_enables_connect_protocol_)) { + visitor_.OnInvalidFrame( + 0, Http2VisitorInterface::InvalidFrameError::kProtocol); + LatchErrorAndNotify( + Http2ErrorCode::PROTOCOL_ERROR, + Http2VisitorInterface::ConnectionError::kInvalidSetting); + return; + } + peer_enables_connect_protocol_ = (value == 1u); + break; default: // TODO(bnc): See if C++17 inline constants are allowed in QUICHE. if (id == kMetadataExtensionId) {
diff --git a/quiche/http2/adapter/oghttp2_session.h b/quiche/http2/adapter/oghttp2_session.h index b4d713b..25b7f52 100644 --- a/quiche/http2/adapter/oghttp2_session.h +++ b/quiche/http2/adapter/oghttp2_session.h
@@ -153,6 +153,9 @@ !goaway_rejected_streams_.empty()); } int GetRemoteWindowSize() const override { return connection_send_window_; } + bool peer_enables_connect_protocol() { + return peer_enables_connect_protocol_; + } // From SpdyFramerVisitorInterface void OnError(http2::Http2DecoderAdapter::SpdyFramerError error, @@ -508,7 +511,7 @@ int32_t initial_stream_receive_window_ = kInitialFlowControlWindowSize; // The initial flow control send window size for any newly created streams. int32_t initial_stream_send_window_ = kInitialFlowControlWindowSize; - uint32_t max_frame_payload_ = 16384u; + uint32_t max_frame_payload_ = kDefaultFramePayloadSizeLimit; // The maximum number of concurrent streams that this connection can open to // its peer and allow from its peer, respectively. Although the initial value // is unlimited, the spec encourages a value of at least 100. We limit @@ -537,6 +540,8 @@ // Recursion guard for Send(). bool sending_ = false; + bool peer_enables_connect_protocol_ = false; + // Replace this with a stream ID, for multiple GOAWAY support. bool queued_goaway_ = false; bool queued_immediate_goaway_ = false;