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

Unified Diff: google_apis/gcm/engine/connection_handler_impl_unittest.cc

Issue 643133003: [GCM] Fix crash when size packet splits two socket reads (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Tests/corner cases Created 6 years, 2 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: google_apis/gcm/engine/connection_handler_impl_unittest.cc
diff --git a/google_apis/gcm/engine/connection_handler_impl_unittest.cc b/google_apis/gcm/engine/connection_handler_impl_unittest.cc
index 6b89644462c05ea7a1b49ba913fce5d20e229297..56d163ccd299117924fa6500e3fb0d2739f5e65f 100644
--- a/google_apis/gcm/engine/connection_handler_impl_unittest.cc
+++ b/google_apis/gcm/engine/connection_handler_impl_unittest.cc
@@ -136,8 +136,6 @@ class GCMConnectionHandlerImplTest : public testing::Test {
ReadList mock_reads_;
WriteList mock_writes_;
scoped_ptr<net::DelayedSocketData> data_provider_;
- scoped_ptr<SocketInputStream> socket_input_stream_;
- scoped_ptr<SocketOutputStream> socket_output_stream_;
// The connection handler being tested.
scoped_ptr<ConnectionHandlerImpl> connection_handler_;
@@ -674,8 +672,8 @@ TEST_F(GCMConnectionHandlerImplTest, SendMsgSocketDisconnected) {
}
// Receive a message whose size field was corrupted and is larger than the
-// socket's buffer. Should fail gracefully.
-TEST_F(GCMConnectionHandlerImplTest, CorruptedSize) {
+// socket's buffer. Should fail gracefully with a size error.
+TEST_F(GCMConnectionHandlerImplTest, OutOfBuffer) {
std::string handshake_request = EncodeHandshakeRequest();
WriteList write_list(1, net::MockWrite(net::ASYNC,
handshake_request.c_str(),
@@ -705,5 +703,104 @@ TEST_F(GCMConnectionHandlerImplTest, CorruptedSize) {
EXPECT_EQ(net::ERR_FILE_TOO_BIG, last_error());
}
+// Receive a message whose size field was corrupted and takes more than two
+// bytes to encode. Should fail gracefully with a size error.
+TEST_F(GCMConnectionHandlerImplTest, InvalidSizePacket) {
+ std::string handshake_request = EncodeHandshakeRequest();
+ WriteList write_list(1, net::MockWrite(net::ASYNC,
+ handshake_request.c_str(),
+ handshake_request.size()));
+ std::string handshake_response = EncodeHandshakeResponse();
+
+ // Fill a string with 20000 character zero (which uses more than 2 bytes to
+ // encode the size packet).
+ std::string data_message_proto(20000, '0');
+ std::string data_message_pkt =
+ EncodePacket(kDataMessageStanzaTag, data_message_proto);
+ ReadList read_list;
+ read_list.push_back(net::MockRead(net::ASYNC,
+ handshake_response.c_str(),
+ handshake_response.size()));
+ read_list.push_back(net::MockRead(net::ASYNC,
+ data_message_pkt.c_str(),
+ data_message_pkt.size()));
+ BuildSocket(read_list, write_list);
+
+ ScopedMessage received_message;
+ Connect(&received_message);
+ WaitForMessage(); // The login send.
+ WaitForMessage(); // The login response.
+ received_message.reset();
+ WaitForMessage(); // The data message.
+ EXPECT_FALSE(received_message.get());
+ EXPECT_EQ(net::ERR_FILE_TOO_BIG, last_error());
+}
+
+// Make sure a message with an invalid tag is handled gracefully and resets
+// the connection with an invalid argument error.
+TEST_F(GCMConnectionHandlerImplTest, InvalidTag) {
+ std::string handshake_request = EncodeHandshakeRequest();
+ WriteList write_list(1, net::MockWrite(net::ASYNC,
+ handshake_request.c_str(),
+ handshake_request.size()));
+ std::string handshake_response = EncodeHandshakeResponse();
+
+ std::string invalid_message = "0";
+ std::string invalid_message_pkt =
+ EncodePacket(100, invalid_message);
fgorski 2014/10/20 22:11:55 nit: could you name that value? could be a const i
Nicolas Zea 2014/10/21 00:02:54 Done.
+ ReadList read_list;
+ read_list.push_back(net::MockRead(net::ASYNC,
+ handshake_response.c_str(),
+ handshake_response.size()));
+ read_list.push_back(net::MockRead(net::ASYNC,
+ invalid_message_pkt.c_str(),
+ invalid_message_pkt.size()));
+ BuildSocket(read_list, write_list);
+
+ ScopedMessage received_message;
+ Connect(&received_message);
+ WaitForMessage(); // The login send.
+ WaitForMessage(); // The login response.
+ received_message.reset();
+ WaitForMessage(); // The invalid message.
+ EXPECT_FALSE(received_message.get());
+ EXPECT_EQ(net::ERR_INVALID_ARGUMENT, last_error());
+}
+
+// Receive a message where the size field spans two socket reads.
fgorski 2014/10/20 22:11:55 nit: pleas call out that it is byte 2 and 3 explic
Nicolas Zea 2014/10/21 00:02:54 Done.
+TEST_F(GCMConnectionHandlerImplTest, RecvMsgSplitSize) {
+ std::string handshake_request = EncodeHandshakeRequest();
+ WriteList write_list(1, net::MockWrite(net::ASYNC,
+ handshake_request.c_str(),
+ handshake_request.size()));
+ std::string handshake_response = EncodeHandshakeResponse();
+
+ std::string data_message_proto =
+ BuildDataMessage(kDataMsgFromLong, kDataMsgCategoryLong);
+ std::string data_message_pkt =
+ EncodePacket(kDataMessageStanzaTag, data_message_proto);
+ DCHECK_GT(data_message_pkt.size(), 128U);
+ ReadList read_list;
+ read_list.push_back(net::MockRead(net::ASYNC,
+ handshake_response.c_str(),
+ handshake_response.size()));
+ read_list.push_back(net::MockRead(net::ASYNC,
+ data_message_pkt.c_str(),
+ 2));
+ read_list.push_back(net::MockRead(net::ASYNC,
+ data_message_pkt.c_str() + 2,
+ data_message_pkt.size() - 2));
+ BuildSocket(read_list, write_list);
+
+ ScopedMessage received_message;
+ Connect(&received_message);
+ WaitForMessage(); // The login send.
+ WaitForMessage(); // The login response.
+ WaitForMessage(); // The data message.
+ ASSERT_TRUE(received_message.get());
+ EXPECT_EQ(data_message_proto, received_message->SerializeAsString());
+ EXPECT_EQ(net::OK, last_error());
+}
+
} // namespace
} // namespace gcm

Powered by Google App Engine
This is Rietveld 408576698