FTP: Open a fresh data connection after a command error.

net/ftp/ftp_network_transaction.cc

Index: net/ftp/ftp_network_transaction.cc
diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc
index ecbe6cd858138e3653f8fcc4f6b3b0cb2fd67c71..b8374baaff1d256f09a3feb9ef2fc52b06ed9140 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -224,7 +224,8 @@ FtpNetworkTransaction::FtpNetworkTransaction(
       use_epsv_(true),
       data_connection_port_(0),
       socket_factory_(socket_factory),
-      next_state_(STATE_NONE) {
+      next_state_(STATE_NONE),
+      state_after_data_connection_(STATE_CTRL_WRITE_SIZE) {
 }
 
 FtpNetworkTransaction::~FtpNetworkTransaction() {
@@ -346,6 +347,21 @@ void FtpNetworkTransaction::ResetStateForRestart() {
   ctrl_socket_.reset();
   data_socket_.reset();
   next_state_ = STATE_NONE;
+  state_after_data_connection_ = STATE_CTRL_WRITE_SIZE;
+}
+
+void FtpNetworkTransaction::ResetDataConnectionAfterError(State next_state) {
+  // The server _might_ have reset the data connection
+  // (see RFC 959 3.2. ESTABLISHING DATA CONNECTIONS:
+  // "The server MUST close the data connection under the following
+  // conditions:
+  // ...
+  // 5. An irrecoverable error condition occurs.")
+  //
+  // It is ambiguous whan an irrecoverable error condition is,

[eroman, 2012/11/13 20:36:21]

typo: "whan" --> "what" or "when" ?

[Paweł Hajdan Jr., 2012/11/14 17:30:18]

Done.

+  // so we take no chances.
+  state_after_data_connection_ = next_state;
+  next_state_ = use_epsv_ ? STATE_CTRL_WRITE_EPSV : STATE_CTRL_WRITE_PASV;
 }
 
 void FtpNetworkTransaction::DoCallback(int rv) {
@@ -1050,8 +1066,15 @@ int FtpNetworkTransaction::DoCtrlWriteSIZE() {
 
 int FtpNetworkTransaction::ProcessResponseSIZE(
     const FtpCtrlResponse& response) {
+  State state_after_size;
+  if (resource_type_ == RESOURCE_TYPE_FILE)
+    state_after_size = STATE_CTRL_WRITE_RETR;
+  else
+    state_after_size = STATE_CTRL_WRITE_CWD;
+
   switch (GetErrorClass(response.status_code)) {
     case ERROR_CLASS_INITIATED:
+      next_state_ = state_after_size;
       break;
     case ERROR_CLASS_OK:
       if (response.lines.size() != 1)
@@ -1066,10 +1089,14 @@ int FtpNetworkTransaction::ProcessResponseSIZE(
       // Some FTP servers (for example, the qnx one) send a SIZE even for
       // directories.
       response_.expected_content_size = size;
+
+      next_state_ = state_after_size;
       break;
     case ERROR_CLASS_INFO_NEEDED:
+      next_state_ = state_after_size;
       break;
     case ERROR_CLASS_TRANSIENT_ERROR:
+      ResetDataConnectionAfterError(state_after_size);
       break;
     case ERROR_CLASS_PERMANENT_ERROR:
       // It's possible that SIZE failed because the path is a directory.
@@ -1077,17 +1104,14 @@ int FtpNetworkTransaction::ProcessResponseSIZE(
           response.status_code != 550) {
         return Stop(GetNetErrorCodeForFtpResponseCode(response.status_code));
       }
+
+      ResetDataConnectionAfterError(state_after_size);
       break;
     default:
       NOTREACHED();
       return Stop(ERR_UNEXPECTED);
   }
 
-  if (resource_type_ == RESOURCE_TYPE_FILE)
-    next_state_ = STATE_CTRL_WRITE_RETR;
-  else
-    next_state_ = STATE_CTRL_WRITE_CWD;
-
   return OK;
 }
 
@@ -1142,8 +1166,8 @@ int FtpNetworkTransaction::ProcessResponseCWDNotADirectory() {
   // type is. It could still be file, and SIZE could fail because of
   // an access error (http://crbug.com/56734). Try RETR just to be sure.
   resource_type_ = RESOURCE_TYPE_FILE;
-  next_state_ = STATE_CTRL_WRITE_RETR;
 
+  ResetDataConnectionAfterError(STATE_CTRL_WRITE_RETR);
   return OK;
 }
 
@@ -1233,7 +1257,7 @@ int FtpNetworkTransaction::DoDataConnectComplete(int result) {
   if (result != OK)
     return Stop(result);
 
-  next_state_ = STATE_CTRL_WRITE_SIZE;
+  next_state_ = state_after_data_connection_;
   return OK;
 }