Skip to content

Commit

Permalink
Fix WebSocketChannel for NULL payloads.
Browse files Browse the repository at this point in the history 
WebSocketChannel did not handle NULL payloads correctly. Add test cases
for NULL payloads, and fix the bugs.

BUG=316528
TEST=net_unittests --gtest_filter=WebSocketChannel*

Review URL: https://codereview.chromium.org/66543003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@234503 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
ricea@chromium.org committed Nov 12, 2013
1 parent ee7df8f commit 00f4daf
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 5 deletions.
9 changes: 4 additions & 5 deletions net/websockets/websocket_channel.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ ChannelState WebSocketChannel::HandleFrame(
// TODO(ricea): Need to fail the connection if UTF-8 is invalid
// post-reassembly. Requires a streaming UTF-8 validator.
// TODO(ricea): Can this copy be eliminated?
const char* const data_begin = data_buffer->data();
const char* const data_begin = size ? data_buffer->data() : NULL;
const char* const data_end = data_begin + size;
const std::vector<char> data(data_begin, data_end);
// TODO(ricea): Handle the case when ReadFrames returns far
Expand Down Expand Up @@ -668,18 +668,17 @@ void WebSocketChannel::ParseClose(const scoped_refptr<IOBuffer>& buffer,
size_t size,
uint16* code,
std::string* reason) {
const char* data = buffer->data();
reason->clear();
if (size < kWebSocketCloseCodeLength) {
*code = kWebSocketErrorNoStatusReceived;
if (size != 0) {
VLOG(1) << "Close frame with payload size " << size << " received "
<< "(the first byte is " << std::hex << static_cast<int>(data[0])
<< ")";
return;
<< "(the first byte is " << std::hex
<< static_cast<int>(buffer->data()[0]) << ")";
}
return;
}
const char* data = buffer->data();
uint16 unchecked_code = 0;
ReadBigEndian(data, &unchecked_code);
COMPILE_ASSERT(sizeof(unchecked_code) == kWebSocketCloseCodeLength,
Expand Down
125 changes: 125 additions & 0 deletions net/websockets/websocket_channel_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,20 @@ TEST_F(WebSocketChannelDeletingTest, FailChannelDueToBadControlFrame) {
EXPECT_EQ(NULL, channel_.get());
}

// Version of above test with NULL data.
TEST_F(WebSocketChannelDeletingTest, FailChannelDueToBadControlFrameNull) {
scoped_ptr<ReadableFakeWebSocketStream> stream(
new ReadableFakeWebSocketStream);
static const InitFrame frames[] = {
{NOT_FINAL_FRAME, WebSocketFrameHeader::kOpCodePong, NOT_MASKED, NULL}};
stream->PrepareReadFrames(ReadableFakeWebSocketStream::SYNC, OK, frames);
set_stream(stream.Pass());
deleting_ = EVENT_ON_DROP_CHANNEL;

CreateChannelAndConnectSuccessfully();
EXPECT_EQ(NULL, channel_.get());
}

TEST_F(WebSocketChannelDeletingTest, FailChannelDueToPongAfterClose) {
scoped_ptr<ReadableFakeWebSocketStream> stream(
new ReadableFakeWebSocketStream);
Expand All @@ -1077,6 +1091,21 @@ TEST_F(WebSocketChannelDeletingTest, FailChannelDueToPongAfterClose) {
EXPECT_EQ(NULL, channel_.get());
}

TEST_F(WebSocketChannelDeletingTest, FailChannelDueToPongAfterCloseNull) {
scoped_ptr<ReadableFakeWebSocketStream> stream(
new ReadableFakeWebSocketStream);
static const InitFrame frames[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodeClose, NOT_MASKED,
CLOSE_DATA(NORMAL_CLOSURE, "Success")},
{FINAL_FRAME, WebSocketFrameHeader::kOpCodePong, NOT_MASKED, NULL}};
stream->PrepareReadFrames(ReadableFakeWebSocketStream::SYNC, OK, frames);
set_stream(stream.Pass());
deleting_ = EVENT_ON_DROP_CHANNEL;

CreateChannelAndConnectSuccessfully();
EXPECT_EQ(NULL, channel_.get());
}

TEST_F(WebSocketChannelDeletingTest, FailChannelDueToUnknownOpCode) {
scoped_ptr<ReadableFakeWebSocketStream> stream(
new ReadableFakeWebSocketStream);
Expand All @@ -1089,6 +1118,18 @@ TEST_F(WebSocketChannelDeletingTest, FailChannelDueToUnknownOpCode) {
EXPECT_EQ(NULL, channel_.get());
}

TEST_F(WebSocketChannelDeletingTest, FailChannelDueToUnknownOpCodeNull) {
scoped_ptr<ReadableFakeWebSocketStream> stream(
new ReadableFakeWebSocketStream);
static const InitFrame frames[] = {{FINAL_FRAME, 0x7, NOT_MASKED, NULL}};
stream->PrepareReadFrames(ReadableFakeWebSocketStream::SYNC, OK, frames);
set_stream(stream.Pass());
deleting_ = EVENT_ON_DROP_CHANNEL;

CreateChannelAndConnectSuccessfully();
EXPECT_EQ(NULL, channel_.get());
}

TEST_F(WebSocketChannelEventInterfaceTest, ConnectSuccessReported) {
// false means success.
EXPECT_CALL(*event_interface_, OnAddChannelResponse(false, ""));
Expand Down Expand Up @@ -1300,6 +1341,22 @@ TEST_F(WebSocketChannelEventInterfaceTest, FragmentedMessage) {
base::MessageLoop::current()->RunUntilIdle();
}

// A message can consist of one frame with NULL payload.
TEST_F(WebSocketChannelEventInterfaceTest, NullMessage) {
scoped_ptr<ReadableFakeWebSocketStream> stream(
new ReadableFakeWebSocketStream);
static const InitFrame frames[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodeText, NOT_MASKED, NULL}};
stream->PrepareReadFrames(ReadableFakeWebSocketStream::SYNC, OK, frames);
set_stream(stream.Pass());
EXPECT_CALL(*event_interface_, OnAddChannelResponse(false, _));
EXPECT_CALL(*event_interface_, OnFlowControl(_));
EXPECT_CALL(
*event_interface_,
OnDataFrame(true, WebSocketFrameHeader::kOpCodeText, AsVector("")));
CreateChannelAndConnectSuccessfully();
}

// A control frame is not permitted to be split into multiple frames. RFC6455
// 5.5 "All control frames ... MUST NOT be fragmented."
TEST_F(WebSocketChannelEventInterfaceTest, MultiFrameControlMessageIsRejected) {
Expand Down Expand Up @@ -1440,6 +1497,22 @@ TEST_F(WebSocketChannelEventInterfaceTest, ControlFrameInDataMessage) {
base::MessageLoop::current()->RunUntilIdle();
}

// It seems redundant to repeat the entirety of the above test, so just test a
// Pong with NULL data.
TEST_F(WebSocketChannelEventInterfaceTest, PongWithNullData) {
scoped_ptr<ReadableFakeWebSocketStream> stream(
new ReadableFakeWebSocketStream);
static const InitFrame frames[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodePong, NOT_MASKED, NULL}};
stream->PrepareReadFrames(ReadableFakeWebSocketStream::ASYNC, OK, frames);
set_stream(stream.Pass());
EXPECT_CALL(*event_interface_, OnAddChannelResponse(false, _));
EXPECT_CALL(*event_interface_, OnFlowControl(_));

CreateChannelAndConnectSuccessfully();
base::MessageLoop::current()->RunUntilIdle();
}

// If a frame has an invalid header, then the connection is closed and
// subsequent frames must not trigger events.
TEST_F(WebSocketChannelEventInterfaceTest, FrameAfterInvalidFrame) {
Expand Down Expand Up @@ -1624,6 +1697,26 @@ TEST_F(WebSocketChannelEventInterfaceTest, CloseWithNoPayloadGivesStatus1005) {
CreateChannelAndConnectSuccessfully();
}

// A version of the above test with NULL payload.
TEST_F(WebSocketChannelEventInterfaceTest,
CloseWithNullPayloadGivesStatus1005) {
scoped_ptr<ReadableFakeWebSocketStream> stream(
new ReadableFakeWebSocketStream);
static const InitFrame frames[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodeClose, NOT_MASKED, NULL}};
stream->PrepareReadFrames(ReadableFakeWebSocketStream::SYNC, OK, frames);
stream->PrepareReadFramesError(ReadableFakeWebSocketStream::SYNC,
ERR_CONNECTION_CLOSED);
set_stream(stream.Pass());
EXPECT_CALL(*event_interface_, OnAddChannelResponse(false, _));
EXPECT_CALL(*event_interface_, OnFlowControl(_));
EXPECT_CALL(*event_interface_, OnClosingHandshake());
EXPECT_CALL(*event_interface_,
OnDropChannel(kWebSocketErrorNoStatusReceived, _));

CreateChannelAndConnectSuccessfully();
}

// If ReadFrames() returns ERR_WS_PROTOCOL_ERROR, then
// kWebSocketErrorProtocolError must be sent to the renderer.
TEST_F(WebSocketChannelEventInterfaceTest, SyncProtocolErrorGivesStatus1002) {
Expand Down Expand Up @@ -1872,6 +1965,21 @@ TEST_F(WebSocketChannelStreamTest, Code1005IsNotEchoed) {
CreateChannelAndConnectSuccessfully();
}

TEST_F(WebSocketChannelStreamTest, Code1005IsNotEchoedNull) {
static const InitFrame frames[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodeClose, NOT_MASKED, NULL}};
static const InitFrame expected[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodeClose, MASKED, ""}};
EXPECT_CALL(*mock_stream_, GetSubProtocol()).Times(AnyNumber());
EXPECT_CALL(*mock_stream_, ReadFrames(_, _))
.WillOnce(ReturnFrames(&frames))
.WillRepeatedly(Return(ERR_IO_PENDING));
EXPECT_CALL(*mock_stream_, WriteFrames(EqualsFrames(expected), _))
.WillOnce(Return(OK));

CreateChannelAndConnectSuccessfully();
}

// RFC6455 5.5.2 "Upon receipt of a Ping frame, an endpoint MUST send a Pong
// frame in response"
// 5.5.3 "A Pong frame sent in response to a Ping frame must have identical
Expand All @@ -1894,6 +2002,23 @@ TEST_F(WebSocketChannelStreamTest, PingRepliedWithPong) {
CreateChannelAndConnectSuccessfully();
}

// A ping with a NULL payload should be responded to with a Pong with an empty
// payload.
TEST_F(WebSocketChannelStreamTest, NullPingRepliedWithEmptyPong) {
static const InitFrame frames[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodePing, NOT_MASKED, NULL}};
static const InitFrame expected[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodePong, MASKED, ""}};
EXPECT_CALL(*mock_stream_, GetSubProtocol()).Times(AnyNumber());
EXPECT_CALL(*mock_stream_, ReadFrames(_, _))
.WillOnce(ReturnFrames(&frames))
.WillRepeatedly(Return(ERR_IO_PENDING));
EXPECT_CALL(*mock_stream_, WriteFrames(EqualsFrames(expected), _))
.WillOnce(Return(OK));

CreateChannelAndConnectSuccessfully();
}

TEST_F(WebSocketChannelStreamTest, PongInTheMiddleOfDataMessage) {
static const InitFrame frames[] = {
{FINAL_FRAME, WebSocketFrameHeader::kOpCodePing,
Expand Down

0 comments on commit 00f4daf

Please sign in to comment.