Resize HttpDecoder::buffer_ when buffering data.
Before this change, HttpDecoder::buffer_ was always empty. A
std::string::reserve() call made sure its storage was large enough, and that
memory was address directly, always beyond the size of the string object.
After this CL, the size of buffer_ reflects the amount of buffered bytes, and
memory beyond its size is never addressed. The reserve() call is preserved to
avoid reallocation and copy as the buffer grows.
PiperOrigin-RevId: 365076891
Change-Id: If58949fd10fe1741416dab4691faa5c3e728ad8e
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index 5377f7c..02b563d 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -282,10 +282,14 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::CANCEL_PUSH): {
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
BufferFramePayload(reader);
break;
}
case static_cast<uint64_t>(HttpFrameType::SETTINGS): {
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
BufferFramePayload(reader);
break;
}
@@ -355,10 +359,14 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::GOAWAY): {
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
BufferFramePayload(reader);
break;
}
case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): {
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
BufferFramePayload(reader);
break;
}
@@ -408,8 +416,11 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::CANCEL_PUSH): {
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
CancelPushFrame frame;
- QuicDataReader reader(buffer_.data(), current_frame_length_);
+ QUICHE_DCHECK_EQ(current_frame_length_, buffer_.size());
+ QuicDataReader reader(buffer_);
if (!reader.ReadVarInt62(&frame.push_id)) {
RaiseError(QUIC_HTTP_FRAME_ERROR,
"Unable to read CANCEL_PUSH push_id.");
@@ -424,8 +435,11 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::SETTINGS): {
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
SettingsFrame frame;
- QuicDataReader reader(buffer_.data(), current_frame_length_);
+ QUICHE_DCHECK_EQ(current_frame_length_, buffer_.size());
+ QuicDataReader reader(buffer_);
if (!ParseSettingsFrame(&reader, &frame)) {
return false;
}
@@ -437,7 +451,10 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::GOAWAY): {
- QuicDataReader reader(buffer_.data(), current_frame_length_);
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
+ QUICHE_DCHECK_EQ(current_frame_length_, buffer_.size());
+ QuicDataReader reader(buffer_);
GoAwayFrame frame;
if (!reader.ReadVarInt62(&frame.id)) {
RaiseError(QUIC_HTTP_FRAME_ERROR, "Unable to read GOAWAY ID.");
@@ -451,7 +468,10 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): {
- QuicDataReader reader(buffer_.data(), current_frame_length_);
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
+ QUICHE_DCHECK_EQ(current_frame_length_, buffer_.size());
+ QuicDataReader reader(buffer_);
MaxPushIdFrame frame;
if (!reader.ReadVarInt62(&frame.push_id)) {
RaiseError(QUIC_HTTP_FRAME_ERROR,
@@ -470,7 +490,8 @@
// TODO(bnc): Avoid buffering if the entire frame is present, and
// instead parse directly out of |reader|.
PriorityUpdateFrame frame;
- QuicDataReader reader(buffer_.data(), current_frame_length_);
+ QUICHE_DCHECK_EQ(current_frame_length_, buffer_.size());
+ QuicDataReader reader(buffer_);
if (!ParsePriorityUpdateFrame(&reader, &frame)) {
return false;
}
@@ -481,7 +502,8 @@
// TODO(bnc): Avoid buffering if the entire frame is present, and
// instead parse directly out of |reader|.
PriorityUpdateFrame frame;
- QuicDataReader reader(buffer_.data(), current_frame_length_);
+ QUICHE_DCHECK_EQ(current_frame_length_, buffer_.size());
+ QuicDataReader reader(buffer_);
if (!ParseNewPriorityUpdateFrame(&reader, &frame)) {
return false;
}
@@ -492,7 +514,8 @@
// TODO(bnc): Avoid buffering if the entire frame is present, and
// instead parse directly out of |reader|.
AcceptChFrame frame;
- QuicDataReader reader(buffer_.data(), current_frame_length_);
+ QUICHE_DCHECK_EQ(current_frame_length_, buffer_.size());
+ QuicDataReader reader(buffer_);
if (!ParseAcceptChFrame(&reader, &frame)) {
return false;
}
@@ -536,16 +559,21 @@
void HttpDecoder::BufferFramePayload(QuicDataReader* reader) {
if (current_frame_length_ == remaining_frame_length_) {
- buffer_.erase(buffer_.size());
+ buffer_.clear();
buffer_.reserve(current_frame_length_);
}
+
+ QUICHE_DCHECK_EQ(current_frame_length_ - remaining_frame_length_,
+ buffer_.size());
QuicByteCount bytes_to_read = std::min<QuicByteCount>(
remaining_frame_length_, reader->BytesRemaining());
- bool success = reader->ReadBytes(
- &(buffer_[0]) + current_frame_length_ - remaining_frame_length_,
- bytes_to_read);
- QUICHE_DCHECK(success);
+ absl::StrAppend(&buffer_, reader->PeekRemainingPayload().substr(
+ /* pos = */ 0, bytes_to_read));
+ reader->Seek(bytes_to_read);
+
remaining_frame_length_ -= bytes_to_read;
+ QUICHE_DCHECK_EQ(current_frame_length_ - remaining_frame_length_,
+ buffer_.size());
}
void HttpDecoder::BufferFrameLength(QuicDataReader* reader) {
