Do not call HttpDecoder::ProcessInput after error.
This CL changes API contract and call sites. Without the change to the call
sites, //gfe/gfe2/test_tools/fuzzing:gfe_quic_fuzzer would crash on the new
DCHECK_NE(QUIC_NO_ERROR, error_). This is because even if Visitor::OnError()
closes the connection, the sequencer might have more data that QuicSpdyStream is
eager to read and feed to HttpDecoder::ProcessInput().
Currently HttpDecoder::ProcessInput() ignores input upon error. However, I'm
going to modify the call sites to use PeekRegion() instead of
PrefetchNextRegion() and spin until decoding is paused by a visitor method or no
new region is there to peek at. Since HttpDecoder::ProcessInput() returns 0
if called after an error, this would just result in PeekRegion() being called
with the same offset, returning true, and the cycle spinning in an infinite
loop.
gfe-relnote: n/a, change to QUIC v99-only code path.
PiperOrigin-RevId: 255168809
Change-Id: I6b6f0b0986f2f6bec0a8eedb695dc91943d57bf2
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index 1bcbfaa..ffd5c74 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -45,10 +45,17 @@
HttpDecoder::~HttpDecoder() {}
QuicByteCount HttpDecoder::ProcessInput(const char* data, QuicByteCount len) {
+ DCHECK_EQ(QUIC_NO_ERROR, error_);
+ DCHECK_NE(STATE_ERROR, state_);
+
QuicDataReader reader(data, len);
bool continue_processing = true;
- while (continue_processing && error_ == QUIC_NO_ERROR &&
+ while (continue_processing &&
(reader.BytesRemaining() != 0 || state_ == STATE_FINISH_PARSING)) {
+ // |continue_processing| must have been set to false upon error.
+ DCHECK_EQ(QUIC_NO_ERROR, error_);
+ DCHECK_NE(STATE_ERROR, state_);
+
switch (state_) {
case STATE_READING_FRAME_TYPE:
ReadFrameType(&reader);
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h
index fa0a27b..515daae 100644
--- a/quic/core/http/http_decoder.h
+++ b/quic/core/http/http_decoder.h
@@ -124,10 +124,14 @@
// visitor method returns false or an error occurs. Returns the number of
// bytes processed. Does not process any input if called after an error.
// Paused processing can be resumed by calling ProcessInput() again with the
- // unprocessed portion of data.
+ // unprocessed portion of data. Must not be called after an error has
+ // occurred.
QuicByteCount ProcessInput(const char* data, QuicByteCount len);
+ // Returns an error code other than QUIC_NO_ERROR if and only if
+ // Visitor::OnError() has been called.
QuicErrorCode error() const { return error_; }
+
const std::string& error_detail() const { return error_detail_; }
private:
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc
index e7efad1..9e166e0 100644
--- a/quic/core/http/quic_receive_control_stream.cc
+++ b/quic/core/http/quic_receive_control_stream.cc
@@ -141,7 +141,8 @@
void QuicReceiveControlStream::OnDataAvailable() {
iovec iov;
- while (!reading_stopped() && sequencer()->PrefetchNextRegion(&iov)) {
+ while (!reading_stopped() && decoder_.error() == QUIC_NO_ERROR &&
+ sequencer()->PrefetchNextRegion(&iov)) {
decoder_.ProcessInput(reinterpret_cast<const char*>(iov.iov_base),
iov.iov_len);
}
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index 92138a4..b7522c0 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -623,7 +623,8 @@
}
iovec iov;
- while (!reading_stopped() && sequencer()->PrefetchNextRegion(&iov)) {
+ while (!reading_stopped() && decoder_.error() == QUIC_NO_ERROR &&
+ sequencer()->PrefetchNextRegion(&iov)) {
DCHECK(!sequencer()->IsClosed());
decoder_.ProcessInput(reinterpret_cast<const char*>(iov.iov_base),
iov.iov_len);
