Have server-side OgHttp2Session send a GOAWAY on connection error.
OgHttp2Session::LatchErrorAndNotify() means the connection is not long for this
world, so this CL updates the method to send a (server-side) GOAWAY.
PiperOrigin-RevId: 403179565
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index e1c540d..34a39eb 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -1438,13 +1438,18 @@
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x0, 0));
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::INTERNAL_ERROR)));
int send_result = adapter->Send();
// Some bytes should have been serialized.
EXPECT_EQ(0, send_result);
- // SETTINGS and SETTINGS ack
+ // SETTINGS, SETTINGS ack, and GOAWAY
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::SETTINGS}));
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::GOAWAY}));
}
// Exercises the case when a visitor chooses to reject a frame based solely on
@@ -1487,13 +1492,18 @@
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x0, 0));
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::INTERNAL_ERROR)));
int send_result = adapter->Send();
// Some bytes should have been serialized.
EXPECT_EQ(0, send_result);
- // SETTINGS and SETTINGS ack
+ // SETTINGS, SETTINGS ack, and GOAWAY
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::SETTINGS}));
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::GOAWAY}));
}
TEST(OgHttp2AdapterServerTest, ServerRejectsBeginningOfData) {
@@ -1547,13 +1557,18 @@
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x0, 0));
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::INTERNAL_ERROR)));
int send_result = adapter->Send();
// Some bytes should have been serialized.
EXPECT_EQ(0, send_result);
- // SETTINGS and SETTINGS ack.
+ // SETTINGS, SETTINGS ack, and GOAWAY.
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::SETTINGS}));
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::GOAWAY}));
}
TEST(OgHttp2AdapterServerTest, ServerRejectsStreamData) {
@@ -1607,13 +1622,18 @@
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x0, 0));
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::INTERNAL_ERROR)));
int send_result = adapter->Send();
// Some bytes should have been serialized.
EXPECT_EQ(0, send_result);
- // SETTINGS and SETTINGS ack.
+ // SETTINGS, SETTINGS ack, and GOAWAY.
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::SETTINGS}));
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::GOAWAY}));
}
// Exercises a naive mutually recursive test client and server. This test fails
@@ -1732,13 +1752,18 @@
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x0, 0));
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 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();
// Some bytes should have been serialized.
EXPECT_EQ(0, send_result);
- // SETTINGS and SETTINGS ack.
+ // SETTINGS, SETTINGS ack, and GOAWAY.
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
- spdy::SpdyFrameType::SETTINGS}));
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::GOAWAY}));
}
} // namespace
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 1f8fcf2..8c5e6c8 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -17,6 +17,7 @@
namespace {
using ConnectionError = Http2VisitorInterface::ConnectionError;
+using SpdyFramerError = Http2DecoderAdapter::SpdyFramerError;
// #define OGHTTP2_DEBUG_TRACE 1
@@ -146,6 +147,47 @@
std::function<void()> f_;
};
+Http2ErrorCode GetHttp2ErrorCode(SpdyFramerError error) {
+ switch (error) {
+ case SpdyFramerError::SPDY_NO_ERROR:
+ return Http2ErrorCode::NO_ERROR;
+ case SpdyFramerError::SPDY_INVALID_STREAM_ID:
+ case SpdyFramerError::SPDY_INVALID_CONTROL_FRAME:
+ case SpdyFramerError::SPDY_INVALID_PADDING:
+ case SpdyFramerError::SPDY_INVALID_DATA_FRAME_FLAGS:
+ case SpdyFramerError::SPDY_UNEXPECTED_FRAME:
+ return Http2ErrorCode::PROTOCOL_ERROR;
+ case SpdyFramerError::SPDY_CONTROL_PAYLOAD_TOO_LARGE:
+ case SpdyFramerError::SPDY_INVALID_CONTROL_FRAME_SIZE:
+ case SpdyFramerError::SPDY_OVERSIZED_PAYLOAD:
+ return Http2ErrorCode::FRAME_SIZE_ERROR;
+ case SpdyFramerError::SPDY_DECOMPRESS_FAILURE:
+ case SpdyFramerError::SPDY_HPACK_INDEX_VARINT_ERROR:
+ case SpdyFramerError::SPDY_HPACK_NAME_LENGTH_VARINT_ERROR:
+ case SpdyFramerError::SPDY_HPACK_VALUE_LENGTH_VARINT_ERROR:
+ case SpdyFramerError::SPDY_HPACK_NAME_TOO_LONG:
+ case SpdyFramerError::SPDY_HPACK_VALUE_TOO_LONG:
+ case SpdyFramerError::SPDY_HPACK_NAME_HUFFMAN_ERROR:
+ case SpdyFramerError::SPDY_HPACK_VALUE_HUFFMAN_ERROR:
+ case SpdyFramerError::SPDY_HPACK_MISSING_DYNAMIC_TABLE_SIZE_UPDATE:
+ case SpdyFramerError::SPDY_HPACK_INVALID_INDEX:
+ case SpdyFramerError::SPDY_HPACK_INVALID_NAME_INDEX:
+ case SpdyFramerError::SPDY_HPACK_DYNAMIC_TABLE_SIZE_UPDATE_NOT_ALLOWED:
+ case SpdyFramerError::
+ SPDY_HPACK_INITIAL_DYNAMIC_TABLE_SIZE_UPDATE_IS_ABOVE_LOW_WATER_MARK:
+ case SpdyFramerError::
+ SPDY_HPACK_DYNAMIC_TABLE_SIZE_UPDATE_IS_ABOVE_ACKNOWLEDGED_SETTING:
+ case SpdyFramerError::SPDY_HPACK_TRUNCATED_BLOCK:
+ case SpdyFramerError::SPDY_HPACK_FRAGMENT_TOO_LONG:
+ case SpdyFramerError::SPDY_HPACK_COMPRESSED_HEADER_SIZE_EXCEEDS_LIMIT:
+ return Http2ErrorCode::COMPRESSION_ERROR;
+ case SpdyFramerError::SPDY_INTERNAL_FRAMER_ERROR:
+ case SpdyFramerError::SPDY_STOP_PROCESSING:
+ case SpdyFramerError::LAST_ERROR:
+ return Http2ErrorCode::INTERNAL_ERROR;
+ }
+}
+
} // namespace
void OgHttp2Session::PassthroughHeadersHandler::OnHeaderBlockStart() {
@@ -301,7 +343,8 @@
QUICHE_DLOG(INFO) << "Preface doesn't match! Expected: ["
<< absl::CEscape(remaining_preface_) << "], actual: ["
<< absl::CEscape(bytes) << "]";
- LatchErrorAndNotify(ConnectionError::kInvalidConnectionPreface);
+ LatchErrorAndNotify(Http2ErrorCode::PROTOCOL_ERROR,
+ ConnectionError::kInvalidConnectionPreface);
return -1;
}
remaining_preface_.remove_prefix(min_size);
@@ -380,7 +423,8 @@
}
}
if (result < 0) {
- LatchErrorAndNotify(ConnectionError::kSendError);
+ LatchErrorAndNotify(Http2ErrorCode::INTERNAL_ERROR,
+ ConnectionError::kSendError);
return result;
} else if (!serialized_prefix_.empty()) {
return 0;
@@ -416,7 +460,8 @@
spdy::SpdySerializedFrame frame = framer_.SerializeFrame(*frame_ptr);
const int64_t result = visitor_.OnReadyToSend(absl::string_view(frame));
if (result < 0) {
- LatchErrorAndNotify(ConnectionError::kSendError);
+ LatchErrorAndNotify(Http2ErrorCode::INTERNAL_ERROR,
+ ConnectionError::kSendError);
return false;
} else if (result == 0) {
// Write blocked.
@@ -653,12 +698,13 @@
}
}
-void OgHttp2Session::OnError(http2::Http2DecoderAdapter::SpdyFramerError error,
+void OgHttp2Session::OnError(SpdyFramerError error,
std::string detailed_error) {
QUICHE_VLOG(1) << "Error: "
<< http2::Http2DecoderAdapter::SpdyFramerErrorToString(error)
<< " details: " << detailed_error;
- LatchErrorAndNotify(ConnectionError::kParseError);
+ // TODO(diannahu): Consider propagating `detailed_error`.
+ LatchErrorAndNotify(GetHttp2ErrorCode(error), ConnectionError::kParseError);
}
void OgHttp2Session::OnCommonHeader(spdy::SpdyStreamId stream_id,
@@ -811,7 +857,8 @@
const auto new_stream_id = static_cast<Http2StreamId>(stream_id);
if (new_stream_id <= highest_processed_stream_id_) {
// A new stream ID lower than the watermark is a connection error.
- LatchErrorAndNotify(ConnectionError::kInvalidNewStreamId);
+ LatchErrorAndNotify(Http2ErrorCode::PROTOCOL_ERROR,
+ ConnectionError::kInvalidNewStreamId);
return;
}
CreateStream(stream_id);
@@ -873,7 +920,8 @@
stream_id, spdy::ERROR_CODE_INTERNAL_ERROR));
}
} else if (result == Http2VisitorInterface::HEADER_CONNECTION_ERROR) {
- LatchErrorAndNotify(ConnectionError::kHeaderError);
+ LatchErrorAndNotify(Http2ErrorCode::INTERNAL_ERROR,
+ ConnectionError::kHeaderError);
}
}
@@ -1048,7 +1096,8 @@
}
}
-void OgHttp2Session::LatchErrorAndNotify(ConnectionError error) {
+void OgHttp2Session::LatchErrorAndNotify(Http2ErrorCode error_code,
+ ConnectionError error) {
if (latched_error_) {
// Do not kick a connection when it is down.
return;
@@ -1057,6 +1106,11 @@
latched_error_ = true;
visitor_.OnConnectionError(error);
decoder_.StopProcessing();
+ if (IsServerSession()) {
+ EnqueueFrame(absl::make_unique<spdy::SpdyGoAwayIR>(
+ highest_processed_stream_id_, TranslateErrorCode(error_code),
+ ConnectionErrorToString(error)));
+ }
}
} // namespace adapter
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index af17230..31488f4 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -280,7 +280,10 @@
// Returns true if the session can create a new stream.
bool CanCreateStream() const;
- void LatchErrorAndNotify(Http2VisitorInterface::ConnectionError error);
+ // Informs the visitor of the connection `error` and stops processing on the
+ // connection. If server-side, also sends a GOAWAY with `error_code`.
+ void LatchErrorAndNotify(Http2ErrorCode error_code,
+ Http2VisitorInterface::ConnectionError error);
// Receives events when inbound frames are parsed.
Http2VisitorInterface& visitor_;
