When the HTTP/2 decoder encounters an error, OgHttp2Session latches the error state, and returns a negative integer from ProcessBytes.
Returning a negative value makes errors more readily apparent to users of the library.
PiperOrigin-RevId: 400815381
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index f663fb7..413db77 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -289,9 +289,8 @@
EXPECT_CALL(visitor, OnConnectionError());
const int64_t stream_result = adapter->ProcessBytes(stream_frames);
- // Unlike nghttp2, this adapter implementation returns the number of bytes
- // actually processed.
- EXPECT_LT(static_cast<size_t>(stream_result), stream_frames.size());
+ // Negative integer returned to indicate an error.
+ EXPECT_LT(stream_result, 0);
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
@@ -450,7 +449,7 @@
EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
const int64_t stream_result = adapter->ProcessBytes(stream_frames);
- EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
+ EXPECT_LT(stream_result, 0);
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
@@ -522,7 +521,7 @@
EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
const int64_t stream_result = adapter->ProcessBytes(stream_frames);
- EXPECT_EQ(static_cast<size_t>(stream_result), stream_frames.size());
+ EXPECT_LT(stream_result, 0);
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
@@ -681,8 +680,7 @@
EXPECT_CALL(visitor, OnConnectionError());
const int64_t stream_result = adapter->ProcessBytes(stream_frames);
- EXPECT_GT(stream_result, 0);
- EXPECT_LT(stream_result, static_cast<int64_t>(stream_frames.size()));
+ EXPECT_LT(stream_result, 0);
EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
@@ -1267,8 +1265,7 @@
EXPECT_CALL(visitor, OnConnectionError());
const int64_t result = adapter->ProcessBytes(frames);
- EXPECT_GT(result, 0);
- EXPECT_LT(result, static_cast<int64_t>(frames.size()));
+ EXPECT_LT(result, 0);
EXPECT_TRUE(adapter->want_write());
@@ -1317,8 +1314,7 @@
EXPECT_CALL(visitor, OnConnectionError());
const int64_t result = adapter->ProcessBytes(frames);
- EXPECT_GT(result, 0);
- EXPECT_LT(result, static_cast<int64_t>(frames.size()));
+ EXPECT_LT(result, 0);
EXPECT_TRUE(adapter->want_write());
@@ -1378,8 +1374,7 @@
EXPECT_CALL(visitor, OnConnectionError());
const int64_t result = adapter->ProcessBytes(frames);
- EXPECT_GT(result, 0);
- EXPECT_LT(result, static_cast<int64_t>(frames.size()));
+ EXPECT_LT(result, 0);
EXPECT_TRUE(adapter->want_write());
@@ -1439,8 +1434,7 @@
EXPECT_CALL(visitor, OnConnectionError());
const int64_t result = adapter->ProcessBytes(frames);
- EXPECT_GT(result, 0);
- EXPECT_LT(result, static_cast<int64_t>(frames.size()));
+ EXPECT_LT(result, 0);
EXPECT_TRUE(adapter->want_write());
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index eee9c3c..450a56d 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -258,7 +258,7 @@
QUICHE_DLOG(INFO) << "Preface doesn't match! Expected: ["
<< absl::CEscape(remaining_preface_) << "], actual: ["
<< absl::CEscape(bytes) << "]";
- visitor_.OnConnectionError();
+ LatchErrorAndNotify();
return -1;
}
remaining_preface_.remove_prefix(min_size);
@@ -271,7 +271,13 @@
preface_consumed = min_size;
}
int64_t result = decoder_.ProcessInput(bytes.data(), bytes.size());
- return result < 0 ? result : result + preface_consumed;
+ if (latched_error_) {
+ QUICHE_VLOG(2) << "ProcessBytes encountered an error.";
+ return -1;
+ }
+ const int64_t ret = result < 0 ? result : result + preface_consumed;
+ QUICHE_VLOG(2) << "ProcessBytes returning: " << ret;
+ return ret;
}
int OgHttp2Session::Consume(Http2StreamId stream_id, size_t num_bytes) {
@@ -323,7 +329,7 @@
}
}
if (result < 0) {
- visitor_.OnConnectionError();
+ LatchErrorAndNotify();
return result;
} else if (!serialized_prefix_.empty()) {
return 0;
@@ -359,7 +365,7 @@
spdy::SpdySerializedFrame frame = framer_.SerializeFrame(*frame_ptr);
const int64_t result = visitor_.OnReadyToSend(absl::string_view(frame));
if (result < 0) {
- visitor_.OnConnectionError();
+ LatchErrorAndNotify();
return false;
} else if (result == 0) {
// Write blocked.
@@ -601,7 +607,7 @@
QUICHE_VLOG(1) << "Error: "
<< http2::Http2DecoderAdapter::SpdyFramerErrorToString(error)
<< " details: " << detailed_error;
- visitor_.OnConnectionError();
+ LatchErrorAndNotify();
}
void OgHttp2Session::OnCommonHeader(spdy::SpdyStreamId stream_id,
@@ -793,7 +799,7 @@
stream_id, spdy::ERROR_CODE_INTERNAL_ERROR));
}
} else if (result == Http2VisitorInterface::HEADER_CONNECTION_ERROR) {
- visitor_.OnConnectionError();
+ LatchErrorAndNotify();
}
}
@@ -953,5 +959,10 @@
return stream_map_.size() < max_outbound_concurrent_streams_;
}
+void OgHttp2Session::LatchErrorAndNotify() {
+ latched_error_ = true;
+ visitor_.OnConnectionError();
+}
+
} // namespace adapter
} // namespace http2
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index a8beb0b..558b36c 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -259,6 +259,8 @@
// Returns true if the session can create a new stream.
bool CanCreateStream() const;
+ void LatchErrorAndNotify();
+
// Receives events when inbound frames are parsed.
Http2VisitorInterface& visitor_;
@@ -321,6 +323,7 @@
// Replace this with a stream ID, for multiple GOAWAY support.
bool queued_goaway_ = false;
+ bool latched_error_ = false;
};
} // namespace adapter
