Modernize the HTTP/3 HttpDecoder by using non-const references in place
of non-null pointers, when they do not outlive the call. Annotate
with absl::NonNull when pointer do outlive the call.
PiperOrigin-RevId: 656422493
diff --git a/quiche/quic/core/http/http_decoder.cc b/quiche/quic/core/http/http_decoder.cc
index 1df1416..00de94f 100644
--- a/quiche/quic/core/http/http_decoder.cc
+++ b/quiche/quic/core/http/http_decoder.cc
@@ -112,16 +112,16 @@
switch (state_) {
case STATE_READING_FRAME_TYPE:
- continue_processing = ReadFrameType(&reader);
+ continue_processing = ReadFrameType(reader);
break;
case STATE_READING_FRAME_LENGTH:
- continue_processing = ReadFrameLength(&reader);
+ continue_processing = ReadFrameLength(reader);
break;
case STATE_BUFFER_OR_PARSE_PAYLOAD:
- continue_processing = BufferOrParsePayload(&reader);
+ continue_processing = BufferOrParsePayload(reader);
break;
case STATE_READING_FRAME_PAYLOAD:
- continue_processing = ReadFramePayload(&reader);
+ continue_processing = ReadFramePayload(reader);
break;
case STATE_FINISH_PARSING:
continue_processing = FinishParsing();
@@ -145,20 +145,20 @@
return len - reader.BytesRemaining();
}
-bool HttpDecoder::ReadFrameType(QuicDataReader* reader) {
- QUICHE_DCHECK_NE(0u, reader->BytesRemaining());
+bool HttpDecoder::ReadFrameType(QuicDataReader& reader) {
+ QUICHE_DCHECK_NE(0u, reader.BytesRemaining());
if (current_type_field_length_ == 0) {
// A new frame is coming.
- current_type_field_length_ = reader->PeekVarInt62Length();
+ current_type_field_length_ = reader.PeekVarInt62Length();
QUICHE_DCHECK_NE(0u, current_type_field_length_);
- if (current_type_field_length_ > reader->BytesRemaining()) {
+ if (current_type_field_length_ > reader.BytesRemaining()) {
// Buffer a new type field.
remaining_type_field_length_ = current_type_field_length_;
BufferFrameType(reader);
return true;
}
// The reader has all type data needed, so no need to buffer.
- bool success = reader->ReadVarInt62(¤t_frame_type_);
+ bool success = reader.ReadVarInt62(¤t_frame_type_);
QUICHE_DCHECK(success);
} else {
// Buffer the existing type field.
@@ -203,20 +203,20 @@
return true;
}
-bool HttpDecoder::ReadFrameLength(QuicDataReader* reader) {
- QUICHE_DCHECK_NE(0u, reader->BytesRemaining());
+bool HttpDecoder::ReadFrameLength(QuicDataReader& reader) {
+ QUICHE_DCHECK_NE(0u, reader.BytesRemaining());
if (current_length_field_length_ == 0) {
// A new frame is coming.
- current_length_field_length_ = reader->PeekVarInt62Length();
+ current_length_field_length_ = reader.PeekVarInt62Length();
QUICHE_DCHECK_NE(0u, current_length_field_length_);
- if (current_length_field_length_ > reader->BytesRemaining()) {
+ if (current_length_field_length_ > reader.BytesRemaining()) {
// Buffer a new length field.
remaining_length_field_length_ = current_length_field_length_;
BufferFrameLength(reader);
return true;
}
// The reader has all length data needed, so no need to buffer.
- bool success = reader->ReadVarInt62(¤t_frame_length_);
+ bool success = reader.ReadVarInt62(¤t_frame_length_);
QUICHE_DCHECK(success);
} else {
// Buffer the existing length field.
@@ -284,13 +284,11 @@
case static_cast<uint64_t>(HttpFrameType::ACCEPT_CH):
continue_processing = visitor_->OnAcceptChFrameStart(header_length);
break;
+ case static_cast<uint64_t>(HttpFrameType::METADATA):
+ continue_processing =
+ visitor_->OnMetadataFrameStart(header_length, current_frame_length_);
+ break;
default:
- if (current_frame_type_ ==
- static_cast<uint64_t>(HttpFrameType::METADATA)) {
- continue_processing = visitor_->OnMetadataFrameStart(
- header_length, current_frame_length_);
- break;
- }
continue_processing = visitor_->OnUnknownFrameStart(
current_frame_type_, header_length, current_frame_length_);
break;
@@ -326,9 +324,9 @@
return false;
}
-bool HttpDecoder::ReadFramePayload(QuicDataReader* reader) {
+bool HttpDecoder::ReadFramePayload(QuicDataReader& reader) {
QUICHE_DCHECK(!IsFrameBuffered());
- QUICHE_DCHECK_NE(0u, reader->BytesRemaining());
+ QUICHE_DCHECK_NE(0u, reader.BytesRemaining());
QUICHE_DCHECK_NE(0u, remaining_frame_length_);
bool continue_processing = true;
@@ -336,9 +334,9 @@
switch (current_frame_type_) {
case static_cast<uint64_t>(HttpFrameType::DATA): {
QuicByteCount bytes_to_read = std::min<QuicByteCount>(
- remaining_frame_length_, reader->BytesRemaining());
+ remaining_frame_length_, reader.BytesRemaining());
absl::string_view payload;
- bool success = reader->ReadStringPiece(&payload, bytes_to_read);
+ bool success = reader.ReadStringPiece(&payload, bytes_to_read);
QUICHE_DCHECK(success);
QUICHE_DCHECK(!payload.empty());
continue_processing = visitor_->OnDataFramePayload(payload);
@@ -347,9 +345,9 @@
}
case static_cast<uint64_t>(HttpFrameType::HEADERS): {
QuicByteCount bytes_to_read = std::min<QuicByteCount>(
- remaining_frame_length_, reader->BytesRemaining());
+ remaining_frame_length_, reader.BytesRemaining());
absl::string_view payload;
- bool success = reader->ReadStringPiece(&payload, bytes_to_read);
+ bool success = reader.ReadStringPiece(&payload, bytes_to_read);
QUICHE_DCHECK(success);
QUICHE_DCHECK(!payload.empty());
continue_processing = visitor_->OnHeadersFramePayload(payload);
@@ -384,19 +382,18 @@
QUICHE_NOTREACHED();
break;
}
+ case static_cast<uint64_t>(HttpFrameType::METADATA): {
+ QuicByteCount bytes_to_read = std::min<QuicByteCount>(
+ remaining_frame_length_, reader.BytesRemaining());
+ absl::string_view payload;
+ bool success = reader.ReadStringPiece(&payload, bytes_to_read);
+ QUICHE_DCHECK(success);
+ QUICHE_DCHECK(!payload.empty());
+ continue_processing = visitor_->OnMetadataFramePayload(payload);
+ remaining_frame_length_ -= payload.length();
+ break;
+ }
default: {
- if (current_frame_type_ ==
- static_cast<uint64_t>(HttpFrameType::METADATA)) {
- QuicByteCount bytes_to_read = std::min<QuicByteCount>(
- remaining_frame_length_, reader->BytesRemaining());
- absl::string_view payload;
- bool success = reader->ReadStringPiece(&payload, bytes_to_read);
- QUICHE_DCHECK(success);
- QUICHE_DCHECK(!payload.empty());
- continue_processing = visitor_->OnMetadataFramePayload(payload);
- remaining_frame_length_ -= payload.length();
- break;
- }
continue_processing = HandleUnknownFramePayload(reader);
break;
}
@@ -452,12 +449,11 @@
QUICHE_NOTREACHED();
break;
}
+ case static_cast<uint64_t>(HttpFrameType::METADATA): {
+ continue_processing = visitor_->OnMetadataFrameEnd();
+ break;
+ }
default:
- if (current_frame_type_ ==
- static_cast<uint64_t>(HttpFrameType::METADATA)) {
- continue_processing = visitor_->OnMetadataFrameEnd();
- break;
- }
continue_processing = visitor_->OnUnknownFrameEnd();
}
@@ -471,61 +467,61 @@
state_ = STATE_READING_FRAME_TYPE;
}
-bool HttpDecoder::HandleUnknownFramePayload(QuicDataReader* reader) {
- QuicByteCount bytes_to_read = std::min<QuicByteCount>(
- remaining_frame_length_, reader->BytesRemaining());
+bool HttpDecoder::HandleUnknownFramePayload(QuicDataReader& reader) {
+ QuicByteCount bytes_to_read =
+ std::min<QuicByteCount>(remaining_frame_length_, reader.BytesRemaining());
absl::string_view payload;
- bool success = reader->ReadStringPiece(&payload, bytes_to_read);
+ bool success = reader.ReadStringPiece(&payload, bytes_to_read);
QUICHE_DCHECK(success);
QUICHE_DCHECK(!payload.empty());
remaining_frame_length_ -= payload.length();
return visitor_->OnUnknownFramePayload(payload);
}
-bool HttpDecoder::BufferOrParsePayload(QuicDataReader* reader) {
+bool HttpDecoder::BufferOrParsePayload(QuicDataReader& reader) {
QUICHE_DCHECK(IsFrameBuffered());
QUICHE_DCHECK_EQ(current_frame_length_,
buffer_.size() + remaining_frame_length_);
- if (buffer_.empty() && reader->BytesRemaining() >= current_frame_length_) {
+ if (buffer_.empty() && reader.BytesRemaining() >= current_frame_length_) {
// |*reader| contains entire payload, which might be empty.
remaining_frame_length_ = 0;
- QuicDataReader current_payload_reader(reader->PeekRemainingPayload().data(),
+ QuicDataReader current_payload_reader(reader.PeekRemainingPayload().data(),
current_frame_length_);
- bool continue_processing = ParseEntirePayload(¤t_payload_reader);
+ bool continue_processing = ParseEntirePayload(current_payload_reader);
- reader->Seek(current_frame_length_);
+ reader.Seek(current_frame_length_);
ResetForNextFrame();
return continue_processing;
}
// Buffer as much of the payload as |*reader| contains.
- QuicByteCount bytes_to_read = std::min<QuicByteCount>(
- remaining_frame_length_, reader->BytesRemaining());
- absl::StrAppend(&buffer_, reader->PeekRemainingPayload().substr(
+ QuicByteCount bytes_to_read =
+ std::min<QuicByteCount>(remaining_frame_length_, reader.BytesRemaining());
+ absl::StrAppend(&buffer_, reader.PeekRemainingPayload().substr(
/* pos = */ 0, bytes_to_read));
- reader->Seek(bytes_to_read);
+ reader.Seek(bytes_to_read);
remaining_frame_length_ -= bytes_to_read;
QUICHE_DCHECK_EQ(current_frame_length_,
buffer_.size() + remaining_frame_length_);
if (remaining_frame_length_ > 0) {
- QUICHE_DCHECK(reader->IsDoneReading());
+ QUICHE_DCHECK(reader.IsDoneReading());
return false;
}
QuicDataReader buffer_reader(buffer_);
- bool continue_processing = ParseEntirePayload(&buffer_reader);
+ bool continue_processing = ParseEntirePayload(buffer_reader);
buffer_.clear();
ResetForNextFrame();
return continue_processing;
}
-bool HttpDecoder::ParseEntirePayload(QuicDataReader* reader) {
+bool HttpDecoder::ParseEntirePayload(QuicDataReader& reader) {
QUICHE_DCHECK(IsFrameBuffered());
- QUICHE_DCHECK_EQ(current_frame_length_, reader->BytesRemaining());
+ QUICHE_DCHECK_EQ(current_frame_length_, reader.BytesRemaining());
QUICHE_DCHECK_EQ(0u, remaining_frame_length_);
switch (current_frame_type_) {
@@ -535,18 +531,18 @@
}
case static_cast<uint64_t>(HttpFrameType::SETTINGS): {
SettingsFrame frame;
- if (!ParseSettingsFrame(reader, &frame)) {
+ if (!ParseSettingsFrame(reader, frame)) {
return false;
}
return visitor_->OnSettingsFrame(frame);
}
case static_cast<uint64_t>(HttpFrameType::GOAWAY): {
GoAwayFrame frame;
- if (!reader->ReadVarInt62(&frame.id)) {
+ if (!reader.ReadVarInt62(&frame.id)) {
RaiseError(QUIC_HTTP_FRAME_ERROR, "Unable to read GOAWAY ID.");
return false;
}
- if (!reader->IsDoneReading()) {
+ if (!reader.IsDoneReading()) {
RaiseError(QUIC_HTTP_FRAME_ERROR, "Superfluous data in GOAWAY frame.");
return false;
}
@@ -554,12 +550,12 @@
}
case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): {
uint64_t unused;
- if (!reader->ReadVarInt62(&unused)) {
+ if (!reader.ReadVarInt62(&unused)) {
RaiseError(QUIC_HTTP_FRAME_ERROR,
"Unable to read MAX_PUSH_ID push_id.");
return false;
}
- if (!reader->IsDoneReading()) {
+ if (!reader.IsDoneReading()) {
RaiseError(QUIC_HTTP_FRAME_ERROR,
"Superfluous data in MAX_PUSH_ID frame.");
return false;
@@ -568,14 +564,14 @@
}
case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): {
PriorityUpdateFrame frame;
- if (!ParsePriorityUpdateFrame(reader, &frame)) {
+ if (!ParsePriorityUpdateFrame(reader, frame)) {
return false;
}
return visitor_->OnPriorityUpdateFrame(frame);
}
case static_cast<uint64_t>(HttpFrameType::ACCEPT_CH): {
AcceptChFrame frame;
- if (!ParseAcceptChFrame(reader, &frame)) {
+ if (!ParseAcceptChFrame(reader, frame)) {
return false;
}
return visitor_->OnAcceptChFrame(frame);
@@ -587,24 +583,24 @@
}
}
-void HttpDecoder::BufferFrameLength(QuicDataReader* reader) {
+void HttpDecoder::BufferFrameLength(QuicDataReader& reader) {
QuicByteCount bytes_to_read = std::min<QuicByteCount>(
- remaining_length_field_length_, reader->BytesRemaining());
+ remaining_length_field_length_, reader.BytesRemaining());
bool success =
- reader->ReadBytes(length_buffer_.data() + current_length_field_length_ -
- remaining_length_field_length_,
- bytes_to_read);
+ reader.ReadBytes(length_buffer_.data() + current_length_field_length_ -
+ remaining_length_field_length_,
+ bytes_to_read);
QUICHE_DCHECK(success);
remaining_length_field_length_ -= bytes_to_read;
}
-void HttpDecoder::BufferFrameType(QuicDataReader* reader) {
+void HttpDecoder::BufferFrameType(QuicDataReader& reader) {
QuicByteCount bytes_to_read = std::min<QuicByteCount>(
- remaining_type_field_length_, reader->BytesRemaining());
+ remaining_type_field_length_, reader.BytesRemaining());
bool success =
- reader->ReadBytes(type_buffer_.data() + current_type_field_length_ -
- remaining_type_field_length_,
- bytes_to_read);
+ reader.ReadBytes(type_buffer_.data() + current_type_field_length_ -
+ remaining_type_field_length_,
+ bytes_to_read);
QUICHE_DCHECK(success);
remaining_type_field_length_ -= bytes_to_read;
}
@@ -616,20 +612,20 @@
visitor_->OnError(this);
}
-bool HttpDecoder::ParseSettingsFrame(QuicDataReader* reader,
- SettingsFrame* frame) {
- while (!reader->IsDoneReading()) {
+bool HttpDecoder::ParseSettingsFrame(QuicDataReader& reader,
+ SettingsFrame& frame) {
+ while (!reader.IsDoneReading()) {
uint64_t id;
- if (!reader->ReadVarInt62(&id)) {
+ if (!reader.ReadVarInt62(&id)) {
RaiseError(QUIC_HTTP_FRAME_ERROR, "Unable to read setting identifier.");
return false;
}
uint64_t content;
- if (!reader->ReadVarInt62(&content)) {
+ if (!reader.ReadVarInt62(&content)) {
RaiseError(QUIC_HTTP_FRAME_ERROR, "Unable to read setting value.");
return false;
}
- auto result = frame->values.insert({id, content});
+ auto result = frame.values.insert({id, content});
if (!result.second) {
RaiseError(QUIC_HTTP_DUPLICATE_SETTING_IDENTIFIER,
"Duplicate setting identifier.");
@@ -639,36 +635,36 @@
return true;
}
-bool HttpDecoder::ParsePriorityUpdateFrame(QuicDataReader* reader,
- PriorityUpdateFrame* frame) {
- if (!reader->ReadVarInt62(&frame->prioritized_element_id)) {
+bool HttpDecoder::ParsePriorityUpdateFrame(QuicDataReader& reader,
+ PriorityUpdateFrame& frame) {
+ if (!reader.ReadVarInt62(&frame.prioritized_element_id)) {
RaiseError(QUIC_HTTP_FRAME_ERROR, "Unable to read prioritized element id.");
return false;
}
- absl::string_view priority_field_value = reader->ReadRemainingPayload();
- frame->priority_field_value =
+ absl::string_view priority_field_value = reader.ReadRemainingPayload();
+ frame.priority_field_value =
std::string(priority_field_value.data(), priority_field_value.size());
return true;
}
-bool HttpDecoder::ParseAcceptChFrame(QuicDataReader* reader,
- AcceptChFrame* frame) {
+bool HttpDecoder::ParseAcceptChFrame(QuicDataReader& reader,
+ AcceptChFrame& frame) {
absl::string_view origin;
absl::string_view value;
- while (!reader->IsDoneReading()) {
- if (!reader->ReadStringPieceVarInt62(&origin)) {
+ while (!reader.IsDoneReading()) {
+ if (!reader.ReadStringPieceVarInt62(&origin)) {
RaiseError(QUIC_HTTP_FRAME_ERROR, "Unable to read ACCEPT_CH origin.");
return false;
}
- if (!reader->ReadStringPieceVarInt62(&value)) {
+ if (!reader.ReadStringPieceVarInt62(&value)) {
RaiseError(QUIC_HTTP_FRAME_ERROR, "Unable to read ACCEPT_CH value.");
return false;
}
// Copy data.
- frame->entries.push_back({std::string(origin.data(), origin.size()),
- std::string(value.data(), value.size())});
+ frame.entries.push_back({std::string(origin.data(), origin.size()),
+ std::string(value.data(), value.size())});
}
return true;
}
diff --git a/quiche/quic/core/http/http_decoder.h b/quiche/quic/core/http/http_decoder.h
index 6d305e4..e959807 100644
--- a/quiche/quic/core/http/http_decoder.h
+++ b/quiche/quic/core/http/http_decoder.h
@@ -7,6 +7,7 @@
#include <cstdint>
+#include "absl/base/nullability.h"
#include "absl/strings/string_view.h"
#include "quiche/quic/core/http/http_frames.h"
#include "quiche/quic/core/quic_error_codes.h"
@@ -127,7 +128,7 @@
};
// |visitor| must be non-null, and must outlive HttpDecoder.
- explicit HttpDecoder(Visitor* visitor);
+ explicit HttpDecoder(absl::Nonnull<Visitor*> visitor);
~HttpDecoder();
@@ -182,11 +183,11 @@
// if there are any errors. Also calls OnDataFrameStart() or
// OnHeadersFrameStart() for appropriate frame types. Returns whether the
// processing should continue.
- bool ReadFrameType(QuicDataReader* reader);
+ bool ReadFrameType(QuicDataReader& reader);
// Reads the length of a frame from |reader|. Sets error_ and error_detail_
// if there are any errors. Returns whether processing should continue.
- bool ReadFrameLength(QuicDataReader* reader);
+ bool ReadFrameLength(QuicDataReader& reader);
// Returns whether the current frame is of a buffered type.
// The payload of buffered frames is buffered by HttpDecoder, and parsed by
@@ -199,7 +200,7 @@
// For buffered frame types, calls BufferOrParsePayload(). For other frame
// types, reads the payload of the current frame from |reader| and calls
// visitor methods. Returns whether processing should continue.
- bool ReadFramePayload(QuicDataReader* reader);
+ bool ReadFramePayload(QuicDataReader& reader);
// For buffered frame types, this method is only called if frame payload is
// empty, and it calls BufferOrParsePayload(). For other frame types, this
@@ -213,41 +214,41 @@
// Read payload of unknown frame from |reader| and call
// Visitor::OnUnknownFramePayload(). Returns true decoding should continue,
// false if it should be paused.
- bool HandleUnknownFramePayload(QuicDataReader* reader);
+ bool HandleUnknownFramePayload(QuicDataReader& reader);
// Buffers any remaining frame payload from |*reader| into |buffer_| if
// necessary. Parses the frame payload if complete. Parses out of |*reader|
// without unnecessary copy if |*reader| contains entire payload.
// Returns whether processing should continue.
// Must only be called when current frame type is buffered.
- bool BufferOrParsePayload(QuicDataReader* reader);
+ bool BufferOrParsePayload(QuicDataReader& reader);
// Parses the entire payload of certain kinds of frames that are parsed in a
// single pass. |reader| must have at least |current_frame_length_| bytes.
// Returns whether processing should continue.
// Must only be called when current frame type is buffered.
- bool ParseEntirePayload(QuicDataReader* reader);
+ bool ParseEntirePayload(QuicDataReader& reader);
// Buffers any remaining frame length field from |reader| into
// |length_buffer_|.
- void BufferFrameLength(QuicDataReader* reader);
+ void BufferFrameLength(QuicDataReader& reader);
// Buffers any remaining frame type field from |reader| into |type_buffer_|.
- void BufferFrameType(QuicDataReader* reader);
+ void BufferFrameType(QuicDataReader& reader);
// Sets |error_| and |error_detail_| accordingly.
void RaiseError(QuicErrorCode error, std::string error_detail);
// Parses the payload of a SETTINGS frame from |reader| into |frame|.
- bool ParseSettingsFrame(QuicDataReader* reader, SettingsFrame* frame);
+ bool ParseSettingsFrame(QuicDataReader& reader, SettingsFrame& frame);
// Parses the payload of a PRIORITY_UPDATE frame (draft-02, type 0xf0700)
// from |reader| into |frame|.
- bool ParsePriorityUpdateFrame(QuicDataReader* reader,
- PriorityUpdateFrame* frame);
+ bool ParsePriorityUpdateFrame(QuicDataReader& reader,
+ PriorityUpdateFrame& frame);
// Parses the payload of an ACCEPT_CH frame from |reader| into |frame|.
- bool ParseAcceptChFrame(QuicDataReader* reader, AcceptChFrame* frame);
+ bool ParseAcceptChFrame(QuicDataReader& reader, AcceptChFrame& frame);
// Returns the max frame size of a given |frame_type|.
QuicByteCount MaxFrameLength(uint64_t frame_type);
