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);