Add ProcessBytesError entries for bad client magic and user callback failures.
This CL adds ProcessBytesError entries for invalid connection preface (bad
client magic) and visitor (user) callback failures. These values respectively
correspond to the nghttp2_session_mem_recv() return values
NGHTTP2_ERR_BAD_CLIENT_MAGIC and NGHTTP2_ERR_CALLBACK_FAILURE
(https://nghttp2.org/documentation/nghttp2_session_mem_recv.html).
This CL will allow OgHttp2Session in the future to return negative values for
these error scenarios while potentially handling other connection-level errors
differently, in line with nghttp2 behavior.
The codec_impl_test LazyDecreaseMaxConcurrentStreamsIgnoreError passes with
this CL (http://sponge2/1581019e-4ecf-4ec0-8869-a07e735f50cf).
PiperOrigin-RevId: 416873218
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 84d6473..781c49f 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -256,6 +256,7 @@
if (result_ == Http2VisitorInterface::HEADER_OK) {
const bool result = visitor_.OnEndHeadersForStream(stream_id_);
if (!result) {
+ session_.fatal_visitor_callback_failure_ = true;
session_.decoder_.StopProcessing();
}
} else {
@@ -272,6 +273,10 @@
switch (error) {
case ProcessBytesError::kUnspecified:
return -1;
+ case ProcessBytesError::kInvalidConnectionPreface:
+ return -903; // NGHTTP2_ERR_BAD_CLIENT_MAGIC
+ case ProcessBytesError::kVisitorCallbackFailed:
+ return -902; // NGHTTP2_ERR_CALLBACK_FAILURE
}
return -1;
}
@@ -415,7 +420,7 @@
<< absl::CEscape(bytes) << "]";
LatchErrorAndNotify(Http2ErrorCode::PROTOCOL_ERROR,
ConnectionError::kInvalidConnectionPreface);
- return ProcessBytesError::kUnspecified;
+ return ProcessBytesError::kInvalidConnectionPreface;
}
remaining_preface_.remove_prefix(min_size);
bytes.remove_prefix(min_size);
@@ -428,6 +433,11 @@
}
int64_t result = decoder_.ProcessInput(bytes.data(), bytes.size());
QUICHE_VLOG(2) << "ProcessBytes result: " << result;
+ if (fatal_visitor_callback_failure_) {
+ QUICHE_DCHECK(latched_error_);
+ QUICHE_VLOG(2) << "Visitor callback failed while processing bytes.";
+ return ProcessBytesError::kVisitorCallbackFailed;
+ }
if (latched_error_ || result < 0) {
QUICHE_VLOG(2) << "ProcessBytes encountered an error.";
return ProcessBytesError::kUnspecified;
@@ -896,6 +906,7 @@
highest_received_stream_id_);
const bool result = visitor_.OnFrameHeader(stream_id, length, type, flags);
if (!result) {
+ fatal_visitor_callback_failure_ = true;
decoder_.StopProcessing();
}
}
@@ -915,6 +926,7 @@
const bool result = visitor_.OnBeginDataForStream(stream_id, length);
if (!result) {
+ fatal_visitor_callback_failure_ = true;
decoder_.StopProcessing();
}
}
@@ -934,6 +946,7 @@
const bool result =
visitor_.OnDataForStream(stream_id, absl::string_view(data, len));
if (!result) {
+ fatal_visitor_callback_failure_ = true;
decoder_.StopProcessing();
}
}
@@ -1076,6 +1089,7 @@
const bool result = visitor_.OnGoAway(last_accepted_stream_id,
TranslateErrorCode(error_code), "");
if (!result) {
+ fatal_visitor_callback_failure_ = true;
decoder_.StopProcessing();
}
}
@@ -1116,8 +1130,11 @@
// The new stream would exceed our advertised and acknowledged
// MAX_CONCURRENT_STREAMS. For parity with nghttp2, treat this error as a
// connection-level PROTOCOL_ERROR.
- visitor_.OnInvalidFrame(
+ bool ok = visitor_.OnInvalidFrame(
stream_id, Http2VisitorInterface::InvalidFrameError::kProtocol);
+ if (!ok) {
+ fatal_visitor_callback_failure_ = true;
+ }
LatchErrorAndNotify(Http2ErrorCode::PROTOCOL_ERROR,
ConnectionError::kExceededMaxConcurrentStreams);
return;
@@ -1130,6 +1147,7 @@
const bool ok = visitor_.OnInvalidFrame(
stream_id, Http2VisitorInterface::InvalidFrameError::kRefusedStream);
if (!ok) {
+ fatal_visitor_callback_failure_ = true;
LatchErrorAndNotify(Http2ErrorCode::REFUSED_STREAM,
ConnectionError::kExceededMaxConcurrentStreams);
}
@@ -1224,11 +1242,13 @@
result == Http2VisitorInterface::HEADER_HTTP_MESSAGING) {
const bool ok = visitor_.OnInvalidFrame(stream_id, frame_error);
if (!ok) {
+ fatal_visitor_callback_failure_ = true;
LatchErrorAndNotify(error_code, ConnectionError::kHeaderError);
}
}
}
} else if (result == Http2VisitorInterface::HEADER_CONNECTION_ERROR) {
+ fatal_visitor_callback_failure_ = true;
LatchErrorAndNotify(Http2ErrorCode::INTERNAL_ERROR,
ConnectionError::kHeaderError);
}
@@ -1263,6 +1283,7 @@
end_metadata_ = false;
}
} else {
+ fatal_visitor_callback_failure_ = true;
decoder_.StopProcessing();
}
} else {
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index c3fbab2..f7c7779 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -293,6 +293,10 @@
enum class ProcessBytesError {
// A general, unspecified error.
kUnspecified,
+ // The (server-side) session received an invalid client connection preface.
+ kInvalidConnectionPreface,
+ // A user/visitor callback failed with a fatal error.
+ kVisitorCallbackFailed,
};
using ProcessBytesResult = absl::variant<int64_t, ProcessBytesError>;
@@ -470,6 +474,9 @@
// True if a fatal sending error has occurred.
bool fatal_send_error_ = false;
+
+ // True if a fatal processing visitor callback failed.
+ bool fatal_visitor_callback_failure_ = false;
};
} // namespace adapter
