Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(297)

Unified Diff: net/websockets/websocket_frame_parser_unittest.cc

Issue 10824081: Add WebSocketError to indicate decoding failure reason (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: ready for the next review Created 8 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/websockets/websocket_frame_parser_unittest.cc
diff --git a/net/websockets/websocket_frame_parser_unittest.cc b/net/websockets/websocket_frame_parser_unittest.cc
index 7bde1f5ca44bae22c958e9dac2d55e1e7790d7ab..ceac9c2366fd9c13e95fe8cbab1ae322e28ceba4 100644
--- a/net/websockets/websocket_frame_parser_unittest.cc
+++ b/net/websockets/websocket_frame_parser_unittest.cc
@@ -30,35 +30,36 @@ struct FrameHeaderTestCase {
size_t frame_header_length;
uint64 frame_length;
bool failed;
+ net::WebSocketError error_code;
};
-// TODO(toyoshim): Provide error code and check if the reason is correct.
-const FrameHeaderTestCase kFrameHeaderTests[] = {
- { "\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 },
+} // Unnamed namespace
+
+namespace net {
+
+static const FrameHeaderTestCase kFrameHeaderTests[] = {
+ { "\x81\x00", 2, GG_UINT64_C(0), false, WEB_SOCKET_OK },
+ { "\x81\x7D", 2, GG_UINT64_C(125), false, WEB_SOCKET_OK },
+ { "\x81\x7E\x00\x7E", 4, GG_UINT64_C(126), false, WEB_SOCKET_OK },
+ { "\x81\x7E\xFF\xFF", 4, GG_UINT64_C(0xFFFF), false, WEB_SOCKET_OK },
{ "\x81\x7F\x00\x00\x00\x00\x00\x01\x00\x00", 10, GG_UINT64_C(0x10000),
- false },
+ false, WEB_SOCKET_OK },
{ "\x81\x7F\x00\x00\x00\x00\x7F\xFF\xFF\xFF", 10, GG_UINT64_C(0x7FFFFFFF),
- false },
+ false, WEB_SOCKET_OK },
{ "\x81\x7F\x00\x00\x00\x00\x80\x00\x00\x00", 10, GG_UINT64_C(0x80000000),
- true },
+ true, WEB_SOCKET_ERR_MESSAGE_TOO_BIG },
{ "\x81\x7F\x7F\xFF\xFF\xFF\xFF\xFF\xFF\xFF", 10,
- GG_UINT64_C(0x7FFFFFFFFFFFFFFF), true }
+ GG_UINT64_C(0x7FFFFFFFFFFFFFFF), true, WEB_SOCKET_ERR_MESSAGE_TOO_BIG }
};
const int kNumFrameHeaderTests = arraysize(kFrameHeaderTests);
mmenke 2012/08/02 14:08:22 These should still be in the anonymous namespace.
Takashi Toyoshima 2012/08/03 05:53:14 Ah, sorry. I misunderstand your first comment. Do
mmenke 2012/08/03 14:22:02 Yes, that's exactly what I meant. Come to think o
-} // Unnamed namespace
-
-namespace net {
TEST(WebSocketFrameParserTest, DecodeNormalFrame) {
WebSocketFrameParser parser;
ScopedVector<WebSocketFrameChunk> frames;
EXPECT_TRUE(parser.Decode(kHelloFrame, kHelloFrameLength, &frames));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
ASSERT_EQ(1u, frames.size());
WebSocketFrameChunk* frame = frames[0];
ASSERT_TRUE(frame != NULL);
@@ -85,7 +86,7 @@ TEST(WebSocketFrameParserTest, DecodeMaskedFrame) {
ScopedVector<WebSocketFrameChunk> frames;
EXPECT_TRUE(parser.Decode(kMaskedHelloFrame, kMaskedHelloFrameLength,
&frames));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
ASSERT_EQ(1u, frames.size());
WebSocketFrameChunk* frame = frames[0];
ASSERT_TRUE(frame != NULL);
@@ -143,7 +144,7 @@ TEST(WebSocketFrameParserTest, DecodeManyFrames) {
ScopedVector<WebSocketFrameChunk> frames;
EXPECT_TRUE(parser.Decode(&input.front(), input.size(), &frames));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
ASSERT_EQ(static_cast<size_t>(kNumInputs), frames.size());
for (int i = 0; i < kNumInputs; ++i) {
@@ -189,7 +190,7 @@ TEST(WebSocketFrameParserTest, DecodePartialFrame) {
ScopedVector<WebSocketFrameChunk> frames1;
EXPECT_TRUE(parser.Decode(&input1.front(), input1.size(), &frames1));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
EXPECT_EQ(1u, frames1.size());
if (frames1.size() != 1u)
continue;
@@ -220,7 +221,7 @@ TEST(WebSocketFrameParserTest, DecodePartialFrame) {
ScopedVector<WebSocketFrameChunk> frames2;
EXPECT_TRUE(parser.Decode(&input2.front(), input2.size(), &frames2));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
EXPECT_EQ(1u, frames2.size());
if (frames2.size() != 1u)
continue;
@@ -259,7 +260,7 @@ TEST(WebSocketFrameParserTest, DecodePartialMaskedFrame) {
ScopedVector<WebSocketFrameChunk> frames1;
EXPECT_TRUE(parser.Decode(&input1.front(), input1.size(), &frames1));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
EXPECT_EQ(1u, frames1.size());
if (frames1.size() != 1u)
continue;
@@ -290,7 +291,7 @@ TEST(WebSocketFrameParserTest, DecodePartialMaskedFrame) {
ScopedVector<WebSocketFrameChunk> frames2;
EXPECT_TRUE(parser.Decode(&input2.front(), input2.size(), &frames2));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
EXPECT_EQ(1u, frames2.size());
if (frames2.size() != 1u)
continue;
@@ -329,7 +330,7 @@ TEST(WebSocketFrameParserTest, DecodeFramesOfVariousLengths) {
ScopedVector<WebSocketFrameChunk> frames;
EXPECT_EQ(!kFrameHeaderTests[i].failed,
parser.Decode(&input.front(), input.size(), &frames));
- EXPECT_EQ(kFrameHeaderTests[i].failed, parser.failed());
+ EXPECT_EQ(kFrameHeaderTests[i].error_code, parser.websocket_error());
if (kFrameHeaderTests[i].failed) {
EXPECT_EQ(0u, frames.size());
} else {
@@ -386,11 +387,16 @@ TEST(WebSocketFrameParserTest, DecodePartialHeader) {
for (size_t j = 0; j < frame_header_length; ++j) {
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)
+ if (failed) {
+ EXPECT_EQ(kFrameHeaderTests[i].error_code, parser.websocket_error());
+ } else {
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
+ }
+ if (!kFrameHeaderTests[i].failed && j == last_byte_offset) {
EXPECT_EQ(1u, frames.size());
- else
+ } else {
EXPECT_EQ(0u, frames.size());
+ }
}
if (frames.size() != 1u)
continue;
@@ -442,15 +448,15 @@ TEST(WebSocketFrameParserTest, InvalidLengthEncoding) {
WebSocketFrameParser parser;
ScopedVector<WebSocketFrameChunk> frames;
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
EXPECT_FALSE(parser.Decode(frame_header, frame_header_length, &frames));
- EXPECT_TRUE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_ERR_PROTOCOL_ERROR, parser.websocket_error());
EXPECT_EQ(0u, frames.size());
// Once the parser has failed, it no longer accepts any input (even if
// the input is empty).
EXPECT_FALSE(parser.Decode("", 0, &frames));
- EXPECT_TRUE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_ERR_PROTOCOL_ERROR, parser.websocket_error());
EXPECT_EQ(0u, frames.size());
}
}
@@ -492,7 +498,7 @@ TEST(WebSocketFrameParserTest, FrameTypes) {
ScopedVector<WebSocketFrameChunk> frames;
EXPECT_TRUE(parser.Decode(frame_header, frame_header_length, &frames));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
EXPECT_EQ(1u, frames.size());
if (frames.size() != 1u)
continue;
@@ -513,7 +519,7 @@ TEST(WebSocketFrameParserTest, FrameTypes) {
EXPECT_EQ(opcode, header->opcode);
EXPECT_FALSE(header->masked);
EXPECT_EQ(0u, header->payload_length);
- };
+ }
}
TEST(WebSocketFrameParserTest, FinalBitAndReservedBits) {
@@ -548,7 +554,7 @@ TEST(WebSocketFrameParserTest, FinalBitAndReservedBits) {
ScopedVector<WebSocketFrameChunk> frames;
EXPECT_TRUE(parser.Decode(frame_header, frame_header_length, &frames));
- EXPECT_FALSE(parser.failed());
+ EXPECT_EQ(WEB_SOCKET_OK, parser.websocket_error());
EXPECT_EQ(1u, frames.size());
if (frames.size() != 1u)
continue;
« net/websockets/websocket_frame_parser.cc ('K') | « net/websockets/websocket_frame_parser.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698