Add Http2DecoderAdapter::SetMaxFrameSize(), currently unused.
This CL performs several minor refactors/non-functional changes:
- Adds a setter SetMaxFrameSize() to store the new value in
Http2DecoderAdapter and its Http2FrameDecoder member.
- Renames recv_frame_size_limit_ to max_frame_size_, to reflect that the
field corresponds to SETTINGS_MAX_FRAME_SIZE.
- Removes some unneeded local variables.
- Removes the frame_decoder_->set_maximum_payload_size() call from
ProcessInput(). This change is a no-op because
Http2DecoderAdapter's recv_frame_size_limit_/max_frame_size_ before was not
altered from its default value [1] (so the repeated calls in ProcessInput()
were no-ops). Furthermore, the default value already matches the
frame_decoder_ default value for its maximum_payload_size_ [2].
In production, ResetInternal() is only called in construction.
The addition of SetMaxFrameSize() is important for a future change in
OgHttp2Session, which will call this new method when sending custom
SETTINGS_MAX_FRAME_SIZE values to the peer.
[1] http://google3/third_party/spdy/core/http2_frame_decoder_adapter.h;l=313;rcl=402940499
[2] http://google3/third_party/http2/decoder/http2_frame_decoder.cc;l=37;rcl=433094660
PiperOrigin-RevId: 433892628
diff --git a/spdy/core/http2_frame_decoder_adapter.cc b/spdy/core/http2_frame_decoder_adapter.cc
index 7e424a3..320befa 100644
--- a/spdy/core/http2_frame_decoder_adapter.cc
+++ b/spdy/core/http2_frame_decoder_adapter.cc
@@ -264,9 +264,6 @@
}
size_t Http2DecoderAdapter::ProcessInput(const char* data, size_t len) {
- size_t limit = recv_frame_size_limit_;
- frame_decoder_->set_maximum_payload_size(limit);
-
size_t total_processed = 0;
while (len > 0 && spdy_state_ != SPDY_ERROR) {
// Process one at a time so that we update the adapter's internal
@@ -312,6 +309,11 @@
"Ignoring further events on this connection.");
}
+void Http2DecoderAdapter::SetMaxFrameSize(size_t max_frame_size) {
+ max_frame_size_ = max_frame_size;
+ frame_decoder_->set_maximum_payload_size(max_frame_size);
+}
+
// ===========================================================================
// Implementations of the methods declared by Http2FrameDecoderListener.
@@ -767,8 +769,7 @@
void Http2DecoderAdapter::OnFrameSizeError(const Http2FrameHeader& header) {
QUICHE_DVLOG(1) << "OnFrameSizeError: " << header;
- size_t recv_limit = recv_frame_size_limit_;
- if (header.payload_length > recv_limit) {
+ if (header.payload_length > max_frame_size_) {
if (header.type == Http2FrameType::DATA) {
SetSpdyErrorAndNotify(SpdyFramerError::SPDY_OVERSIZED_PAYLOAD, "");
} else {
diff --git a/spdy/core/http2_frame_decoder_adapter.h b/spdy/core/http2_frame_decoder_adapter.h
index afe9415..259924c 100644
--- a/spdy/core/http2_frame_decoder_adapter.h
+++ b/spdy/core/http2_frame_decoder_adapter.h
@@ -162,6 +162,10 @@
// events for this connection.
void StopProcessing();
+ // Sets the limit on the size of received HTTP/2 frame payloads. Corresponds
+ // to SETTINGS_MAX_FRAME_SIZE as advertised to the peer.
+ void SetMaxFrameSize(size_t max_frame_size);
+
private:
bool OnFrameHeader(const Http2FrameHeader& header) override;
void OnDataStart(const Http2FrameHeader& header) override;
@@ -310,7 +314,7 @@
// The limit on the size of received HTTP/2 payloads as specified in the
// SETTINGS_MAX_FRAME_SIZE advertised to peer.
- size_t recv_frame_size_limit_ = spdy::kHttp2DefaultFramePayloadLimit;
+ size_t max_frame_size_ = spdy::kHttp2DefaultFramePayloadLimit;
// Has OnFrameHeader been called?
bool decoded_frame_header_ = false;
diff --git a/spdy/core/spdy_framer_test.cc b/spdy/core/spdy_framer_test.cc
index 9f04576..1b20d9b 100644
--- a/spdy/core/spdy_framer_test.cc
+++ b/spdy/core/spdy_framer_test.cc
@@ -693,8 +693,8 @@
}
}
-// Test that if we receive a frame with payload length field at the
-// advertised max size, we do not set an error in ProcessInput.
+// Test that if we receive a frame with a payload length field at the default
+// max size, we do not set an error in ProcessInput.
TEST_P(SpdyFramerTest, AcceptMaxFrameSizeSetting) {
testing::StrictMock<test::MockSpdyFramerVisitor> visitor;
deframer_.set_visitor(&visitor);
@@ -718,8 +718,8 @@
EXPECT_FALSE(deframer_.HasError());
}
-// Test that if we receive a frame with payload length larger than the
-// advertised max size, we set an error of SPDY_INVALID_CONTROL_FRAME_SIZE.
+// Test that if we receive a frame with a payload length larger than the default
+// max size, we set an error of SPDY_INVALID_CONTROL_FRAME_SIZE.
TEST_P(SpdyFramerTest, ExceedMaxFrameSizeSetting) {
testing::StrictMock<test::MockSpdyFramerVisitor> visitor;
deframer_.set_visitor(&visitor);
@@ -746,6 +746,34 @@
deframer_.spdy_framer_error());
}
+// Test that if we set a larger max frame size and then receive a frame with a
+// payload length at that larger size, we do not set an error in ProcessInput.
+TEST_P(SpdyFramerTest, AcceptLargerMaxFrameSizeSetting) {
+ testing::StrictMock<test::MockSpdyFramerVisitor> visitor;
+ deframer_.set_visitor(&visitor);
+
+ const size_t big_frame_size = (1 << 14) + 1;
+ deframer_.SetMaxFrameSize(big_frame_size);
+
+ // DATA frame with larger-than-default but acceptable payload length.
+ unsigned char kH2FrameData[] = {
+ 0x00, 0x40, 0x01, // Length: 2^14 + 1
+ 0x00, // Type: DATA
+ 0x00, // Flags: None
+ 0x00, 0x00, 0x00, 0x01, // Stream: 1
+ 0x00, 0x00, 0x00, 0x00, // Junk payload
+ };
+
+ SpdySerializedFrame frame(reinterpret_cast<char*>(kH2FrameData),
+ sizeof(kH2FrameData), false);
+
+ EXPECT_CALL(visitor, OnCommonHeader(1, big_frame_size, 0x0, 0x0));
+ EXPECT_CALL(visitor, OnDataFrameHeader(1, big_frame_size, false));
+ EXPECT_CALL(visitor, OnStreamFrameData(1, _, 4));
+ deframer_.ProcessInput(frame.data(), frame.size());
+ EXPECT_FALSE(deframer_.HasError());
+}
+
// Test that if we receive a DATA frame with padding length larger than the
// payload length, we set an error of SPDY_INVALID_PADDING
TEST_P(SpdyFramerTest, OversizedDataPaddingError) {