Commit 05cd3643 authored by Yoichi Osato's avatar Yoichi Osato Committed by Commit Bot

[WebSocket] Rename WebSocketFrame member and update comment.

This is a follow-up patch of previous change:
"[WebSocket] Reduce memcpy at websocket_frame_parser.cc"
https://chromium-review.googlesource.com/c/chromium/src/+/1774420

Bug: 1001915
Change-Id: Ib13c28038a82b4b6ca0eb53a3e4d56ac5242720c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1792030Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarJohn Chen <johnchen@chromium.org>
Reviewed-by: default avatarAdam Rice <ricea@chromium.org>
Auto-Submit: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694826}
parent 177034fb
...@@ -286,7 +286,7 @@ void WebSocket::OnReadDuringOpen(const char* data, int len) { ...@@ -286,7 +286,7 @@ void WebSocket::OnReadDuringOpen(const char* data, int len) {
current_masking_key_ = header->masking_key; current_masking_key_ = header->masking_key;
} }
auto& buffer = frame_chunks[i]->data; auto& buffer = frame_chunks[i]->payload;
std::vector<char> payload(buffer.begin(), buffer.end()); std::vector<char> payload(buffer.begin(), buffer.end());
if (is_current_frame_masked_) { if (is_current_frame_masked_) {
MaskWebSocketFramePayload(current_masking_key_, current_frame_offset_, MaskWebSocketFramePayload(current_masking_key_, current_frame_offset_,
......
...@@ -180,7 +180,7 @@ int WebSocketBasicStream::WriteFrames( ...@@ -180,7 +180,7 @@ int WebSocketBasicStream::WriteFrames(
static_cast<uint64_t>(remaining_size)); static_cast<uint64_t>(remaining_size));
const int frame_size = static_cast<int>(frame->header.payload_length); const int frame_size = static_cast<int>(frame->header.payload_length);
if (frame_size > 0) { if (frame_size > 0) {
const char* const frame_data = frame->data; const char* const frame_data = frame->payload;
std::copy(frame_data, frame_data + frame_size, dest); std::copy(frame_data, frame_data + frame_size, dest);
MaskWebSocketFramePayload(mask, 0, dest, frame_size); MaskWebSocketFramePayload(mask, 0, dest, frame_size);
dest += frame_size; dest += frame_size;
...@@ -360,7 +360,7 @@ int WebSocketBasicStream::ConvertChunkToFrame( ...@@ -360,7 +360,7 @@ int WebSocketBasicStream::ConvertChunkToFrame(
} }
DCHECK(current_frame_header_) << "Unexpected header-less chunk received " DCHECK(current_frame_header_) << "Unexpected header-less chunk received "
<< "(final_chunk = " << chunk->final_chunk << "(final_chunk = " << chunk->final_chunk
<< ", data size = " << chunk->data.size() << ", payload size = " << chunk->payload.size()
<< ") (bug in WebSocketFrameParser?)"; << ") (bug in WebSocketFrameParser?)";
const bool is_final_chunk = chunk->final_chunk; const bool is_final_chunk = chunk->final_chunk;
const WebSocketFrameHeader::OpCode opcode = current_frame_header_->opcode; const WebSocketFrameHeader::OpCode opcode = current_frame_header_->opcode;
...@@ -384,13 +384,13 @@ int WebSocketBasicStream::ConvertChunkToFrame( ...@@ -384,13 +384,13 @@ int WebSocketBasicStream::ConvertChunkToFrame(
if (!is_final_chunk) { if (!is_final_chunk) {
DVLOG(2) << "Encountered a split control frame, opcode " << opcode; DVLOG(2) << "Encountered a split control frame, opcode " << opcode;
AddToIncompleteControlFrameBody(chunk->data); AddToIncompleteControlFrameBody(chunk->payload);
return OK; return OK;
} }
if (!incomplete_control_frame_body_.empty()) { if (!incomplete_control_frame_body_.empty()) {
DVLOG(2) << "Rejoining a split control frame, opcode " << opcode; DVLOG(2) << "Rejoining a split control frame, opcode " << opcode;
AddToIncompleteControlFrameBody(chunk->data); AddToIncompleteControlFrameBody(chunk->payload);
DCHECK(is_final_chunk); DCHECK(is_final_chunk);
DCHECK(complete_control_frame_body_.empty()); DCHECK(complete_control_frame_body_.empty());
complete_control_frame_body_ = std::move(incomplete_control_frame_body_); complete_control_frame_body_ = std::move(incomplete_control_frame_body_);
...@@ -403,13 +403,13 @@ int WebSocketBasicStream::ConvertChunkToFrame( ...@@ -403,13 +403,13 @@ int WebSocketBasicStream::ConvertChunkToFrame(
// header. A check for exact equality can only be used when the whole frame // header. A check for exact equality can only be used when the whole frame
// arrives in one chunk. // arrives in one chunk.
DCHECK_GE(current_frame_header_->payload_length, DCHECK_GE(current_frame_header_->payload_length,
base::checked_cast<uint64_t>(chunk->data.size())); base::checked_cast<uint64_t>(chunk->payload.size()));
DCHECK(!is_first_chunk || !is_final_chunk || DCHECK(!is_first_chunk || !is_final_chunk ||
current_frame_header_->payload_length == current_frame_header_->payload_length ==
base::checked_cast<uint64_t>(chunk->data.size())); base::checked_cast<uint64_t>(chunk->payload.size()));
// Convert the chunk to a complete frame. // Convert the chunk to a complete frame.
*frame = CreateFrame(is_final_chunk, chunk->data); *frame = CreateFrame(is_final_chunk, chunk->payload);
return OK; return OK;
} }
...@@ -429,7 +429,7 @@ std::unique_ptr<WebSocketFrame> WebSocketBasicStream::CreateFrame( ...@@ -429,7 +429,7 @@ std::unique_ptr<WebSocketFrame> WebSocketBasicStream::CreateFrame(
result_frame->header.CopyFrom(*current_frame_header_); result_frame->header.CopyFrom(*current_frame_header_);
result_frame->header.final = is_final_chunk_in_message; result_frame->header.final = is_final_chunk_in_message;
result_frame->header.payload_length = data.size(); result_frame->header.payload_length = data.size();
result_frame->data = data.data(); result_frame->payload = data.data();
// Ensure that opcodes Text and Binary are only used for the first frame in // Ensure that opcodes Text and Binary are only used for the first frame in
// the message. Also clear the reserved bits. // the message. Also clear the reserved bits.
// TODO(ricea): If a future extension requires the reserved bits to be // TODO(ricea): If a future extension requires the reserved bits to be
......
...@@ -146,9 +146,7 @@ class NET_EXPORT_PRIVATE WebSocketBasicStream : public WebSocketStream { ...@@ -146,9 +146,7 @@ class NET_EXPORT_PRIVATE WebSocketBasicStream : public WebSocketStream {
// bounds checks. // bounds checks.
void AddToIncompleteControlFrameBody(base::span<const char> data); void AddToIncompleteControlFrameBody(base::span<const char> data);
// Storage for pending reads. All active WebSockets spend all the time with a // Storage for pending reads.
// call to ReadFrames() pending, so there is no benefit in trying to share
// this between sockets.
scoped_refptr<IOBufferWithSize> read_buffer_; scoped_refptr<IOBufferWithSize> read_buffer_;
// The connection, wrapped in a ClientSocketHandle so that we can prevent it // The connection, wrapped in a ClientSocketHandle so that we can prevent it
...@@ -164,7 +162,7 @@ class NET_EXPORT_PRIVATE WebSocketBasicStream : public WebSocketStream { ...@@ -164,7 +162,7 @@ class NET_EXPORT_PRIVATE WebSocketBasicStream : public WebSocketStream {
// Although it should rarely happen in practice, a control frame can arrive // Although it should rarely happen in practice, a control frame can arrive
// broken into chunks. This variable provides storage for a partial control // broken into chunks. This variable provides storage for a partial control
// frame until the rest arrives. It will be NULL the rest of the time. // frame until the rest arrives. It will be empty the rest of the time.
std::vector<char> incomplete_control_frame_body_; std::vector<char> incomplete_control_frame_body_;
// Storage for payload of combined (see |incomplete_control_frame_body_|) // Storage for payload of combined (see |incomplete_control_frame_body_|)
// control frame. // control frame.
......
...@@ -257,7 +257,7 @@ class WebSocketBasicStreamSocketWriteTest ...@@ -257,7 +257,7 @@ class WebSocketBasicStreamSocketWriteTest
frame_buffers_.push_back(buffer); frame_buffers_.push_back(buffer);
memcpy(buffer->data(), kWriteFrame + kWriteFrameSize - payload_size, memcpy(buffer->data(), kWriteFrame + kWriteFrameSize - payload_size,
payload_size); payload_size);
frame->data = buffer->data(); frame->payload = buffer->data();
WebSocketFrameHeader& header = frame->header; WebSocketFrameHeader& header = frame->header;
header.final = true; header.final = true;
header.masked = true; header.masked = true;
...@@ -356,7 +356,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest, HeaderOnlyChunk) { ...@@ -356,7 +356,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest, HeaderOnlyChunk) {
EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk()); EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk());
ASSERT_EQ(1U, frames_.size()); ASSERT_EQ(1U, frames_.size());
EXPECT_EQ(nullptr, frames_[0]->data); EXPECT_EQ(nullptr, frames_[0]->payload);
EXPECT_EQ(0U, frames_[0]->header.payload_length); EXPECT_EQ(0U, frames_[0]->header.payload_length);
EXPECT_EQ(WebSocketFrameHeader::kOpCodeText, frames_[0]->header.opcode); EXPECT_EQ(WebSocketFrameHeader::kOpCodeText, frames_[0]->header.opcode);
} }
...@@ -372,7 +372,7 @@ TEST_F(WebSocketBasicStreamSocketTest, HeaderBodySeparated) { ...@@ -372,7 +372,7 @@ TEST_F(WebSocketBasicStreamSocketTest, HeaderBodySeparated) {
CreateStream(reads, base::span<MockWrite>()); CreateStream(reads, base::span<MockWrite>());
EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk()); EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk());
ASSERT_EQ(1U, frames_.size()); ASSERT_EQ(1U, frames_.size());
EXPECT_EQ(nullptr, frames_[0]->data); EXPECT_EQ(nullptr, frames_[0]->payload);
EXPECT_EQ(WebSocketFrameHeader::kOpCodeText, frames_[0]->header.opcode); EXPECT_EQ(WebSocketFrameHeader::kOpCodeText, frames_[0]->header.opcode);
frames_.clear(); frames_.clear();
EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()),
...@@ -580,7 +580,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest, EmptyFirstFrame) { ...@@ -580,7 +580,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest, EmptyFirstFrame) {
EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk()); EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk());
ASSERT_EQ(1U, frames_.size()); ASSERT_EQ(1U, frames_.size());
EXPECT_EQ(nullptr, frames_[0]->data); EXPECT_EQ(nullptr, frames_[0]->payload);
EXPECT_EQ(0U, frames_[0]->header.payload_length); EXPECT_EQ(0U, frames_[0]->header.payload_length);
} }
...@@ -627,7 +627,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest, EmptyFinalFrame) { ...@@ -627,7 +627,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest, EmptyFinalFrame) {
EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk()); EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk());
ASSERT_EQ(1U, frames_.size()); ASSERT_EQ(1U, frames_.size());
EXPECT_EQ(nullptr, frames_[0]->data); EXPECT_EQ(nullptr, frames_[0]->payload);
EXPECT_EQ(0U, frames_[0]->header.payload_length); EXPECT_EQ(0U, frames_[0]->header.payload_length);
} }
...@@ -658,7 +658,7 @@ TEST_F(WebSocketBasicStreamSocketTest, HttpReadBufferIsUsed) { ...@@ -658,7 +658,7 @@ TEST_F(WebSocketBasicStreamSocketTest, HttpReadBufferIsUsed) {
EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk()); EXPECT_THAT(stream_->ReadFrames(&frames_, cb_.callback()), IsOk());
ASSERT_EQ(1U, frames_.size()); ASSERT_EQ(1U, frames_.size());
ASSERT_TRUE(frames_[0]->data); ASSERT_TRUE(frames_[0]->payload);
EXPECT_EQ(UINT64_C(6), frames_[0]->header.payload_length); EXPECT_EQ(UINT64_C(6), frames_[0]->header.payload_length);
} }
...@@ -673,7 +673,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest, ...@@ -673,7 +673,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest,
IsError(ERR_IO_PENDING)); IsError(ERR_IO_PENDING));
EXPECT_THAT(cb_.WaitForResult(), IsOk()); EXPECT_THAT(cb_.WaitForResult(), IsOk());
ASSERT_EQ(1U, frames_.size()); ASSERT_EQ(1U, frames_.size());
ASSERT_TRUE(frames_[0]->data); ASSERT_TRUE(frames_[0]->payload);
EXPECT_EQ(UINT64_C(6), frames_[0]->header.payload_length); EXPECT_EQ(UINT64_C(6), frames_[0]->header.payload_length);
EXPECT_EQ(WebSocketFrameHeader::kOpCodeText, frames_[0]->header.opcode); EXPECT_EQ(WebSocketFrameHeader::kOpCodeText, frames_[0]->header.opcode);
} }
...@@ -694,7 +694,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest, ...@@ -694,7 +694,7 @@ TEST_F(WebSocketBasicStreamSocketSingleReadTest,
ASSERT_EQ(1U, frames_.size()); ASSERT_EQ(1U, frames_.size());
EXPECT_EQ(WebSocketFrameHeader::kOpCodeClose, frames_[0]->header.opcode); EXPECT_EQ(WebSocketFrameHeader::kOpCodeClose, frames_[0]->header.opcode);
EXPECT_EQ(kCloseFrameSize - 2, frames_[0]->header.payload_length); EXPECT_EQ(kCloseFrameSize - 2, frames_[0]->header.payload_length);
EXPECT_EQ(std::string(frames_[0]->data, kCloseFrameSize - 2), EXPECT_EQ(std::string(frames_[0]->payload, kCloseFrameSize - 2),
std::string(kCloseFrame + 2, kCloseFrameSize - 2)); std::string(kCloseFrame + 2, kCloseFrameSize - 2));
} }
...@@ -946,7 +946,7 @@ TEST_F(WebSocketBasicStreamSocketTest, WriteNonNulMask) { ...@@ -946,7 +946,7 @@ TEST_F(WebSocketBasicStreamSocketTest, WriteNonNulMask) {
const size_t payload_size = unmasked_payload.size(); const size_t payload_size = unmasked_payload.size();
auto buffer = base::MakeRefCounted<IOBuffer>(payload_size); auto buffer = base::MakeRefCounted<IOBuffer>(payload_size);
memcpy(buffer->data(), unmasked_payload.data(), payload_size); memcpy(buffer->data(), unmasked_payload.data(), payload_size);
frame->data = buffer->data(); frame->payload = buffer->data();
WebSocketFrameHeader& header = frame->header; WebSocketFrameHeader& header = frame->header;
header.final = true; header.final = true;
header.masked = true; header.masked = true;
......
...@@ -703,7 +703,7 @@ ChannelState WebSocketChannel::HandleFrame( ...@@ -703,7 +703,7 @@ ChannelState WebSocketChannel::HandleFrame(
// Respond to the frame appropriately to its type. // Respond to the frame appropriately to its type.
return HandleFrameByState( return HandleFrameByState(
opcode, frame->header.final, opcode, frame->header.final,
base::make_span(frame->data, frame->header.payload_length)); base::make_span(frame->payload, frame->header.payload_length));
} }
ChannelState WebSocketChannel::HandleFrameByState( ChannelState WebSocketChannel::HandleFrameByState(
...@@ -902,7 +902,7 @@ ChannelState WebSocketChannel::SendFrameInternal( ...@@ -902,7 +902,7 @@ ChannelState WebSocketChannel::SendFrameInternal(
header.final = fin; header.final = fin;
header.masked = true; header.masked = true;
header.payload_length = buffer_size; header.payload_length = buffer_size;
frame->data = buffer->data(); frame->payload = buffer->data();
if (data_being_sent_) { if (data_being_sent_) {
// Either the link to the WebSocket server is saturated, or several messages // Either the link to the WebSocket server is saturated, or several messages
......
...@@ -72,9 +72,9 @@ std::ostream& operator<<(std::ostream& os, const WebSocketFrameHeader& header) { ...@@ -72,9 +72,9 @@ std::ostream& operator<<(std::ostream& os, const WebSocketFrameHeader& header) {
std::ostream& operator<<(std::ostream& os, const WebSocketFrame& frame) { std::ostream& operator<<(std::ostream& os, const WebSocketFrame& frame) {
os << "{" << frame.header << ", "; os << "{" << frame.header << ", ";
if (frame.data) { if (frame.payload) {
return os << "\"" return os << "\""
<< base::StringPiece(frame.data, frame.header.payload_length) << base::StringPiece(frame.payload, frame.header.payload_length)
<< "\"}"; << "\"}";
} }
return os << "NULL}"; return os << "NULL}";
...@@ -362,7 +362,7 @@ std::vector<std::unique_ptr<WebSocketFrame>> CreateFrameVector( ...@@ -362,7 +362,7 @@ std::vector<std::unique_ptr<WebSocketFrame>> CreateFrameVector(
auto buffer = base::MakeRefCounted<IOBuffer>(frame_length); auto buffer = base::MakeRefCounted<IOBuffer>(frame_length);
result_frame_data->push_back(buffer); result_frame_data->push_back(buffer);
memcpy(buffer->data(), source_frame.data, frame_length); memcpy(buffer->data(), source_frame.data, frame_length);
result_frame->data = buffer->data(); result_frame->payload = buffer->data();
} }
result_frames.push_back(std::move(result_frame)); result_frames.push_back(std::move(result_frame));
} }
...@@ -424,7 +424,7 @@ class EqualsFramesMatcher : public ::testing::MatcherInterface< ...@@ -424,7 +424,7 @@ class EqualsFramesMatcher : public ::testing::MatcherInterface<
return false; return false;
} }
if (expected_length != 0 && if (expected_length != 0 &&
memcmp(actual_frame.data, expected_frame.data, memcmp(actual_frame.payload, expected_frame.data,
actual_frame.header.payload_length) != 0) { actual_frame.header.payload_length) != 0) {
*listener << "the data content differs"; *listener << "the data content differs";
return false; return false;
...@@ -595,8 +595,8 @@ class EchoeyFakeWebSocketStream : public FakeWebSocketStream { ...@@ -595,8 +595,8 @@ class EchoeyFakeWebSocketStream : public FakeWebSocketStream {
for (const auto& frame : *frames) { for (const auto& frame : *frames) {
auto buffer = base::MakeRefCounted<IOBuffer>( auto buffer = base::MakeRefCounted<IOBuffer>(
static_cast<size_t>(frame->header.payload_length)); static_cast<size_t>(frame->header.payload_length));
memcpy(buffer->data(), frame->data, frame->header.payload_length); memcpy(buffer->data(), frame->payload, frame->header.payload_length);
frame->data = buffer->data(); frame->payload = buffer->data();
buffers_.push_back(buffer); buffers_.push_back(buffer);
} }
stored_frames_.insert(stored_frames_.end(), stored_frames_.insert(stored_frames_.end(),
...@@ -2349,8 +2349,8 @@ TEST_F(WebSocketChannelStreamTest, WrittenBinaryFramesAre8BitClean) { ...@@ -2349,8 +2349,8 @@ TEST_F(WebSocketChannelStreamTest, WrittenBinaryFramesAre8BitClean) {
ASSERT_EQ(1U, frames->size()); ASSERT_EQ(1U, frames->size());
const WebSocketFrame* out_frame = (*frames)[0].get(); const WebSocketFrame* out_frame = (*frames)[0].get();
EXPECT_EQ(kBinaryBlobSize, out_frame->header.payload_length); EXPECT_EQ(kBinaryBlobSize, out_frame->header.payload_length);
ASSERT_TRUE(out_frame->data); ASSERT_TRUE(out_frame->payload);
EXPECT_EQ(0, memcmp(kBinaryBlob, out_frame->data, kBinaryBlobSize)); EXPECT_EQ(0, memcmp(kBinaryBlob, out_frame->payload, kBinaryBlobSize));
} }
// Test the read path for 8-bit cleanliness as well. // Test the read path for 8-bit cleanliness as well.
...@@ -2362,7 +2362,7 @@ TEST_F(WebSocketChannelEventInterfaceTest, ReadBinaryFramesAre8BitClean) { ...@@ -2362,7 +2362,7 @@ TEST_F(WebSocketChannelEventInterfaceTest, ReadBinaryFramesAre8BitClean) {
frame_header.payload_length = kBinaryBlobSize; frame_header.payload_length = kBinaryBlobSize;
auto buffer = base::MakeRefCounted<IOBuffer>(kBinaryBlobSize); auto buffer = base::MakeRefCounted<IOBuffer>(kBinaryBlobSize);
memcpy(buffer->data(), kBinaryBlob, kBinaryBlobSize); memcpy(buffer->data(), kBinaryBlob, kBinaryBlobSize);
frame->data = buffer->data(); frame->payload = buffer->data();
std::vector<std::unique_ptr<WebSocketFrame>> frames; std::vector<std::unique_ptr<WebSocketFrame>> frames;
frames.push_back(std::move(frame)); frames.push_back(std::move(frame));
auto stream = std::make_unique<ReadableFakeWebSocketStream>(); auto stream = std::make_unique<ReadableFakeWebSocketStream>();
......
...@@ -137,9 +137,10 @@ int WebSocketDeflateStream::Deflate( ...@@ -137,9 +137,10 @@ int WebSocketDeflateStream::Deflate(
frames_to_write.push_back(std::move(frame)); frames_to_write.push_back(std::move(frame));
current_writing_opcode_ = WebSocketFrameHeader::kOpCodeContinuation; current_writing_opcode_ = WebSocketFrameHeader::kOpCodeContinuation;
} else { } else {
if (frame->data && if (frame->payload &&
!deflater_.AddBytes( !deflater_.AddBytes(
frame->data, static_cast<size_t>(frame->header.payload_length))) { frame->payload,
static_cast<size_t>(frame->header.payload_length))) {
DVLOG(1) << "WebSocket protocol error. " DVLOG(1) << "WebSocket protocol error. "
<< "deflater_.AddBytes() returns an error."; << "deflater_.AddBytes() returns an error.";
return ERR_WS_PROTOCOL_ERROR; return ERR_WS_PROTOCOL_ERROR;
...@@ -221,7 +222,7 @@ int WebSocketDeflateStream::AppendCompressedFrame( ...@@ -221,7 +222,7 @@ int WebSocketDeflateStream::AppendCompressedFrame(
compressed->header.final = header.final; compressed->header.final = header.final;
compressed->header.reserved1 = compressed->header.reserved1 =
(opcode != WebSocketFrameHeader::kOpCodeContinuation); (opcode != WebSocketFrameHeader::kOpCodeContinuation);
compressed->data = compressed_payload->data(); compressed->payload = compressed_payload->data();
compressed->header.payload_length = compressed_payload->size(); compressed->header.payload_length = compressed_payload->size();
current_writing_opcode_ = WebSocketFrameHeader::kOpCodeContinuation; current_writing_opcode_ = WebSocketFrameHeader::kOpCodeContinuation;
...@@ -272,7 +273,7 @@ int WebSocketDeflateStream::AppendPossiblyCompressedMessage( ...@@ -272,7 +273,7 @@ int WebSocketDeflateStream::AppendPossiblyCompressedMessage(
compressed->header.opcode = opcode; compressed->header.opcode = opcode;
compressed->header.final = true; compressed->header.final = true;
compressed->header.reserved1 = true; compressed->header.reserved1 = true;
compressed->data = compressed_payload->data(); compressed->payload = compressed_payload->data();
compressed->header.payload_length = compressed_payload->size(); compressed->header.payload_length = compressed_payload->size();
predictor_->RecordWrittenDataFrame(compressed.get()); predictor_->RecordWrittenDataFrame(compressed.get());
...@@ -319,9 +320,10 @@ int WebSocketDeflateStream::Inflate( ...@@ -319,9 +320,10 @@ int WebSocketDeflateStream::Inflate(
frames_to_output.push_back(std::move(frame)); frames_to_output.push_back(std::move(frame));
} else { } else {
DCHECK_EQ(reading_state_, READING_COMPRESSED_MESSAGE); DCHECK_EQ(reading_state_, READING_COMPRESSED_MESSAGE);
if (frame->data && if (frame->payload &&
!inflater_.AddBytes( !inflater_.AddBytes(
frame->data, static_cast<size_t>(frame->header.payload_length))) { frame->payload,
static_cast<size_t>(frame->header.payload_length))) {
DVLOG(1) << "WebSocket protocol error. " DVLOG(1) << "WebSocket protocol error. "
<< "inflater_.AddBytes() returns an error."; << "inflater_.AddBytes() returns an error.";
return ERR_WS_PROTOCOL_ERROR; return ERR_WS_PROTOCOL_ERROR;
...@@ -354,7 +356,7 @@ int WebSocketDeflateStream::Inflate( ...@@ -354,7 +356,7 @@ int WebSocketDeflateStream::Inflate(
inflated->header.opcode = current_reading_opcode_; inflated->header.opcode = current_reading_opcode_;
inflated->header.final = is_final; inflated->header.final = is_final;
inflated->header.reserved1 = false; inflated->header.reserved1 = false;
inflated->data = data->data(); inflated->payload = data->data();
inflated->header.payload_length = data->size(); inflated->header.payload_length = data->size();
DVLOG(3) << "Inflated frame: opcode=" << inflated->header.opcode DVLOG(3) << "Inflated frame: opcode=" << inflated->header.opcode
<< " final=" << inflated->header.final << " final=" << inflated->header.final
......
...@@ -88,7 +88,7 @@ class WebSocketFuzzedStream final : public WebSocketStream { ...@@ -88,7 +88,7 @@ class WebSocketFuzzedStream final : public WebSocketStream {
auto buffer = base::MakeRefCounted<IOBufferWithSize>(payload.size()); auto buffer = base::MakeRefCounted<IOBufferWithSize>(payload.size());
memcpy(buffer->data(), payload.data(), payload.size()); memcpy(buffer->data(), payload.data(), payload.size());
buffers_.push_back(buffer); buffers_.push_back(buffer);
frame->data = buffer->data(); frame->payload = buffer->data();
frame->header.payload_length = payload.size(); frame->header.payload_length = payload.size();
return frame; return frame;
} }
......
...@@ -63,8 +63,9 @@ std::string ToString(const scoped_refptr<IOBufferWithSize>& buffer) { ...@@ -63,8 +63,9 @@ std::string ToString(const scoped_refptr<IOBufferWithSize>& buffer) {
} }
std::string ToString(const WebSocketFrame* frame) { std::string ToString(const WebSocketFrame* frame) {
return frame->data ? std::string(frame->data, frame->header.payload_length) return frame->payload
: ""; ? std::string(frame->payload, frame->header.payload_length)
: "";
} }
std::string ToString(const std::unique_ptr<WebSocketFrame>& frame) { std::string ToString(const std::unique_ptr<WebSocketFrame>& frame) {
...@@ -249,7 +250,7 @@ class WebSocketDeflateStreamTest : public ::testing::Test { ...@@ -249,7 +250,7 @@ class WebSocketDeflateStreamTest : public ::testing::Test {
frame->header.reserved1 = (flag & kReserved1); frame->header.reserved1 = (flag & kReserved1);
auto buffer = std::make_unique<char[]>(data.size()); auto buffer = std::make_unique<char[]>(data.size());
memcpy(buffer.get(), data.c_str(), data.size()); memcpy(buffer.get(), data.c_str(), data.size());
frame->data = buffer.get(); frame->payload = buffer.get();
data_buffers.push_back(std::move(buffer)); data_buffers.push_back(std::move(buffer));
frame->header.payload_length = data.size(); frame->header.payload_length = data.size();
frames->push_back(std::move(frame)); frames->push_back(std::move(frame));
...@@ -1202,8 +1203,9 @@ TEST_F(WebSocketDeflateStreamTest, LargeDeflatedFramesShouldBeSplit) { ...@@ -1202,8 +1203,9 @@ TEST_F(WebSocketDeflateStreamTest, LargeDeflatedFramesShouldBeSplit) {
ASSERT_THAT(deflate_stream_->WriteFrames(&frames, CompletionOnceCallback()), ASSERT_THAT(deflate_stream_->WriteFrames(&frames, CompletionOnceCallback()),
IsOk()); IsOk());
for (auto& frame : *stub.frames()) { for (auto& frame : *stub.frames()) {
buffers.push_back(std::string(frame->data, frame->header.payload_length)); buffers.push_back(
frame->data = (buffers.end() - 1)->data(); std::string(frame->payload, frame->header.payload_length));
frame->payload = (buffers.end() - 1)->data();
} }
total_compressed_frames.insert( total_compressed_frames.insert(
total_compressed_frames.end(), total_compressed_frames.end(),
......
...@@ -99,10 +99,14 @@ struct NET_EXPORT_PRIVATE WebSocketFrame { ...@@ -99,10 +99,14 @@ struct NET_EXPORT_PRIVATE WebSocketFrame {
// |header| is always present. // |header| is always present.
WebSocketFrameHeader header; WebSocketFrameHeader header;
// |data| is always unmasked even if the frame is masked. The size of |data| // |payload| is always unmasked even if the frame is masked. The size of
// is given by |header.payload_length|. // |payload| is given by |header.payload_length|.
// TODO(yoichio): Rename this to "payload". // The lifetime of |payload| is not defined by WebSocketFrameChunk. It is the
const char* data = nullptr; // responsibility of the creator to ensure it remains valid for the lifetime
// of this object. This should be documented in the code that creates this
// object.
// TODO(yoicho): Find more better way to clarify the life cycle.
const char* payload = nullptr;
private: private:
DISALLOW_COPY_AND_ASSIGN(WebSocketFrame); DISALLOW_COPY_AND_ASSIGN(WebSocketFrame);
...@@ -136,10 +140,14 @@ struct NET_EXPORT WebSocketFrameChunk { ...@@ -136,10 +140,14 @@ struct NET_EXPORT WebSocketFrameChunk {
// Indicates this part is the last chunk of a frame. // Indicates this part is the last chunk of a frame.
bool final_chunk; bool final_chunk;
// |data| is always unmasked even if the frame is masked. |data| might be // |payload| is always unmasked even if the frame is masked. |payload| might
// empty in the first chunk. // be empty in the first chunk.
// TODO(yoichio): Rename this to "payload". // The lifetime of |payload| is not defined by WebSocketFrameChunk. It is the
base::span<const char> data; // responsibility of the creator to ensure it remains valid for the lifetime
// of this object. This should be documented in the code that creates this
// object.
// TODO(yoicho): Find more better way to clarify the life cycle.
base::span<const char> payload;
private: private:
DISALLOW_COPY_AND_ASSIGN(WebSocketFrameChunk); DISALLOW_COPY_AND_ASSIGN(WebSocketFrameChunk);
......
...@@ -191,7 +191,7 @@ std::unique_ptr<WebSocketFrameChunk> WebSocketFrameParser::DecodeFramePayload( ...@@ -191,7 +191,7 @@ std::unique_ptr<WebSocketFrameChunk> WebSocketFrameParser::DecodeFramePayload(
} }
frame_chunk->final_chunk = false; frame_chunk->final_chunk = false;
if (chunk_data_size > 0) { if (chunk_data_size > 0) {
frame_chunk->data = data->subspan(0, chunk_data_size); frame_chunk->payload = data->subspan(0, chunk_data_size);
*data = data->subspan(chunk_data_size); *data = data->subspan(chunk_data_size);
frame_offset_ += chunk_data_size; frame_offset_ += chunk_data_size;
} }
......
...@@ -70,8 +70,8 @@ TEST(WebSocketFrameParserTest, DecodeNormalFrame) { ...@@ -70,8 +70,8 @@ TEST(WebSocketFrameParserTest, DecodeNormalFrame) {
} }
EXPECT_TRUE(frame->final_chunk); EXPECT_TRUE(frame->final_chunk);
ASSERT_EQ(static_cast<size_t>(kHelloLength), frame->data.size()); ASSERT_EQ(static_cast<size_t>(kHelloLength), frame->payload.size());
EXPECT_TRUE(std::equal(kHello, kHello + kHelloLength, frame->data.data())); EXPECT_TRUE(std::equal(kHello, kHello + kHelloLength, frame->payload.data()));
} }
TEST(WebSocketFrameParserTest, DecodeMaskedFrame) { TEST(WebSocketFrameParserTest, DecodeMaskedFrame) {
...@@ -97,9 +97,9 @@ TEST(WebSocketFrameParserTest, DecodeMaskedFrame) { ...@@ -97,9 +97,9 @@ TEST(WebSocketFrameParserTest, DecodeMaskedFrame) {
} }
EXPECT_TRUE(frame->final_chunk); EXPECT_TRUE(frame->final_chunk);
ASSERT_EQ(static_cast<size_t>(kHelloLength), frame->data.size()); ASSERT_EQ(static_cast<size_t>(kHelloLength), frame->payload.size());
std::string payload(frame->data.data(), frame->data.size()); std::string payload(frame->payload.data(), frame->payload.size());
MaskWebSocketFramePayload(header->masking_key, 0, &payload[0], MaskWebSocketFramePayload(header->masking_key, 0, &payload[0],
payload.size()); payload.size());
EXPECT_EQ(payload, kHello); EXPECT_EQ(payload, kHello);
...@@ -152,11 +152,11 @@ TEST(WebSocketFrameParserTest, DecodeManyFrames) { ...@@ -152,11 +152,11 @@ TEST(WebSocketFrameParserTest, DecodeManyFrames) {
continue; continue;
EXPECT_TRUE(frame->final_chunk); EXPECT_TRUE(frame->final_chunk);
ASSERT_EQ(kInputs[i].expected_payload_length, ASSERT_EQ(kInputs[i].expected_payload_length,
static_cast<uint64_t>(frame->data.size())); static_cast<uint64_t>(frame->payload.size()));
EXPECT_TRUE(std::equal( EXPECT_TRUE(std::equal(
kInputs[i].expected_payload, kInputs[i].expected_payload,
kInputs[i].expected_payload + kInputs[i].expected_payload_length, kInputs[i].expected_payload + kInputs[i].expected_payload_length,
frame->data.data())); frame->payload.data()));
const WebSocketFrameHeader* header = frame->header.get(); const WebSocketFrameHeader* header = frame->header.get();
EXPECT_TRUE(header != nullptr); EXPECT_TRUE(header != nullptr);
...@@ -198,11 +198,11 @@ TEST(WebSocketFrameParserTest, DecodePartialFrame) { ...@@ -198,11 +198,11 @@ TEST(WebSocketFrameParserTest, DecodePartialFrame) {
continue; continue;
EXPECT_FALSE(frame1->final_chunk); EXPECT_FALSE(frame1->final_chunk);
if (expected1.size() == 0) { if (expected1.size() == 0) {
EXPECT_EQ(nullptr, frame1->data.data()); EXPECT_EQ(nullptr, frame1->payload.data());
} else { } else {
ASSERT_EQ(cutting_pos, static_cast<size_t>(frame1->data.size())); ASSERT_EQ(cutting_pos, static_cast<size_t>(frame1->payload.size()));
EXPECT_TRUE( EXPECT_TRUE(std::equal(expected1.begin(), expected1.end(),
std::equal(expected1.begin(), expected1.end(), frame1->data.data())); frame1->payload.data()));
} }
const WebSocketFrameHeader* header1 = frame1->header.get(); const WebSocketFrameHeader* header1 = frame1->header.get();
EXPECT_TRUE(header1 != nullptr); EXPECT_TRUE(header1 != nullptr);
...@@ -228,11 +228,12 @@ TEST(WebSocketFrameParserTest, DecodePartialFrame) { ...@@ -228,11 +228,12 @@ TEST(WebSocketFrameParserTest, DecodePartialFrame) {
continue; continue;
EXPECT_TRUE(frame2->final_chunk); EXPECT_TRUE(frame2->final_chunk);
if (expected2.size() == 0) { if (expected2.size() == 0) {
EXPECT_EQ(nullptr, frame2->data.data()); EXPECT_EQ(nullptr, frame1->payload.data());
} else { } else {
ASSERT_EQ(expected2.size(), static_cast<uint64_t>(frame2->data.size())); ASSERT_EQ(expected2.size(),
EXPECT_TRUE( static_cast<uint64_t>(frame2->payload.size()));
std::equal(expected2.begin(), expected2.end(), frame2->data.data())); EXPECT_TRUE(std::equal(expected2.begin(), expected2.end(),
frame2->payload.data()));
} }
const WebSocketFrameHeader* header2 = frame2->header.get(); const WebSocketFrameHeader* header2 = frame2->header.get();
EXPECT_TRUE(header2 == nullptr); EXPECT_TRUE(header2 == nullptr);
...@@ -269,11 +270,13 @@ TEST(WebSocketFrameParserTest, DecodePartialMaskedFrame) { ...@@ -269,11 +270,13 @@ TEST(WebSocketFrameParserTest, DecodePartialMaskedFrame) {
if (!header1) if (!header1)
continue; continue;
if (expected1.size() == 0) { if (expected1.size() == 0) {
EXPECT_EQ(nullptr, frame1->data.data()); EXPECT_EQ(nullptr, frame1->payload.data());
} else { } else {
ASSERT_EQ(expected1.size(), static_cast<uint64_t>(frame1->data.size())); ASSERT_EQ(expected1.size(),
std::vector<char> payload1(frame1->data.data(), static_cast<uint64_t>(frame1->payload.size()));
frame1->data.data() + frame1->data.size()); std::vector<char> payload1(
frame1->payload.data(),
frame1->payload.data() + frame1->payload.size());
MaskWebSocketFramePayload(header1->masking_key, 0, &payload1[0], MaskWebSocketFramePayload(header1->masking_key, 0, &payload1[0],
payload1.size()); payload1.size());
EXPECT_EQ(expected1, payload1); EXPECT_EQ(expected1, payload1);
...@@ -298,11 +301,13 @@ TEST(WebSocketFrameParserTest, DecodePartialMaskedFrame) { ...@@ -298,11 +301,13 @@ TEST(WebSocketFrameParserTest, DecodePartialMaskedFrame) {
continue; continue;
EXPECT_TRUE(frame2->final_chunk); EXPECT_TRUE(frame2->final_chunk);
if (expected2.size() == 0) { if (expected2.size() == 0) {
EXPECT_EQ(nullptr, frame2->data.data()); EXPECT_EQ(nullptr, frame2->payload.data());
} else { } else {
ASSERT_EQ(expected2.size(), static_cast<uint64_t>(frame2->data.size())); ASSERT_EQ(expected2.size(),
std::vector<char> payload2(frame2->data.data(), static_cast<uint64_t>(frame2->payload.size()));
frame2->data.data() + frame2->data.size()); std::vector<char> payload2(
frame2->payload.data(),
frame2->payload.data() + frame2->payload.size());
MaskWebSocketFramePayload(header1->masking_key, cutting_pos, &payload2[0], MaskWebSocketFramePayload(header1->masking_key, cutting_pos, &payload2[0],
payload2.size()); payload2.size());
EXPECT_EQ(expected2, payload2); EXPECT_EQ(expected2, payload2);
...@@ -348,12 +353,12 @@ TEST(WebSocketFrameParserTest, DecodeFramesOfVariousLengths) { ...@@ -348,12 +353,12 @@ TEST(WebSocketFrameParserTest, DecodeFramesOfVariousLengths) {
} }
std::vector<char> expected_payload(input_payload_size, 'a'); std::vector<char> expected_payload(input_payload_size, 'a');
if (expected_payload.size() == 0) { if (expected_payload.size() == 0) {
EXPECT_EQ(nullptr, frame->data.data()); EXPECT_EQ(nullptr, frame->payload.data());
} else { } else {
ASSERT_EQ(expected_payload.size(), ASSERT_EQ(expected_payload.size(),
static_cast<uint64_t>(frame->data.size())); static_cast<uint64_t>(frame->payload.size()));
EXPECT_TRUE(std::equal(expected_payload.begin(), expected_payload.end(), EXPECT_TRUE(std::equal(expected_payload.begin(), expected_payload.end(),
frame->data.data())); frame->payload.data()));
} }
const WebSocketFrameHeader* header = frame->header.get(); const WebSocketFrameHeader* header = frame->header.get();
EXPECT_TRUE(header != nullptr); EXPECT_TRUE(header != nullptr);
...@@ -409,7 +414,7 @@ TEST(WebSocketFrameParserTest, DecodePartialHeader) { ...@@ -409,7 +414,7 @@ TEST(WebSocketFrameParserTest, DecodePartialHeader) {
} else { } else {
EXPECT_FALSE(frame->final_chunk); EXPECT_FALSE(frame->final_chunk);
} }
EXPECT_EQ(nullptr, frame->data.data()); EXPECT_EQ(nullptr, frame->payload.data());
const WebSocketFrameHeader* header = frame->header.get(); const WebSocketFrameHeader* header = frame->header.get();
EXPECT_TRUE(header != nullptr); EXPECT_TRUE(header != nullptr);
if (!header) if (!header)
...@@ -507,7 +512,7 @@ TEST(WebSocketFrameParserTest, FrameTypes) { ...@@ -507,7 +512,7 @@ TEST(WebSocketFrameParserTest, FrameTypes) {
if (!frame) if (!frame)
continue; continue;
EXPECT_TRUE(frame->final_chunk); EXPECT_TRUE(frame->final_chunk);
EXPECT_EQ(nullptr, frame->data.data()); EXPECT_EQ(nullptr, frame->payload.data());
const WebSocketFrameHeader* header = frame->header.get(); const WebSocketFrameHeader* header = frame->header.get();
EXPECT_TRUE(header != nullptr); EXPECT_TRUE(header != nullptr);
if (!header) if (!header)
...@@ -563,7 +568,7 @@ TEST(WebSocketFrameParserTest, FinalBitAndReservedBits) { ...@@ -563,7 +568,7 @@ TEST(WebSocketFrameParserTest, FinalBitAndReservedBits) {
if (!frame) if (!frame)
continue; continue;
EXPECT_TRUE(frame->final_chunk); EXPECT_TRUE(frame->final_chunk);
EXPECT_EQ(nullptr, frame->data.data()); EXPECT_EQ(nullptr, frame->payload.data());
const WebSocketFrameHeader* header = frame->header.get(); const WebSocketFrameHeader* header = frame->header.get();
EXPECT_TRUE(header != nullptr); EXPECT_TRUE(header != nullptr);
if (!header) if (!header)
......
...@@ -838,7 +838,7 @@ TEST_P(WebSocketStreamCreateExtensionTest, PerMessageDeflateInflates) { ...@@ -838,7 +838,7 @@ TEST_P(WebSocketStreamCreateExtensionTest, PerMessageDeflateInflates) {
ASSERT_EQ(1U, frames.size()); ASSERT_EQ(1U, frames.size());
ASSERT_EQ(5U, frames[0]->header.payload_length); ASSERT_EQ(5U, frames[0]->header.payload_length);
EXPECT_EQ(std::string("Hello"), EXPECT_EQ(std::string("Hello"),
std::string(frames[0]->data, frames[0]->header.payload_length)); std::string(frames[0]->payload, frames[0]->header.payload_length));
} }
// Unknown extension in the response is rejected // Unknown extension in the response is rejected
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment