summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--net/ftp/ftp_network_transaction.cc18
-rw-r--r--net/ftp/ftp_network_transaction_unittest.cc83
2 files changed, 27 insertions, 74 deletions
diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc
index 5448c34..41f0a66 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -1034,8 +1034,11 @@ int FtpNetworkTransaction::ProcessResponseMLSD(
const FtpCtrlResponse& response) {
switch (GetErrorClass(response.status_code)) {
case ERROR_CLASS_INITIATED:
+ // We want the client to start reading the response at this point.
+ // It got here either through Start or RestartWithAuth. We want that
+ // method to complete. Not setting next state here will make DoLoop exit
+ // and in turn make Start/RestartWithAuth complete.
response_.is_directory_listing = true;
- next_state_ = STATE_CTRL_READ;
break;
case ERROR_CLASS_OK:
response_.is_directory_listing = true;
@@ -1067,8 +1070,11 @@ int FtpNetworkTransaction::ProcessResponseLIST(
const FtpCtrlResponse& response) {
switch (GetErrorClass(response.status_code)) {
case ERROR_CLASS_INITIATED:
+ // We want the client to start reading the response at this point.
+ // It got here either through Start or RestartWithAuth. We want that
+ // method to complete. Not setting next state here will make DoLoop exit
+ // and in turn make Start/RestartWithAuth complete.
response_.is_directory_listing = true;
- next_state_ = STATE_CTRL_READ;
break;
case ERROR_CLASS_OK:
response_.is_directory_listing = true;
@@ -1135,7 +1141,13 @@ int FtpNetworkTransaction::DoDataRead() {
// to be closed on our side too.
data_socket_.reset();
- // No more data so send QUIT Command now and wait for response.
+ if (ctrl_socket_->IsConnected()) {
+ // Wait for the server's response, we should get it before sending QUIT.
+ next_state_ = STATE_CTRL_READ;
+ return OK;
+ }
+
+ // We are no longer connected to the server, so just finish the transaction.
return Stop(OK);
}
diff --git a/net/ftp/ftp_network_transaction_unittest.cc b/net/ftp/ftp_network_transaction_unittest.cc
index c695b87..1c7ca57 100644
--- a/net/ftp/ftp_network_transaction_unittest.cc
+++ b/net/ftp/ftp_network_transaction_unittest.cc
@@ -356,29 +356,6 @@ class FtpSocketDataProviderEscaping : public FtpSocketDataProvider {
DISALLOW_COPY_AND_ASSIGN(FtpSocketDataProviderEscaping);
};
-class FtpSocketDataProviderFileDownloadAcceptedDataConnection
- : public FtpSocketDataProviderFileDownload {
- public:
- FtpSocketDataProviderFileDownloadAcceptedDataConnection() {
- }
-
- virtual MockWriteResult OnWrite(const std::string& data) {
- if (InjectFault())
- return MockWriteResult(true, data.length());
- switch (state()) {
- case PRE_RETR:
- return Verify("RETR /file\r\n", data, PRE_QUIT,
- "150 Accepted Data Connection\r\n");
- default:
- return FtpSocketDataProviderFileDownload::OnWrite(data);
- }
- }
-
- private:
- DISALLOW_COPY_AND_ASSIGN(
- FtpSocketDataProviderFileDownloadAcceptedDataConnection);
-};
-
class FtpSocketDataProviderFileDownloadTransferStarting
: public FtpSocketDataProviderFileDownload {
public:
@@ -596,6 +573,9 @@ class FtpNetworkTransactionTest : public PlatformTest {
int expected_result) {
std::string mock_data("mock-data");
MockRead data_reads[] = {
+ // Usually FTP servers close the data connection after the entire data has
+ // been received.
+ MockRead(false, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
MockRead(mock_data.c_str()),
};
// For compatibility with FileZilla, the transaction code will use two data
@@ -611,7 +591,6 @@ class FtpNetworkTransactionTest : public PlatformTest {
transaction_.Start(&request_info, &callback_, NULL));
EXPECT_NE(LOAD_STATE_IDLE, transaction_.GetLoadState());
ASSERT_EQ(expected_result, callback_.WaitForResult());
- EXPECT_EQ(FtpSocketDataProvider::QUIT, ctrl_socket->state());
if (expected_result == OK) {
scoped_refptr<IOBuffer> io_buffer(new IOBuffer(kBufferSize));
memset(io_buffer->data(), 0, kBufferSize);
@@ -620,7 +599,16 @@ class FtpNetworkTransactionTest : public PlatformTest {
ASSERT_EQ(static_cast<int>(mock_data.length()),
callback_.WaitForResult());
EXPECT_EQ(mock_data, std::string(io_buffer->data(), mock_data.length()));
+
+ // Do another Read to detect that the data socket is now closed.
+ int rv = transaction_.Read(io_buffer.get(), kBufferSize, &callback_);
+ if (rv == ERR_IO_PENDING) {
+ EXPECT_EQ(0, callback_.WaitForResult());
+ } else {
+ EXPECT_EQ(0, rv);
+ }
}
+ EXPECT_EQ(FtpSocketDataProvider::QUIT, ctrl_socket->state());
EXPECT_EQ(LOAD_STATE_IDLE, transaction_.GetLoadState());
}
@@ -733,53 +721,6 @@ TEST_F(FtpNetworkTransactionTest, DownloadTransactionVMS) {
ExecuteTransaction(&ctrl_socket, "ftp://host/file", OK);
}
-TEST_F(FtpNetworkTransactionTest, DownloadTransactionAcceptedDataConnection) {
- FtpSocketDataProviderFileDownloadAcceptedDataConnection ctrl_socket;
- std::string mock_data("mock-data");
- MockRead data_reads[] = {
- MockRead(mock_data.c_str()),
- };
- StaticSocketDataProvider data_socket1(data_reads, arraysize(data_reads),
- NULL, 0);
- mock_socket_factory_.AddSocketDataProvider(&ctrl_socket);
- mock_socket_factory_.AddSocketDataProvider(&data_socket1);
- FtpRequestInfo request_info = GetRequestInfo("ftp://host/file");
-
- // Start the transaction.
- ASSERT_EQ(ERR_IO_PENDING,
- transaction_.Start(&request_info, &callback_, NULL));
- ASSERT_EQ(OK, callback_.WaitForResult());
-
- // The transaction fires the callback when we can start reading data.
- EXPECT_EQ(FtpSocketDataProvider::PRE_QUIT, ctrl_socket.state());
- EXPECT_EQ(LOAD_STATE_SENDING_REQUEST, transaction_.GetLoadState());
- scoped_refptr<IOBuffer> 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(LOAD_STATE_READING_RESPONSE, transaction_.GetLoadState());
- ASSERT_EQ(static_cast<int>(mock_data.length()),
- callback_.WaitForResult());
- EXPECT_EQ(LOAD_STATE_READING_RESPONSE, transaction_.GetLoadState());
- EXPECT_EQ(mock_data, std::string(io_buffer->data(), mock_data.length()));
-
- // FTP server should disconnect the data socket. It is also a signal for the
- // FtpNetworkTransaction that the data transfer is finished.
- ClientSocket* data_socket = mock_socket_factory_.GetMockTCPClientSocket(1);
- ASSERT_TRUE(data_socket);
- data_socket->Disconnect();
-
- // We should issue Reads until one returns EOF...
- ASSERT_EQ(ERR_IO_PENDING,
- transaction_.Read(io_buffer.get(), kBufferSize, &callback_));
-
- // Make sure the transaction finishes cleanly.
- EXPECT_EQ(LOAD_STATE_IDLE, transaction_.GetLoadState());
- ASSERT_EQ(OK, callback_.WaitForResult());
- EXPECT_EQ(FtpSocketDataProvider::QUIT, ctrl_socket.state());
- EXPECT_EQ(LOAD_STATE_IDLE, transaction_.GetLoadState());
-}
-
TEST_F(FtpNetworkTransactionTest, DownloadTransactionTransferStarting) {
FtpSocketDataProviderFileDownloadTransferStarting ctrl_socket;
ExecuteTransaction(&ctrl_socket, "ftp://host/file", OK);