Rewrite QuicSpdyStreamBodyBuffer's consumed byte tracking algorithm. This is motivated by having to consume unknown frames (in a later CL), see cr/258682203 for the end goal. There is some behavioral change as a side effect of the new algorithm: DATA frame headers are consumed immediately, not only when some of the corresponding payload is consumed. This hinges on the previously undocumented property of HttpDecoder that in only calls Visitor::On*FrameStart() methods after the entire frame header is processed. This behavioral change necessitates minor updates to tests. I plan four more CLs after this: one to move HEADERS frame header and payload consumption calculations from QuicSpdyStream to QuicSpdyStreamBodyBuffer; one to consume unknown frames; one to rename QuicSpdyStreamBodyBuffer to QuicSpdyStreamBodyManager or something similar; and one to remove Http3FrameLengths if by that point it's entirely unused. gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99. PiperOrigin-RevId: 259987101 Change-Id: Id8a734fb36466b3502373097faba2c6c81c793de
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h index 99c4d10..80ba385 100644 --- a/quic/core/http/http_decoder.h +++ b/quic/core/http/http_decoder.h
@@ -49,6 +49,9 @@ // All the following methods return true to continue decoding, // and false to pause it. + // On*FrameStart() methods are called after the frame header is completely + // processed. At that point it is safe to consume + // |frame_length.header_length| bytes. // Called when a PRIORITY frame has been received. // |frame_length| contains PRIORITY frame length and payload length.
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc index 97991e8..aee3ba5 100644 --- a/quic/core/http/quic_spdy_stream.cc +++ b/quic/core/http/quic_spdy_stream.cc
@@ -769,7 +769,9 @@ return false; } - body_buffer_.OnDataHeader(frame_lengths); + sequencer()->MarkConsumed( + body_buffer_.OnDataHeader(frame_lengths.header_length)); + return true; } @@ -777,6 +779,7 @@ DCHECK(VersionHasDataFrameHeader(transport_version())); body_buffer_.OnDataPayload(payload); + return true; }
diff --git a/quic/core/http/quic_spdy_stream_body_buffer.cc b/quic/core/http/quic_spdy_stream_body_buffer.cc index c29c766..2ae72b5 100644 --- a/quic/core/http/quic_spdy_stream_body_buffer.cc +++ b/quic/core/http/quic_spdy_stream_body_buffer.cc
@@ -11,89 +11,78 @@ namespace quic { QuicSpdyStreamBodyBuffer::QuicSpdyStreamBodyBuffer() - : bytes_remaining_(0), - total_body_bytes_readable_(0), - total_body_bytes_received_(0), - total_payload_lengths_(0) {} + : total_body_bytes_received_(0) {} -void QuicSpdyStreamBodyBuffer::OnDataHeader(Http3FrameLengths frame_lengths) { - frame_meta_.push_back(frame_lengths); - total_payload_lengths_ += frame_lengths.payload_length; +size_t QuicSpdyStreamBodyBuffer::OnDataHeader(QuicByteCount length) { + DCHECK_NE(0u, length); + + if (fragments_.empty()) { + // DATA frame header can be consumed immediately, because all previously + // received DATA frame payload bytes have been read. + return length; + } + + // DATA frame header length will be consumed after last fragment is read. + fragments_.back().trailing_consumable_bytes += length; + return 0; } void QuicSpdyStreamBodyBuffer::OnDataPayload(QuicStringPiece payload) { DCHECK(!payload.empty()); - bodies_.push_back(payload); + + fragments_.push_back({payload, 0}); total_body_bytes_received_ += payload.length(); - total_body_bytes_readable_ += payload.length(); - DCHECK_LE(total_body_bytes_received_, total_payload_lengths_); } size_t QuicSpdyStreamBodyBuffer::OnBodyConsumed(size_t num_bytes) { - // Check if the stream has enough decoded data. - if (num_bytes > total_body_bytes_readable_) { - QUIC_BUG << "Invalid argument to OnBodyConsumed." - << " expect to consume: " << num_bytes - << ", but not enough bytes available. " - << "Total bytes readable are: " << total_body_bytes_readable_; - return 0; - } - // Discard references in the stream before the sequencer marks them consumed. - size_t remaining = num_bytes; - while (remaining > 0) { - if (bodies_.empty()) { - QUIC_BUG << "Failed to consume because body buffer is empty."; + QuicByteCount bytes_to_consume = 0; + size_t remaining_bytes = num_bytes; + + while (remaining_bytes > 0) { + if (fragments_.empty()) { + QUIC_BUG << "Not enough available body to consume."; return 0; } - auto body = bodies_.front(); - bodies_.pop_front(); - if (body.length() <= remaining) { - remaining -= body.length(); - } else { - body = body.substr(remaining, body.length() - remaining); - bodies_.push_front(body); - remaining = 0; - } - } - // Consume headers. - size_t bytes_to_consume = 0; - while (bytes_remaining_ < num_bytes) { - if (frame_meta_.empty()) { - QUIC_BUG << "Faild to consume because frame header buffer is empty."; - return 0; - } - auto meta = frame_meta_.front(); - frame_meta_.pop_front(); - bytes_remaining_ += meta.payload_length; - bytes_to_consume += meta.header_length; - } - bytes_to_consume += num_bytes; + Fragment& fragment = fragments_.front(); + const QuicStringPiece body = fragment.body; - // Update accountings. - bytes_remaining_ -= num_bytes; - total_body_bytes_readable_ -= num_bytes; + if (body.length() > remaining_bytes) { + // Consume leading |remaining_bytes| bytes of body. + bytes_to_consume += remaining_bytes; + fragment.body = body.substr(remaining_bytes); + return bytes_to_consume; + } + + // Consume entire fragment and the following + // |trailing_consumable_bytes| bytes. + remaining_bytes -= body.length(); + bytes_to_consume += body.length() + fragment.trailing_consumable_bytes; + fragments_.pop_front(); + } return bytes_to_consume; } int QuicSpdyStreamBodyBuffer::PeekBody(iovec* iov, size_t iov_len) const { - DCHECK(iov != nullptr); + DCHECK(iov); DCHECK_GT(iov_len, 0u); - if (bodies_.empty()) { + // TODO(bnc): Is this really necessary? + if (fragments_.empty()) { iov[0].iov_base = nullptr; iov[0].iov_len = 0; return 0; } - // Fill iovs with references from the stream. + size_t iov_filled = 0; - while (iov_filled < bodies_.size() && iov_filled < iov_len) { - QuicStringPiece body = bodies_[iov_filled]; + while (iov_filled < fragments_.size() && iov_filled < iov_len) { + QuicStringPiece body = fragments_[iov_filled].body; iov[iov_filled].iov_base = const_cast<char*>(body.data()); iov[iov_filled].iov_len = body.size(); iov_filled++; } + return iov_filled; } @@ -101,32 +90,50 @@ size_t iov_len, size_t* total_bytes_read) { *total_bytes_read = 0; - QuicByteCount total_remaining = total_body_bytes_readable_; + QuicByteCount bytes_to_consume = 0; + + // The index of iovec to write to. size_t index = 0; - size_t src_offset = 0; - for (size_t i = 0; i < iov_len && total_remaining > 0; ++i) { - char* dest = reinterpret_cast<char*>(iov[i].iov_base); - size_t dest_remaining = iov[i].iov_len; - while (dest_remaining > 0 && total_remaining > 0) { - auto body = bodies_[index]; - size_t bytes_to_copy = - std::min<size_t>(body.length() - src_offset, dest_remaining); - memcpy(dest, body.substr(src_offset, bytes_to_copy).data(), - bytes_to_copy); + // Address to write to within current iovec. + char* dest = reinterpret_cast<char*>(iov[index].iov_base); + // Remaining space in current iovec. + size_t dest_remaining = iov[index].iov_len; + + while (!fragments_.empty()) { + Fragment& fragment = fragments_.front(); + const QuicStringPiece body = fragment.body; + + const size_t bytes_to_copy = + std::min<size_t>(body.length(), dest_remaining); + memcpy(dest, body.data(), bytes_to_copy); + bytes_to_consume += bytes_to_copy; + *total_bytes_read += bytes_to_copy; + + if (bytes_to_copy == body.length()) { + // Entire fragment read. + bytes_to_consume += fragment.trailing_consumable_bytes; + fragments_.pop_front(); + } else { + // Consume leading |bytes_to_copy| bytes of body. + fragment.body = body.substr(bytes_to_copy); + } + + if (bytes_to_copy == dest_remaining) { + // Current iovec full. + ++index; + if (index == iov_len) { + break; + } + dest = reinterpret_cast<char*>(iov[index].iov_base); + dest_remaining = iov[index].iov_len; + } else { + // Advance destination parameters within this iovec. dest += bytes_to_copy; dest_remaining -= bytes_to_copy; - *total_bytes_read += bytes_to_copy; - total_remaining -= bytes_to_copy; - if (bytes_to_copy < body.length() - src_offset) { - src_offset += bytes_to_copy; - } else { - index++; - src_offset = 0; - } } } - return OnBodyConsumed(*total_bytes_read); + return bytes_to_consume; } } // namespace quic
diff --git a/quic/core/http/quic_spdy_stream_body_buffer.h b/quic/core/http/quic_spdy_stream_body_buffer.h index a37f3bf..77c17b8 100644 --- a/quic/core/http/quic_spdy_stream_body_buffer.h +++ b/quic/core/http/quic_spdy_stream_body_buffer.h
@@ -5,72 +5,81 @@ #ifndef QUICHE_QUIC_CORE_HTTP_QUIC_SPDY_STREAM_BODY_BUFFER_H_ #define QUICHE_QUIC_CORE_HTTP_QUIC_SPDY_STREAM_BODY_BUFFER_H_ -#include "net/third_party/quiche/src/quic/core/http/http_decoder.h" +#include "net/third_party/quiche/src/quic/core/quic_constants.h" #include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" #include "net/third_party/quiche/src/quic/platform/api/quic_containers.h" #include "net/third_party/quiche/src/quic/platform/api/quic_export.h" #include "net/third_party/quiche/src/quic/platform/api/quic_iovec.h" #include "net/third_party/quiche/src/quic/platform/api/quic_macros.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h" namespace quic { -class QuicStreamSequencer; - -// Buffer decoded body for QuicSpdyStream. It also talks to the sequencer to -// consume data. +// "Body" means DATA frame payload. +// QuicSpdyStreamBodyBuffer does two things: it keeps references to body +// fragments (owned by QuicStreamSequencer) and offers methods to read them; and +// calculates the total number of bytes (including DATA frame headers) the +// caller needs to mark consumed (with QuicStreamSequencer) whenever DATA frame +// headers are received or body bytes are read. +// TODO(bnc): Rename to QuicSpdyStreamBodyManager or similar. class QUIC_EXPORT_PRIVATE QuicSpdyStreamBodyBuffer { public: QuicSpdyStreamBodyBuffer(); ~QuicSpdyStreamBodyBuffer() = default; - // Add metadata of the frame to accountings. - // Called when QuicSpdyStream receives data frame header. - void OnDataHeader(Http3FrameLengths frame_lengths); + // Called when DATA frame header bytes are received. |length| must be + // positive. Returns number of bytes the caller shall mark consumed, which + // might be zero. + QUIC_MUST_USE_RESULT size_t OnDataHeader(QuicByteCount length); - // Add new data payload to buffer. - // Called when QuicSpdyStream received data payload. - // Data pointed by payload must be alive until consumed by - // QuicStreamSequencer::MarkConsumed(). + // Called when DATA frame payload is received. |payload| is added to + // |fragments_|. The data pointed to by |payload| must be kept alive until an + // OnBodyConsumed() or ReadBody() call consumes it. |payload| must not be + // empty. void OnDataPayload(QuicStringPiece payload); - // Internally marks |num_bytes| of DATA frame payload consumed, and returns - // the number of bytes that the caller should mark consumed with the - // sequencer, including DATA frame header bytes, if any. + // Internally marks |num_bytes| of DATA frame payload consumed. |num_bytes| + // might be zero. Returns the number of bytes that the caller should mark + // consumed with the sequencer, which is the sum of |num_bytes| for payload + // and additional DATA frame header bytes, if any. QUIC_MUST_USE_RESULT size_t OnBodyConsumed(size_t num_bytes); - // Fill up to |iov_len| with bodies available in buffer. No data is consumed. - // |iov|.iov_base will point to data in the buffer, and |iov|.iov_len will - // be set to the underlying data length accordingly. - // Returns the number of iov used. + // Set up to |iov_len| elements of iov[] to point to available bodies: each + // iov[i].iov_base will point to a body fragment, and iov[i].iov_len will be + // set to its length. No data is copied, no data is consumed. Returns the + // number of iov set. int PeekBody(iovec* iov, size_t iov_len) const; - // Copies from buffer into |iov| up to |iov_len|, and consume data in - // sequencer. |iov.iov_base| and |iov.iov_len| are preassigned and will not be - // changed. |*total_bytes_read| is set to the number of bytes read. Returns - // the number of bytes that should be marked consumed with the sequencer. + // Copies data from available bodies into at most |iov_len| elements of iov[]. + // Internally consumes copied payload bytes as well as all interleaving and + // immediately following DATA frame header bytes. |iov.iov_base| and + // |iov.iov_len| are preassigned and will not be changed. Returns the total + // number of bytes the caller shall mark consumed. Sets + // |*total_bytes_read| to the total number of DATA payload bytes read. QUIC_MUST_USE_RESULT size_t ReadBody(const struct iovec* iov, size_t iov_len, size_t* total_bytes_read); - bool HasBytesToRead() const { return !bodies_.empty(); } + bool HasBytesToRead() const { return !fragments_.empty(); } uint64_t total_body_bytes_received() const { return total_body_bytes_received_; } private: - // Storage for decoded data. - QuicDeque<QuicStringPiece> bodies_; - // Storage for header lengths. - QuicDeque<Http3FrameLengths> frame_meta_; - // Bytes in the first available data frame that are not consumed yet. - QuicByteCount bytes_remaining_; - // Total available body data in the stream. - QuicByteCount total_body_bytes_readable_; - // Total bytes read from the stream excluding headers. + // A Fragment instance represents a body fragment with a count of bytes + // received afterwards but before the next body fragment that can be marked + // consumed as soon as all of the body fragment is read. + struct Fragment { + // |body| must not be empty. + QuicStringPiece body; + // Might be zero. + QuicByteCount trailing_consumable_bytes; + }; + // Queue of DATA frame payload fragments and byte counts. + QuicDeque<Fragment> fragments_; + // Total body bytes received. QuicByteCount total_body_bytes_received_; - // Total length of payloads tracked by frame_meta_. - QuicByteCount total_payload_lengths_; }; } // namespace quic
diff --git a/quic/core/http/quic_spdy_stream_body_buffer_test.cc b/quic/core/http/quic_spdy_stream_body_buffer_test.cc index d5e8161..7c4787d 100644 --- a/quic/core/http/quic_spdy_stream_body_buffer_test.cc +++ b/quic/core/http/quic_spdy_stream_body_buffer_test.cc
@@ -4,8 +4,11 @@ #include "net/third_party/quiche/src/quic/core/http/quic_spdy_stream_body_buffer.h" +#include <algorithm> +#include <numeric> #include <string> +#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h" #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" @@ -21,139 +24,255 @@ QuicSpdyStreamBodyBuffer body_buffer_; }; -TEST_F(QuicSpdyStreamBodyBufferTest, ReceiveBodies) { - std::string body(1024, 'a'); +TEST_F(QuicSpdyStreamBodyBufferTest, HasBytesToRead) { EXPECT_FALSE(body_buffer_.HasBytesToRead()); - body_buffer_.OnDataHeader(Http3FrameLengths(3, 1024)); - body_buffer_.OnDataPayload(body); - EXPECT_EQ(1024u, body_buffer_.total_body_bytes_received()); - EXPECT_TRUE(body_buffer_.HasBytesToRead()); -} + EXPECT_EQ(0u, body_buffer_.total_body_bytes_received()); -TEST_F(QuicSpdyStreamBodyBufferTest, PeekBody) { - std::string body(1024, 'a'); - body_buffer_.OnDataHeader(Http3FrameLengths(3, 1024)); - body_buffer_.OnDataPayload(body); - EXPECT_EQ(1024u, body_buffer_.total_body_bytes_received()); - iovec vec; - EXPECT_EQ(1, body_buffer_.PeekBody(&vec, 1)); - EXPECT_EQ(1024u, vec.iov_len); - EXPECT_EQ(body, - QuicStringPiece(static_cast<const char*>(vec.iov_base), 1024)); -} - -// Buffer only receives 1 frame. Stream consumes less or equal than a frame. -TEST_F(QuicSpdyStreamBodyBufferTest, MarkConsumedPartialSingleFrame) { - testing::InSequence seq; - std::string body(1024, 'a'); const QuicByteCount header_length = 3; - Http3FrameLengths lengths(header_length, 1024); - body_buffer_.OnDataHeader(lengths); - body_buffer_.OnDataPayload(body); - EXPECT_EQ(header_length + 1024, body_buffer_.OnBodyConsumed(1024)); -} + EXPECT_EQ(header_length, body_buffer_.OnDataHeader(header_length)); -// Buffer received 2 frames. Stream consumes multiple times. -TEST_F(QuicSpdyStreamBodyBufferTest, MarkConsumedMultipleFrames) { - testing::InSequence seq; - // 1st frame. - std::string body1(1024, 'a'); - const QuicByteCount header_length1 = 2; - Http3FrameLengths lengths1(header_length1, 1024); - body_buffer_.OnDataHeader(lengths1); - body_buffer_.OnDataPayload(body1); + EXPECT_FALSE(body_buffer_.HasBytesToRead()); + EXPECT_EQ(0u, body_buffer_.total_body_bytes_received()); - // 2nd frame. - std::string body2(2048, 'b'); - const QuicByteCount header_length2 = 4; - Http3FrameLengths lengths2(header_length2, 2048); - body_buffer_.OnDataHeader(lengths2); - body_buffer_.OnDataPayload(body2); - - EXPECT_EQ(header_length1 + 512, body_buffer_.OnBodyConsumed(512)); - EXPECT_EQ(header_length2 + 2048, body_buffer_.OnBodyConsumed(2048)); - EXPECT_EQ(512u, body_buffer_.OnBodyConsumed(512)); -} - -TEST_F(QuicSpdyStreamBodyBufferTest, MarkConsumedMoreThanBuffered) { std::string body(1024, 'a'); - Http3FrameLengths lengths(3, 1024); - body_buffer_.OnDataHeader(lengths); + body_buffer_.OnDataPayload(body); + + EXPECT_TRUE(body_buffer_.HasBytesToRead()); + EXPECT_EQ(1024u, body_buffer_.total_body_bytes_received()); +} + +TEST_F(QuicSpdyStreamBodyBufferTest, ConsumeMoreThanAvailable) { + std::string body(1024, 'a'); body_buffer_.OnDataPayload(body); size_t bytes_to_consume = 0; - EXPECT_QUIC_BUG( - bytes_to_consume = body_buffer_.OnBodyConsumed(2048), - "Invalid argument to OnBodyConsumed. expect to consume: 2048, but not " - "enough bytes available. Total bytes readable are: 1024"); + EXPECT_QUIC_BUG(bytes_to_consume = body_buffer_.OnBodyConsumed(2048), + "Not enough available body to consume."); EXPECT_EQ(0u, bytes_to_consume); } -// Buffer receives 1 frame. Stream read from the buffer. -TEST_F(QuicSpdyStreamBodyBufferTest, ReadSingleBody) { - testing::InSequence seq; - std::string body(1024, 'a'); - const QuicByteCount header_length = 2; - Http3FrameLengths lengths(header_length, 1024); - body_buffer_.OnDataHeader(lengths); - body_buffer_.OnDataPayload(body); +struct { + std::vector<QuicByteCount> frame_header_lengths; + std::vector<const char*> frame_payloads; + std::vector<QuicByteCount> body_bytes_to_read; + std::vector<QuicByteCount> expected_return_values; +} const kOnBodyConsumedTestData[] = { + // One frame consumed in one call. + {{2}, {"foobar"}, {6}, {6}}, + // Two frames consumed in one call. + {{3, 5}, {"foobar", "baz"}, {9}, {14}}, + // One frame consumed in two calls. + {{2}, {"foobar"}, {4, 2}, {4, 2}}, + // Two frames consumed in two calls matching frame boundaries. + {{3, 5}, {"foobar", "baz"}, {6, 3}, {11, 3}}, + // Two frames consumed in two calls, + // the first call only consuming part of the first frame. + {{3, 5}, {"foobar", "baz"}, {5, 4}, {5, 9}}, + // Two frames consumed in two calls, + // the first call consuming the entire first frame and part of the second. + {{3, 5}, {"foobar", "baz"}, {7, 2}, {12, 2}}, +}; - char base[1024]; - iovec iov = {&base[0], 1024}; - size_t total_bytes_read = 0; - EXPECT_EQ(header_length + 1024, - body_buffer_.ReadBody(&iov, 1, &total_bytes_read)); - EXPECT_EQ(1024u, total_bytes_read); - EXPECT_EQ(1024u, iov.iov_len); - EXPECT_EQ(body, - QuicStringPiece(static_cast<const char*>(iov.iov_base), 1024)); +TEST_F(QuicSpdyStreamBodyBufferTest, OnBodyConsumed) { + for (size_t test_case_index = 0; + test_case_index < QUIC_ARRAYSIZE(kOnBodyConsumedTestData); + ++test_case_index) { + const std::vector<QuicByteCount>& frame_header_lengths = + kOnBodyConsumedTestData[test_case_index].frame_header_lengths; + const std::vector<const char*>& frame_payloads = + kOnBodyConsumedTestData[test_case_index].frame_payloads; + const std::vector<QuicByteCount>& body_bytes_to_read = + kOnBodyConsumedTestData[test_case_index].body_bytes_to_read; + const std::vector<QuicByteCount>& expected_return_values = + kOnBodyConsumedTestData[test_case_index].expected_return_values; + + for (size_t frame_index = 0; frame_index < frame_header_lengths.size(); + ++frame_index) { + // Frame header of first frame can immediately be consumed, but not the + // other frames. Each test case start with an empty + // QuicSpdyStreamBodyBuffer. + EXPECT_EQ(frame_index == 0 ? frame_header_lengths[frame_index] : 0u, + body_buffer_.OnDataHeader(frame_header_lengths[frame_index])); + body_buffer_.OnDataPayload(frame_payloads[frame_index]); + } + + for (size_t call_index = 0; call_index < body_bytes_to_read.size(); + ++call_index) { + EXPECT_EQ(expected_return_values[call_index], + body_buffer_.OnBodyConsumed(body_bytes_to_read[call_index])); + } + + EXPECT_FALSE(body_buffer_.HasBytesToRead()); + } } -// Buffer receives 2 frames, stream read from the buffer multiple times. -TEST_F(QuicSpdyStreamBodyBufferTest, ReadMultipleBody) { - testing::InSequence seq; - // 1st frame. - std::string body1(1024, 'a'); - const QuicByteCount header_length1 = 2; - Http3FrameLengths lengths1(header_length1, 1024); - body_buffer_.OnDataHeader(lengths1); - body_buffer_.OnDataPayload(body1); +struct { + std::vector<QuicByteCount> frame_header_lengths; + std::vector<const char*> frame_payloads; + size_t iov_len; +} const kPeekBodyTestData[] = { + // No frames, more iovecs than frames. + {{}, {}, 1}, + // One frame, same number of iovecs. + {{3}, {"foobar"}, 1}, + // One frame, more iovecs than frames. + {{3}, {"foobar"}, 2}, + // Two frames, fewer iovecs than frames. + {{3, 5}, {"foobar", "baz"}, 1}, + // Two frames, same number of iovecs. + {{3, 5}, {"foobar", "baz"}, 2}, + // Two frames, more iovecs than frames. + {{3, 5}, {"foobar", "baz"}, 3}, +}; - // 2nd frame. - std::string body2(2048, 'b'); - const QuicByteCount header_length2 = 4; - Http3FrameLengths lengths2(header_length2, 2048); - body_buffer_.OnDataHeader(lengths2); - body_buffer_.OnDataPayload(body2); +TEST_F(QuicSpdyStreamBodyBufferTest, PeekBody) { + for (size_t test_case_index = 0; + test_case_index < QUIC_ARRAYSIZE(kPeekBodyTestData); ++test_case_index) { + const std::vector<QuicByteCount>& frame_header_lengths = + kPeekBodyTestData[test_case_index].frame_header_lengths; + const std::vector<const char*>& frame_payloads = + kPeekBodyTestData[test_case_index].frame_payloads; + size_t iov_len = kPeekBodyTestData[test_case_index].iov_len; - // First read of 512 bytes. - char base[512]; - iovec iov = {&base[0], 512}; - size_t total_bytes_read = 0; - EXPECT_EQ(header_length1 + 512, - body_buffer_.ReadBody(&iov, 1, &total_bytes_read)); - EXPECT_EQ(512u, total_bytes_read); - EXPECT_EQ(512u, iov.iov_len); - EXPECT_EQ(body1.substr(0, 512), - QuicStringPiece(static_cast<const char*>(iov.iov_base), 512)); + QuicSpdyStreamBodyBuffer body_buffer; - // Second read of 2048 bytes. - char base2[2048]; - iovec iov2 = {&base2[0], 2048}; - EXPECT_EQ(header_length2 + 2048, - body_buffer_.ReadBody(&iov2, 1, &total_bytes_read)); - EXPECT_EQ(2048u, total_bytes_read); - EXPECT_EQ(2048u, iov2.iov_len); - EXPECT_EQ(body1.substr(512, 512) + body2.substr(0, 1536), - QuicStringPiece(static_cast<const char*>(iov2.iov_base), 2048)); + for (size_t frame_index = 0; frame_index < frame_header_lengths.size(); + ++frame_index) { + // Frame header of first frame can immediately be consumed, but not the + // other frames. Each test case uses a new QuicSpdyStreamBodyBuffer + // instance. + EXPECT_EQ(frame_index == 0 ? frame_header_lengths[frame_index] : 0u, + body_buffer.OnDataHeader(frame_header_lengths[frame_index])); + body_buffer.OnDataPayload(frame_payloads[frame_index]); + } - // Third read of the rest 512 bytes. - char base3[512]; - iovec iov3 = {&base3[0], 512}; - EXPECT_EQ(512u, body_buffer_.ReadBody(&iov3, 1, &total_bytes_read)); - EXPECT_EQ(512u, total_bytes_read); - EXPECT_EQ(512u, iov3.iov_len); - EXPECT_EQ(body2.substr(1536, 512), - QuicStringPiece(static_cast<const char*>(iov3.iov_base), 512)); + std::vector<iovec> iovecs; + iovecs.resize(iov_len); + size_t iovs_filled = std::min(frame_payloads.size(), iov_len); + ASSERT_EQ(iovs_filled, + static_cast<size_t>(body_buffer.PeekBody(&iovecs[0], iov_len))); + for (size_t iovec_index = 0; iovec_index < iovs_filled; ++iovec_index) { + EXPECT_EQ(frame_payloads[iovec_index], + QuicStringPiece( + static_cast<const char*>(iovecs[iovec_index].iov_base), + iovecs[iovec_index].iov_len)); + } + } +} + +struct { + std::vector<QuicByteCount> frame_header_lengths; + std::vector<const char*> frame_payloads; + std::vector<std::vector<QuicByteCount>> iov_lengths; + std::vector<QuicByteCount> expected_total_bytes_read; + std::vector<QuicByteCount> expected_return_values; +} const kReadBodyTestData[] = { + // One frame, one read with smaller iovec. + {{4}, {"foo"}, {{2}}, {2}, {2}}, + // One frame, one read with same size iovec. + {{4}, {"foo"}, {{3}}, {3}, {3}}, + // One frame, one read with larger iovec. + {{4}, {"foo"}, {{5}}, {3}, {3}}, + // One frame, one read with two iovecs, smaller total size. + {{4}, {"foobar"}, {{2, 3}}, {5}, {5}}, + // One frame, one read with two iovecs, same total size. + {{4}, {"foobar"}, {{2, 4}}, {6}, {6}}, + // One frame, one read with two iovecs, larger total size in last iovec. + {{4}, {"foobar"}, {{2, 6}}, {6}, {6}}, + // One frame, one read with extra iovecs, body ends at iovec boundary. + {{4}, {"foobar"}, {{2, 4, 4, 3}}, {6}, {6}}, + // One frame, one read with extra iovecs, body ends not at iovec boundary. + {{4}, {"foobar"}, {{2, 7, 4, 3}}, {6}, {6}}, + // One frame, two reads with two iovecs each, smaller total size. + {{4}, {"foobarbaz"}, {{2, 1}, {3, 2}}, {3, 5}, {3, 5}}, + // One frame, two reads with two iovecs each, same total size. + {{4}, {"foobarbaz"}, {{2, 1}, {4, 2}}, {3, 6}, {3, 6}}, + // One frame, two reads with two iovecs each, larger total size. + {{4}, {"foobarbaz"}, {{2, 1}, {4, 10}}, {3, 6}, {3, 6}}, + // Two frames, one read with smaller iovec. + {{4, 3}, {"foobar", "baz"}, {{8}}, {8}, {11}}, + // Two frames, one read with same size iovec. + {{4, 3}, {"foobar", "baz"}, {{9}}, {9}, {12}}, + // Two frames, one read with larger iovec. + {{4, 3}, {"foobar", "baz"}, {{10}}, {9}, {12}}, + // Two frames, one read with two iovecs, smaller total size. + {{4, 3}, {"foobar", "baz"}, {{4, 3}}, {7}, {10}}, + // Two frames, one read with two iovecs, same total size. + {{4, 3}, {"foobar", "baz"}, {{4, 5}}, {9}, {12}}, + // Two frames, one read with two iovecs, larger total size in last iovec. + {{4, 3}, {"foobar", "baz"}, {{4, 6}}, {9}, {12}}, + // Two frames, one read with extra iovecs, body ends at iovec boundary. + {{4, 3}, {"foobar", "baz"}, {{4, 6, 4, 3}}, {9}, {12}}, + // Two frames, one read with extra iovecs, body ends not at iovec boundary. + {{4, 3}, {"foobar", "baz"}, {{4, 7, 4, 3}}, {9}, {12}}, + // Two frames, two reads with two iovecs each, reads end on frame boundary. + {{4, 3}, {"foobar", "baz"}, {{2, 4}, {2, 1}}, {6, 3}, {9, 3}}, + // Three frames, three reads, extra iovecs, no iovec ends on frame boundary. + {{4, 3, 6}, + {"foobar", "bazquux", "qux"}, + {{4, 3}, {2, 3}, {5, 3}}, + {7, 5, 4}, + {10, 5, 10}}, +}; + +TEST_F(QuicSpdyStreamBodyBufferTest, ReadBody) { + for (size_t test_case_index = 0; + test_case_index < QUIC_ARRAYSIZE(kReadBodyTestData); ++test_case_index) { + const std::vector<QuicByteCount>& frame_header_lengths = + kReadBodyTestData[test_case_index].frame_header_lengths; + const std::vector<const char*>& frame_payloads = + kReadBodyTestData[test_case_index].frame_payloads; + const std::vector<std::vector<QuicByteCount>>& iov_lengths = + kReadBodyTestData[test_case_index].iov_lengths; + const std::vector<QuicByteCount>& expected_total_bytes_read = + kReadBodyTestData[test_case_index].expected_total_bytes_read; + const std::vector<QuicByteCount>& expected_return_values = + kReadBodyTestData[test_case_index].expected_return_values; + + QuicSpdyStreamBodyBuffer body_buffer; + + std::string received_body; + + for (size_t frame_index = 0; frame_index < frame_header_lengths.size(); + ++frame_index) { + // Frame header of first frame can immediately be consumed, but not the + // other frames. Each test case uses a new QuicSpdyStreamBodyBuffer + // instance. + EXPECT_EQ(frame_index == 0 ? frame_header_lengths[frame_index] : 0u, + body_buffer.OnDataHeader(frame_header_lengths[frame_index])); + body_buffer.OnDataPayload(frame_payloads[frame_index]); + received_body.append(frame_payloads[frame_index]); + } + + std::string read_body; + + for (size_t call_index = 0; call_index < iov_lengths.size(); ++call_index) { + // Allocate single buffer for iovecs. + size_t total_iov_length = std::accumulate(iov_lengths[call_index].begin(), + iov_lengths[call_index].end(), + static_cast<size_t>(0)); + std::string buffer(total_iov_length, 'z'); + + // Construct iovecs pointing to contiguous areas in the buffer. + std::vector<iovec> iovecs; + size_t offset = 0; + for (size_t iov_length : iov_lengths[call_index]) { + CHECK(offset + iov_length <= buffer.size()); + iovecs.push_back({&buffer[offset], iov_length}); + offset += iov_length; + } + + // Make sure |total_bytes_read| differs from |expected_total_bytes_read|. + size_t total_bytes_read = expected_total_bytes_read[call_index] + 12; + EXPECT_EQ( + expected_return_values[call_index], + body_buffer.ReadBody(&iovecs[0], iovecs.size(), &total_bytes_read)); + read_body.append(buffer.substr(0, total_bytes_read)); + } + + EXPECT_EQ(received_body.substr(0, read_body.size()), read_body); + EXPECT_EQ(read_body.size() < received_body.size(), + body_buffer.HasBytesToRead()); + } } } // anonymous namespace
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index f9777ec..ddfb651 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -2104,16 +2104,18 @@ // DATA frame. QuicStringPiece data_payload(kDataFramePayload); std::string data_frame = DataFrame(data_payload); + QuicByteCount data_frame_header_length = + data_frame.size() - data_payload.size(); - // DATA frame is not consumed because payload has to be buffered. - // TODO(bnc): Consume frame header as soon as possible. + // DATA frame header is consumed. + // DATA frame payload is not consumed because payload has to be buffered. OnStreamFrame(data_frame); - EXPECT_EQ(0u, NewlyConsumedBytes()); + EXPECT_EQ(data_frame_header_length, NewlyConsumedBytes()); // Consume all but last byte of data. EXPECT_EQ(data_payload.substr(0, data_payload.size() - 1), ReadFromStream(data_payload.size() - 1)); - EXPECT_EQ(data_frame.size() - 1, NewlyConsumedBytes()); + EXPECT_EQ(data_payload.size() - 1, NewlyConsumedBytes()); // Trailing HEADERS frame with QPACK encoded // single header field "custom-key: custom-value".