Add HttpDecoder::OnPriorityFrameStart() so that QuicSpdyStream knows the size
of the priority frame and can consume correct amount of bytes.
gfe-relnote: v99 only, not in prod.
PiperOrigin-RevId: 254279623
Change-Id: Ic24fd04b055033c0735a224e76a949714b3ef268
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index 4513f04..179ab83 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -160,27 +160,32 @@
return;
}
- // Calling the following two visitor methods does not require parsing of any
+ // Calling the following visitor methods does not require parsing of any
// frame payload.
+ auto frame_meta = Http3FrameLengths(
+ current_length_field_length_ + current_type_field_length_,
+ current_frame_length_);
if (current_frame_type_ == 0x0) {
- if (!visitor_->OnDataFrameStart(Http3FrameLengths(
- current_length_field_length_ + current_type_field_length_,
- current_frame_length_))) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ if (!visitor_->OnDataFrameStart(frame_meta)) {
+ RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down on DATA frame start.");
return;
}
} else if (current_frame_type_ == 0x1) {
- if (!visitor_->OnHeadersFrameStart(Http3FrameLengths(
- current_length_field_length_ + current_type_field_length_,
- current_frame_length_))) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ if (!visitor_->OnHeadersFrameStart(frame_meta)) {
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on HEADERS frame start.");
return;
}
} else if (current_frame_type_ == 0x4) {
- if (!visitor_->OnSettingsFrameStart(Http3FrameLengths(
- current_length_field_length_ + current_type_field_length_,
- current_frame_length_))) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ if (!visitor_->OnSettingsFrameStart(frame_meta)) {
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on SETTINGS frame start.");
+ return;
+ }
+ } else if (current_frame_type_ == 0x2) {
+ if (!visitor_->OnPriorityFrameStart(frame_meta)) {
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on PRIORITY frame start.");
return;
}
}
@@ -204,7 +209,7 @@
}
DCHECK(!payload.empty());
if (!visitor_->OnDataFramePayload(payload)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down on DATA frame.");
return;
}
remaining_frame_length_ -= payload.length();
@@ -220,7 +225,7 @@
}
DCHECK(!payload.empty());
if (!visitor_->OnHeadersFramePayload(payload)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down on HEADERS frame.");
return;
}
remaining_frame_length_ -= payload.length();
@@ -251,7 +256,8 @@
}
remaining_frame_length_ -= bytes_remaining - reader->BytesRemaining();
if (!visitor_->OnPushPromiseFrameStart(push_id)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on PUSH_PROMISE frame start.");
return;
}
}
@@ -268,7 +274,8 @@
}
DCHECK(!payload.empty());
if (!visitor_->OnPushPromiseFramePayload(payload)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on PUSH_PROMISE frame.");
return;
}
remaining_frame_length_ -= payload.length();
@@ -323,14 +330,15 @@
switch (current_frame_type_) {
case 0x0: { // DATA
if (!visitor_->OnDataFrameEnd()) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down on DATA frame end.");
return;
}
break;
}
case 0x1: { // HEADERS
if (!visitor_->OnHeadersFrameEnd()) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on HEADERS frame end.");
return;
}
break;
@@ -344,7 +352,7 @@
return;
}
if (!visitor_->OnPriorityFrame(frame)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down on PRIORITY frame.");
return;
}
break;
@@ -358,7 +366,8 @@
return;
}
if (!visitor_->OnCancelPushFrame(frame)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on CANCEL_PUSH frame.");
return;
}
break;
@@ -370,14 +379,15 @@
return;
}
if (!visitor_->OnSettingsFrame(frame)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down on SETTINGS frame.");
return;
}
break;
}
case 0x5: { // PUSH_PROMISE
if (!visitor_->OnPushPromiseFrameEnd()) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on PUSH_PROMISE frame end.");
return;
}
break;
@@ -396,7 +406,7 @@
}
frame.stream_id = static_cast<QuicStreamId>(stream_id);
if (!visitor_->OnGoAwayFrame(frame)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down on GOAWAY frame.");
return;
}
break;
@@ -410,7 +420,8 @@
return;
}
if (!visitor_->OnMaxPushIdFrame(frame)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on MAX_PUSH_ID frame.");
return;
}
break;
@@ -424,7 +435,8 @@
return;
}
if (!visitor_->OnDuplicatePushFrame(frame)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Visitor shut down.");
+ RaiseError(QUIC_INTERNAL_ERROR,
+ "Visitor shut down on DUPLICATE_PUSH frame.");
return;
}
break;
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h
index 9f0db73..0304b56 100644
--- a/quic/core/http/http_decoder.h
+++ b/quic/core/http/http_decoder.h
@@ -40,6 +40,11 @@
// Called if an error is detected.
virtual void OnError(HttpDecoder* decoder) = 0;
+ // Called when a PRIORITY frame has been received.
+ // |frame_length| contains PRIORITY frame length and payload length.
+ // Returns true to permit furthuring decoding, and false to prevent it.
+ virtual bool OnPriorityFrameStart(Http3FrameLengths frame_length) = 0;
+
// Called when a PRIORITY frame has been successfully parsed.
// Returns true to permit furthuring decoding, and false to prevent it.
virtual bool OnPriorityFrame(const PriorityFrame& frame) = 0;
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc
index c25d086..b74918a 100644
--- a/quic/core/http/http_decoder_test.cc
+++ b/quic/core/http/http_decoder_test.cc
@@ -23,6 +23,7 @@
// Called if an error is detected.
MOCK_METHOD1(OnError, void(HttpDecoder* decoder));
+ MOCK_METHOD1(OnPriorityFrameStart, bool(Http3FrameLengths frame_lengths));
MOCK_METHOD1(OnPriorityFrame, bool(const PriorityFrame& frame));
MOCK_METHOD1(OnCancelPushFrame, bool(const CancelPushFrame& frame));
MOCK_METHOD1(OnMaxPushIdFrame, bool(const MaxPushIdFrame& frame));
@@ -47,6 +48,7 @@
class HttpDecoderTest : public QuicTest {
public:
HttpDecoderTest() {
+ ON_CALL(visitor_, OnPriorityFrameStart(_)).WillByDefault(Return(true));
ON_CALL(visitor_, OnPriorityFrame(_)).WillByDefault(Return(true));
ON_CALL(visitor_, OnCancelPushFrame(_)).WillByDefault(Return(true));
ON_CALL(visitor_, OnMaxPushIdFrame(_)).WillByDefault(Return(true));
@@ -199,7 +201,7 @@
.WillOnce(Return(false));
EXPECT_EQ(0u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder_.error());
- EXPECT_EQ("Visitor shut down.", decoder_.error_detail());
+ EXPECT_EQ("Visitor shut down on CANCEL_PUSH frame.", decoder_.error_detail());
}
TEST_F(HttpDecoderTest, PushPromiseFrame) {
@@ -242,7 +244,8 @@
EXPECT_CALL(visitor_, OnPushPromiseFrameStart(1)).WillOnce(Return(false));
EXPECT_EQ(0u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder_.error());
- EXPECT_EQ("Visitor shut down.", decoder_.error_detail());
+ EXPECT_EQ("Visitor shut down on PUSH_PROMISE frame start.",
+ decoder_.error_detail());
}
TEST_F(HttpDecoderTest, MaxPushId) {
@@ -273,7 +276,7 @@
.WillOnce(Return(false));
EXPECT_EQ(0u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder_.error());
- EXPECT_EQ("Visitor shut down.", decoder_.error_detail());
+ EXPECT_EQ("Visitor shut down on MAX_PUSH_ID frame.", decoder_.error_detail());
}
TEST_F(HttpDecoderTest, DuplicatePush) {
@@ -303,7 +306,8 @@
.WillOnce(Return(false));
EXPECT_EQ(0u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder_.error());
- EXPECT_EQ("Visitor shut down.", decoder_.error_detail());
+ EXPECT_EQ("Visitor shut down on DUPLICATE_PUSH frame.",
+ decoder_.error_detail());
}
TEST_F(HttpDecoderTest, PriorityFrame) {
@@ -329,6 +333,7 @@
frame.weight = 0xFF;
// Process the full frame.
+ EXPECT_CALL(visitor_, OnPriorityFrameStart(Http3FrameLengths(2, 4)));
EXPECT_CALL(visitor_, OnPriorityFrame(frame));
EXPECT_EQ(QUIC_ARRAYSIZE(input),
decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
@@ -336,6 +341,7 @@
EXPECT_EQ("", decoder_.error_detail());
// Process the frame incremently.
+ EXPECT_CALL(visitor_, OnPriorityFrameStart(Http3FrameLengths(2, 4)));
EXPECT_CALL(visitor_, OnPriorityFrame(frame));
for (char c : input) {
EXPECT_EQ(1u, decoder_.ProcessInput(&c, 1));
@@ -357,6 +363,7 @@
frame2.exclusive = true;
frame2.weight = 0xFF;
+ EXPECT_CALL(visitor_, OnPriorityFrameStart(Http3FrameLengths(2, 2)));
EXPECT_CALL(visitor_, OnPriorityFrame(frame2));
EXPECT_EQ(QUIC_ARRAYSIZE(input2),
decoder_.ProcessInput(input2, QUIC_ARRAYSIZE(input2)));
@@ -364,10 +371,11 @@
EXPECT_EQ("", decoder_.error_detail());
// Test on the situation when the visitor wants to stop processing.
+ EXPECT_CALL(visitor_, OnPriorityFrameStart(Http3FrameLengths(2, 4)));
EXPECT_CALL(visitor_, OnPriorityFrame(frame)).WillOnce(Return(false));
EXPECT_EQ(0u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder_.error());
- EXPECT_EQ("Visitor shut down.", decoder_.error_detail());
+ EXPECT_EQ("Visitor shut down on PRIORITY frame.", decoder_.error_detail());
}
TEST_F(HttpDecoderTest, SettingsFrame) {
@@ -419,7 +427,8 @@
.WillOnce(Return(false));
EXPECT_EQ(0u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder_.error());
- EXPECT_EQ("Visitor shut down.", decoder_.error_detail());
+ EXPECT_EQ("Visitor shut down on SETTINGS frame start.",
+ decoder_.error_detail());
}
TEST_F(HttpDecoderTest, DataFrame) {
@@ -459,7 +468,7 @@
.WillOnce(Return(false));
EXPECT_EQ(0u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder_.error());
- EXPECT_EQ("Visitor shut down.", decoder_.error_detail());
+ EXPECT_EQ("Visitor shut down on DATA frame start.", decoder_.error_detail());
}
TEST_F(HttpDecoderTest, FrameHeaderPartialDelivery) {
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc
index 986c4e4..e7efad1 100644
--- a/quic/core/http/quic_receive_control_stream.cc
+++ b/quic/core/http/quic_receive_control_stream.cc
@@ -4,6 +4,7 @@
#include "net/third_party/quiche/src/quic/core/http/quic_receive_control_stream.h"
+#include "net/third_party/quiche/src/quic/core/http/http_decoder.h"
#include "net/third_party/quiche/src/quic/core/http/quic_spdy_session.h"
namespace quic {
@@ -24,6 +25,11 @@
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
}
+ bool OnPriorityFrameStart(Http3FrameLengths /*frame_lengths*/) override {
+ CloseConnectionOnWrongFrame("Priority");
+ return false;
+ }
+
bool OnPriorityFrame(const PriorityFrame& /*frame*/) override {
CloseConnectionOnWrongFrame("Priority");
return false;
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index aa20771..02601ba 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -8,6 +8,7 @@
#include <string>
#include <utility>
+#include "net/third_party/quiche/src/quic/core/http/http_decoder.h"
#include "net/third_party/quiche/src/quic/core/http/quic_spdy_session.h"
#include "net/third_party/quiche/src/quic/core/http/spdy_utils.h"
#include "net/third_party/quiche/src/quic/core/qpack/qpack_decoded_headers_accumulator.h"
@@ -45,6 +46,11 @@
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
}
+ bool OnPriorityFrameStart(Http3FrameLengths /*frame_lengths*/) override {
+ CloseConnectionOnWrongFrame("Priority");
+ return false;
+ }
+
bool OnPriorityFrame(const PriorityFrame& /*frame*/) override {
CloseConnectionOnWrongFrame("Priority");
return false;
