From e0bb61938b9d8a296c2a62d2dd36725c005a30fd Mon Sep 17 00:00:00 2001 From: "phajdan.jr@chromium.org" Date: Mon, 6 Jul 2009 16:49:51 +0000 Subject: Make new FtpNetworkTransaction handle short reads correctly. The problem was that some functions were parsing response lines, but without checking that they have the entire line available. This change will also make it easier to handle multi-line greeting (230 welcome messages). I plan to do that afterwards. TEST=Covered by net_unittests. http://crbug.com/15259 Review URL: http://codereview.chromium.org/149043 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19954 0039d316-1c4b-4281-b951-d872f2087c98 --- net/ftp/ftp_network_transaction_unittest.cc | 101 +++++++++++++++------------- 1 file changed, 55 insertions(+), 46 deletions(-) (limited to 'net/ftp/ftp_network_transaction_unittest.cc') diff --git a/net/ftp/ftp_network_transaction_unittest.cc b/net/ftp/ftp_network_transaction_unittest.cc index d381b0b..51cf77b 100644 --- a/net/ftp/ftp_network_transaction_unittest.cc +++ b/net/ftp/ftp_network_transaction_unittest.cc @@ -235,15 +235,16 @@ class FtpNetworkTransactionTest : public PlatformTest { return info; } - void TransactionFailHelper(FtpMockControlSocket* ctrl_socket, - const char* request, - FtpMockControlSocket::State state, - FtpMockControlSocket::State next_state, - const char* response, - int expected_result) { - ctrl_socket->InjectFailure(state, next_state, response); - StaticMockSocket data_socket1; - StaticMockSocket data_socket2; + void ExecuteTransaction(FtpMockControlSocket* ctrl_socket, + const char* request, + int expected_result) { + std::string mock_data("mock-data"); + MockRead data_reads[] = { + MockRead(mock_data.c_str()), + }; + // TODO(phajdan.jr): FTP transaction should not open two data sockets. + StaticMockSocket data_socket1(data_reads, NULL); + StaticMockSocket data_socket2(data_reads, NULL); mock_socket_factory_.AddMockSocket(ctrl_socket); mock_socket_factory_.AddMockSocket(&data_socket1); mock_socket_factory_.AddMockSocket(&data_socket2); @@ -251,6 +252,25 @@ class FtpNetworkTransactionTest : public PlatformTest { ASSERT_EQ(ERR_IO_PENDING, transaction_.Start(&request_info, &callback_)); EXPECT_EQ(expected_result, callback_.WaitForResult()); EXPECT_EQ(FtpMockControlSocket::QUIT, ctrl_socket->state()); + if (expected_result == OK) { + scoped_refptr io_buffer(new IOBuffer(kBufferSize)); + memset(io_buffer->data(), 0, kBufferSize); + ASSERT_EQ(ERR_IO_PENDING, + transaction_.Read(io_buffer.get(), kBufferSize, &callback_)); + EXPECT_EQ(static_cast(mock_data.length()), + callback_.WaitForResult()); + EXPECT_EQ(mock_data, std::string(io_buffer->data(), mock_data.length())); + } + } + + void TransactionFailHelper(FtpMockControlSocket* ctrl_socket, + const char* request, + FtpMockControlSocket::State state, + FtpMockControlSocket::State next_state, + const char* response, + int expected_result) { + ctrl_socket->InjectFailure(state, next_state, response); + ExecuteTransaction(ctrl_socket, request, expected_result); } scoped_refptr session_; @@ -270,47 +290,36 @@ TEST_F(FtpNetworkTransactionTest, FailedLookup) { TEST_F(FtpNetworkTransactionTest, DirectoryTransaction) { FtpMockControlSocketDirectoryListing ctrl_socket; - std::string test_string("mock-directory-listing"); - MockRead data_reads[] = { - MockRead(test_string.c_str()), - }; - // TODO(phajdan.jr): FTP transaction should not open two data sockets. - StaticMockSocket data_socket1; - StaticMockSocket data_socket2(data_reads, NULL); - mock_socket_factory_.AddMockSocket(&ctrl_socket); - mock_socket_factory_.AddMockSocket(&data_socket1); - mock_socket_factory_.AddMockSocket(&data_socket2); - FtpRequestInfo request_info = GetRequestInfo("ftp://host"); - ASSERT_EQ(ERR_IO_PENDING, transaction_.Start(&request_info, &callback_)); - EXPECT_EQ(OK, callback_.WaitForResult()); - EXPECT_EQ(FtpMockControlSocket::QUIT, ctrl_socket.state()); - scoped_refptr io_buffer(new IOBuffer(kBufferSize)); - memset(io_buffer->data(), 0, kBufferSize); - EXPECT_EQ(ERR_IO_PENDING, - transaction_.Read(io_buffer.get(), kBufferSize, &callback_)); - EXPECT_EQ(static_cast(test_string.length()), callback_.WaitForResult()); - EXPECT_EQ(test_string, std::string(io_buffer->data(), test_string.length())); + ExecuteTransaction(&ctrl_socket, "ftp://host", OK); +} + +TEST_F(FtpNetworkTransactionTest, DirectoryTransactionShortReads2) { + FtpMockControlSocketDirectoryListing ctrl_socket; + ctrl_socket.set_short_read_limit(2); + ExecuteTransaction(&ctrl_socket, "ftp://host", OK); +} + +TEST_F(FtpNetworkTransactionTest, DirectoryTransactionShortReads5) { + FtpMockControlSocketDirectoryListing ctrl_socket; + ctrl_socket.set_short_read_limit(5); + ExecuteTransaction(&ctrl_socket, "ftp://host", OK); } TEST_F(FtpNetworkTransactionTest, DownloadTransaction) { FtpMockControlSocketFileDownload ctrl_socket; - std::string test_string("mock-file-contents"); - MockRead data_reads[] = { - MockRead(test_string.c_str()), - }; - StaticMockSocket data_socket(data_reads, NULL); - mock_socket_factory_.AddMockSocket(&ctrl_socket); - mock_socket_factory_.AddMockSocket(&data_socket); - FtpRequestInfo request_info = GetRequestInfo("ftp://host/file"); - ASSERT_EQ(ERR_IO_PENDING, transaction_.Start(&request_info, &callback_)); - EXPECT_EQ(OK, callback_.WaitForResult()); - EXPECT_EQ(FtpMockControlSocket::QUIT, ctrl_socket.state()); - scoped_refptr io_buffer(new IOBuffer(kBufferSize)); - memset(io_buffer->data(), 0, kBufferSize); - EXPECT_EQ(ERR_IO_PENDING, - transaction_.Read(io_buffer.get(), kBufferSize, &callback_)); - EXPECT_EQ(static_cast(test_string.length()), callback_.WaitForResult()); - EXPECT_EQ(test_string, std::string(io_buffer->data(), test_string.length())); + ExecuteTransaction(&ctrl_socket, "ftp://host/file", OK); +} + +TEST_F(FtpNetworkTransactionTest, DownloadTransactionShortReads2) { + FtpMockControlSocketFileDownload ctrl_socket; + ctrl_socket.set_short_read_limit(2); + ExecuteTransaction(&ctrl_socket, "ftp://host/file", OK); +} + +TEST_F(FtpNetworkTransactionTest, DownloadTransactionShortReads5) { + FtpMockControlSocketFileDownload ctrl_socket; + ctrl_socket.set_short_read_limit(5); + ExecuteTransaction(&ctrl_socket, "ftp://host/file", OK); } TEST_F(FtpNetworkTransactionTest, DirectoryTransactionFailUser) { -- cgit v1.1