From 8308b1d12eb84a57650f4e6aa6068b7e4864aa9b Mon Sep 17 00:00:00 2001 From: "toyoshim@chromium.org" Date: Wed, 1 Aug 2012 08:31:29 +0000 Subject: Now WebSocketFrameChunk use std::vector to hold payload data. But, SpdyStream and StreamSocket use IOBuffer in their interfaces. To reduce memory copies, we should use IOBuffer to hold data. BUG=none TEST=net_unittests Review URL: https://chromiumcodereview.appspot.com/10796107 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149387 0039d316-1c4b-4281-b951-d872f2087c98 --- net/websockets/websocket_frame.cc | 1 + net/websockets/websocket_frame.h | 8 +- net/websockets/websocket_frame_parser.cc | 38 ++++--- net/websockets/websocket_frame_parser.h | 2 +- net/websockets/websocket_frame_parser_unittest.cc | 119 ++++++++++++++++------ 5 files changed, 121 insertions(+), 47 deletions(-) diff --git a/net/websockets/websocket_frame.cc b/net/websockets/websocket_frame.cc index 25699e8..da65cba 100644 --- a/net/websockets/websocket_frame.cc +++ b/net/websockets/websocket_frame.cc @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/rand_util.h" #include "net/base/big_endian.h" +#include "net/base/io_buffer.h" #include "net/base/net_errors.h" namespace { diff --git a/net/websockets/websocket_frame.h b/net/websockets/websocket_frame.h index bce6fa2..9f7c7fd 100644 --- a/net/websockets/websocket_frame.h +++ b/net/websockets/websocket_frame.h @@ -8,11 +8,14 @@ #include #include "base/basictypes.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "net/base/net_export.h" namespace net { +class IOBufferWithSize; + // Represents a WebSocket frame header. // // Members of this class correspond to each element in WebSocket frame header @@ -73,8 +76,9 @@ struct NET_EXPORT_PRIVATE WebSocketFrameChunk { // Indicates this part is the last chunk of a frame. bool final_chunk; - // |data| is always unmasked even if the frame is masked. - std::vector data; + // |data| is always unmasked even if the frame is masked. |data| might be + // null in the first chunk. + scoped_refptr data; }; // Contains four-byte data representing "masking key" of WebSocket frames. diff --git a/net/websockets/websocket_frame_parser.cc b/net/websockets/websocket_frame_parser.cc index 1acc64c..edbd0e6 100644 --- a/net/websockets/websocket_frame_parser.cc +++ b/net/websockets/websocket_frame_parser.cc @@ -5,6 +5,7 @@ #include "net/websockets/websocket_frame_parser.h" #include +#include #include "base/basictypes.h" #include "base/logging.h" @@ -12,6 +13,7 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" #include "net/base/big_endian.h" +#include "net/base/io_buffer.h" #include "net/websockets/websocket_frame.h" namespace { @@ -120,6 +122,7 @@ void WebSocketFrameParser::DecodeFrameHeader() { bool masked = (second_byte & kMaskBit) != 0; uint64 payload_length = second_byte & kPayloadLengthMask; bool valid_length_format = true; + bool message_too_big = false; if (payload_length == kPayloadLengthWithTwoByteExtendedLengthField) { if (end - current < 2) return; @@ -138,8 +141,10 @@ void WebSocketFrameParser::DecodeFrameHeader() { payload_length > static_cast(kint64max)) { valid_length_format = false; } + if (payload_length > static_cast(kint32max)) + message_too_big = true; } - if (!valid_length_format) { + if (!valid_length_format || message_too_big) { failed_ = true; buffer_.clear(); current_read_pos_ = 0; @@ -178,26 +183,33 @@ scoped_ptr WebSocketFrameParser::DecodeFramePayload( uint64 next_size = std::min( end - current, current_frame_header_->payload_length - frame_offset_); + // This check must pass because |payload_length| is already checked to be + // less than std::numeric_limits::max() when the header is parsed. + DCHECK_LE(next_size, static_cast(kint32max)); scoped_ptr frame_chunk(new WebSocketFrameChunk); if (first_chunk) { frame_chunk->header.reset(new WebSocketFrameHeader(*current_frame_header_)); } frame_chunk->final_chunk = false; - frame_chunk->data.assign(current, current + next_size); - if (current_frame_header_->masked) { - // Unmask the payload. - // TODO(yutak): This could be faster by doing unmasking for each - // machine word (instead of each byte). - size_t key_offset = frame_offset_ % kMaskingKeyLength; - for (uint64 i = 0; i < next_size; ++i) { - frame_chunk->data[i] ^= masking_key_[key_offset]; - key_offset = (key_offset + 1) % kMaskingKeyLength; + if (next_size) { + frame_chunk->data = new IOBufferWithSize(static_cast(next_size)); + char* io_data = frame_chunk->data->data(); + memcpy(io_data, current, next_size); + if (current_frame_header_->masked) { + // Unmask the payload. + // TODO(yutak): This could be faster by doing unmasking for each + // machine word (instead of each byte). + size_t key_offset = frame_offset_ % kMaskingKeyLength; + for (uint64 i = 0; i < next_size; ++i) { + io_data[i] ^= masking_key_[key_offset]; + key_offset = (key_offset + 1) % kMaskingKeyLength; + } } - } - current_read_pos_ += next_size; - frame_offset_ += next_size; + current_read_pos_ += next_size; + frame_offset_ += next_size; + } DCHECK_LE(frame_offset_, current_frame_header_->payload_length); if (frame_offset_ == current_frame_header_->payload_length) { diff --git a/net/websockets/websocket_frame_parser.h b/net/websockets/websocket_frame_parser.h index a43ff84..a6d55ea 100644 --- a/net/websockets/websocket_frame_parser.h +++ b/net/websockets/websocket_frame_parser.h @@ -27,7 +27,7 @@ class NET_EXPORT_PRIVATE WebSocketFrameParser { ~WebSocketFrameParser(); // Decodes the given byte stream and stores parsed WebSocket frames in - // |frames|. + // |frame_chunks|. // // If the parser encounters invalid payload length format, Decode() fails // and returns false. Once Decode() has failed, the parser refuses to decode diff --git a/net/websockets/websocket_frame_parser_unittest.cc b/net/websockets/websocket_frame_parser_unittest.cc index 9ddffbd..7bde1f5 100644 --- a/net/websockets/websocket_frame_parser_unittest.cc +++ b/net/websockets/websocket_frame_parser_unittest.cc @@ -4,11 +4,13 @@ #include "net/websockets/websocket_frame_parser.h" +#include #include #include "base/basictypes.h" #include "base/memory/scoped_vector.h" #include "base/port.h" +#include "net/base/io_buffer.h" #include "net/websockets/websocket_frame.h" #include "testing/gtest/include/gtest/gtest.h" @@ -27,16 +29,23 @@ struct FrameHeaderTestCase { const char* frame_header; size_t frame_header_length; uint64 frame_length; + bool failed; }; +// TODO(toyoshim): Provide error code and check if the reason is correct. const FrameHeaderTestCase kFrameHeaderTests[] = { - { "\x81\x00", 2, GG_UINT64_C(0) }, - { "\x81\x7D", 2, GG_UINT64_C(125) }, - { "\x81\x7E\x00\x7E", 4, GG_UINT64_C(126) }, - { "\x81\x7E\xFF\xFF", 4, GG_UINT64_C(0xFFFF) }, - { "\x81\x7F\x00\x00\x00\x00\x00\x01\x00\x00", 10, GG_UINT64_C(0x10000) }, + { "\x81\x00", 2, GG_UINT64_C(0), false }, + { "\x81\x7D", 2, GG_UINT64_C(125), false }, + { "\x81\x7E\x00\x7E", 4, GG_UINT64_C(126), false }, + { "\x81\x7E\xFF\xFF", 4, GG_UINT64_C(0xFFFF), false }, + { "\x81\x7F\x00\x00\x00\x00\x00\x01\x00\x00", 10, GG_UINT64_C(0x10000), + false }, + { "\x81\x7F\x00\x00\x00\x00\x7F\xFF\xFF\xFF", 10, GG_UINT64_C(0x7FFFFFFF), + false }, + { "\x81\x7F\x00\x00\x00\x00\x80\x00\x00\x00", 10, GG_UINT64_C(0x80000000), + true }, { "\x81\x7F\x7F\xFF\xFF\xFF\xFF\xFF\xFF\xFF", 10, - GG_UINT64_C(0x7FFFFFFFFFFFFFFF) } + GG_UINT64_C(0x7FFFFFFFFFFFFFFF), true } }; const int kNumFrameHeaderTests = arraysize(kFrameHeaderTests); @@ -66,8 +75,8 @@ TEST(WebSocketFrameParserTest, DecodeNormalFrame) { } EXPECT_TRUE(frame->final_chunk); - std::vector expected_data(kHello, kHello + kHelloLength); - EXPECT_EQ(expected_data, frame->data); + ASSERT_EQ(static_cast(kHelloLength), frame->data->size()); + EXPECT_TRUE(std::equal(kHello, kHello + kHelloLength, frame->data->data())); } TEST(WebSocketFrameParserTest, DecodeMaskedFrame) { @@ -93,8 +102,8 @@ TEST(WebSocketFrameParserTest, DecodeMaskedFrame) { } EXPECT_TRUE(frame->final_chunk); - std::vector expected_data(kHello, kHello + kHelloLength); - EXPECT_EQ(expected_data, frame->data); + ASSERT_EQ(static_cast(kHelloLength), frame->data->size()); + EXPECT_TRUE(std::equal(kHello, kHello + kHelloLength, frame->data->data())); } TEST(WebSocketFrameParserTest, DecodeManyFrames) { @@ -143,10 +152,12 @@ TEST(WebSocketFrameParserTest, DecodeManyFrames) { if (!frame) continue; EXPECT_TRUE(frame->final_chunk); - std::vector expected_data( + ASSERT_EQ(kInputs[i].expected_payload_length, + static_cast(frame->data->size())); + EXPECT_TRUE(std::equal( kInputs[i].expected_payload, - kInputs[i].expected_payload + kInputs[i].expected_payload_length); - EXPECT_EQ(expected_data, frame->data); + kInputs[i].expected_payload + kInputs[i].expected_payload_length, + frame->data->data())); const WebSocketFrameHeader* header = frame->header.get(); EXPECT_TRUE(header != NULL); @@ -187,7 +198,14 @@ TEST(WebSocketFrameParserTest, DecodePartialFrame) { if (!frame1) continue; EXPECT_FALSE(frame1->final_chunk); - EXPECT_EQ(expected1, frame1->data); + if (expected1.size() == 0) { + EXPECT_EQ(NULL, frame1->data.get()); + } else { + ASSERT_EQ(cutting_pos, static_cast(frame1->data->size())); + EXPECT_TRUE(std::equal(expected1.begin(), + expected1.end(), + frame1->data->data())); + } const WebSocketFrameHeader* header1 = frame1->header.get(); EXPECT_TRUE(header1 != NULL); if (!header1) @@ -211,7 +229,14 @@ TEST(WebSocketFrameParserTest, DecodePartialFrame) { if (!frame2) continue; EXPECT_TRUE(frame2->final_chunk); - EXPECT_EQ(expected2, frame2->data); + if (expected2.size() == 0) { + EXPECT_EQ(NULL, frame2->data.get()); + } else { + ASSERT_EQ(expected2.size(), static_cast(frame2->data->size())); + EXPECT_TRUE(std::equal(expected2.begin(), + expected2.end(), + frame2->data->data())); + } const WebSocketFrameHeader* header2 = frame2->header.get(); EXPECT_TRUE(header2 == NULL); } @@ -243,7 +268,14 @@ TEST(WebSocketFrameParserTest, DecodePartialMaskedFrame) { if (!frame1) continue; EXPECT_FALSE(frame1->final_chunk); - EXPECT_EQ(expected1, frame1->data); + if (expected1.size() == 0) { + EXPECT_EQ(NULL, frame1->data.get()); + } else { + ASSERT_EQ(expected1.size(), static_cast(frame1->data->size())); + EXPECT_TRUE(std::equal(expected1.begin(), + expected1.end(), + frame1->data->data())); + } const WebSocketFrameHeader* header1 = frame1->header.get(); EXPECT_TRUE(header1 != NULL); if (!header1) @@ -267,7 +299,14 @@ TEST(WebSocketFrameParserTest, DecodePartialMaskedFrame) { if (!frame2) continue; EXPECT_TRUE(frame2->final_chunk); - EXPECT_EQ(expected2, frame2->data); + if (expected2.size() == 0) { + EXPECT_EQ(NULL, frame2->data.get()); + } else { + ASSERT_EQ(expected2.size(), static_cast(frame2->data->size())); + EXPECT_TRUE(std::equal(expected2.begin(), + expected2.end(), + frame2->data->data())); + } const WebSocketFrameHeader* header2 = frame2->header.get(); EXPECT_TRUE(header2 == NULL); } @@ -288,21 +327,36 @@ TEST(WebSocketFrameParserTest, DecodeFramesOfVariousLengths) { WebSocketFrameParser parser; ScopedVector frames; - EXPECT_TRUE(parser.Decode(&input.front(), input.size(), &frames)); - EXPECT_FALSE(parser.failed()); - EXPECT_EQ(1u, frames.size()); + EXPECT_EQ(!kFrameHeaderTests[i].failed, + parser.Decode(&input.front(), input.size(), &frames)); + EXPECT_EQ(kFrameHeaderTests[i].failed, parser.failed()); + if (kFrameHeaderTests[i].failed) { + EXPECT_EQ(0u, frames.size()); + } else { + EXPECT_EQ(1u, frames.size()); + } if (frames.size() != 1u) continue; WebSocketFrameChunk* frame = frames[0]; EXPECT_TRUE(frame != NULL); if (!frame) continue; - if (frame_length == input_payload_size) + if (frame_length == input_payload_size) { EXPECT_TRUE(frame->final_chunk); - else + } else { EXPECT_FALSE(frame->final_chunk); + } std::vector expected_payload(input_payload_size, 'a'); - EXPECT_EQ(expected_payload, frame->data); + if (expected_payload.size() == 0) { + EXPECT_EQ(NULL, frame->data.get()); + } else { + ASSERT_EQ(expected_payload.size(), + static_cast(frame->data->size())); + EXPECT_TRUE(std::equal( + expected_payload.begin(), + expected_payload.end(), + frame->data->data())); + } const WebSocketFrameHeader* header = frame->header.get(); EXPECT_TRUE(header != NULL); if (!header) @@ -328,10 +382,12 @@ TEST(WebSocketFrameParserTest, DecodePartialHeader) { ScopedVector frames; // Feed each byte to the parser to see if the parser behaves correctly // when it receives partial frame header. + size_t last_byte_offset = frame_header_length - 1; for (size_t j = 0; j < frame_header_length; ++j) { - EXPECT_TRUE(parser.Decode(frame_header + j, 1, &frames)); - EXPECT_FALSE(parser.failed()); - if (j == frame_header_length - 1) + bool failed = kFrameHeaderTests[i].failed && j == last_byte_offset; + EXPECT_EQ(!failed, parser.Decode(frame_header + j, 1, &frames)); + EXPECT_EQ(failed, parser.failed()); + if (!kFrameHeaderTests[i].failed && j == last_byte_offset) EXPECT_EQ(1u, frames.size()); else EXPECT_EQ(0u, frames.size()); @@ -342,11 +398,12 @@ TEST(WebSocketFrameParserTest, DecodePartialHeader) { EXPECT_TRUE(frame != NULL); if (!frame) continue; - if (frame_length == 0u) + if (frame_length == 0u) { EXPECT_TRUE(frame->final_chunk); - else + } else { EXPECT_FALSE(frame->final_chunk); - EXPECT_EQ(std::vector(), frame->data); + } + EXPECT_EQ(NULL, frame->data.get()); const WebSocketFrameHeader* header = frame->header.get(); EXPECT_TRUE(header != NULL); if (!header) @@ -444,7 +501,7 @@ TEST(WebSocketFrameParserTest, FrameTypes) { if (!frame) continue; EXPECT_TRUE(frame->final_chunk); - EXPECT_EQ(std::vector(), frame->data); + EXPECT_EQ(NULL, frame->data.get()); const WebSocketFrameHeader* header = frame->header.get(); EXPECT_TRUE(header != NULL); if (!header) @@ -500,7 +557,7 @@ TEST(WebSocketFrameParserTest, FinalBitAndReservedBits) { if (!frame) continue; EXPECT_TRUE(frame->final_chunk); - EXPECT_EQ(std::vector(), frame->data); + EXPECT_EQ(NULL, frame->data.get()); const WebSocketFrameHeader* header = frame->header.get(); EXPECT_TRUE(header != NULL); if (!header) -- cgit v1.1