Correctly handle multiple control responses for RETR command.

net/ftp/ftp_network_transaction.cc

 // Copyright (c) 2008 The Chromium Authors. All rights reserved.  Use of this
 // source code is governed by a BSD-style license that can be found in the
 // LICENSE file.
 
 #include "net/ftp/ftp_network_transaction.h"
 
 #include "base/compiler_specific.h"
 #include "base/string_util.h"
 #include "net/base/connection_type_histograms.h"
 #include "net/base/net_errors.h"
@@ skipping 92 matching lines @@
   return LOAD_STATE_IDLE;
 }
 
 uint64 FtpNetworkTransaction::GetUploadProgress() const {
   return 0;
 }
 
 // Used to prepare and send FTP commad.
 int FtpNetworkTransaction::SendFtpCommand(const std::string& command,
                                           Command cmd) {
+  // If we send a new command when we still have unprocessed responses
+  // for previous commands, the response receiving code will have no way to know
+  // which responses are for which command.
+  DCHECK(!ctrl_response_buffer_.ResponseAvailable());
+
   DCHECK(!write_command_buf_);
   DCHECK(!write_buf_);
   DLOG(INFO) << " >> " << command;
 
   command_sent_ = cmd;
   write_buf_written_ = 0;
   write_command_buf_ = new IOBufferWithSize(command.length() + 2);
   write_buf_ = new ReusedIOBuffer(write_command_buf_,
                                   write_command_buf_->size());
   memcpy(write_command_buf_->data(), command.data(), command.length());
   memcpy(write_command_buf_->data() + command.length(), kCRLF, 2);
 
   next_state_ = STATE_CTRL_WRITE_COMMAND;
   return OK;
 }
 
 int FtpNetworkTransaction::ProcessCtrlResponse() {
   FtpCtrlResponse response = ctrl_response_buffer_.PopResponse();

[wtc, 2009/08/04 23:46:30]

Should this function empty ctrl_response_buffer_ before it
returns an error?  This seems to be required by the new
DCHECK you added to FtpNetworkTransaction::SendFtpCommand.

 
-  // TODO(phajdan.jr): Remove when http://crbug.com/18036 is diagnosed.
-  DLOG(INFO) << "Consumed one control response.";
-
-  // We always expect only one response, even if it's multiline.
-  DCHECK(!ctrl_response_buffer_.ResponseAvailable());
-
   int rv = OK;
   switch (command_sent_) {
     case COMMAND_NONE:
       // TODO(phajdan.jr): Check for errors in the welcome message.
       next_state_ = STATE_CTRL_WRITE_USER;
       break;
     case COMMAND_USER:
       rv = ProcessResponseUSER(response);
       break;
     case COMMAND_PASS:
@@ skipping 29 matching lines @@
     case COMMAND_MDTM:
       rv = ProcessResponseMDTM(response);
       break;
     case COMMAND_QUIT:
       rv = ProcessResponseQUIT(response);
       break;
     default:
       DLOG(INFO) << "Missing Command response handling!";
       return ERR_FAILED;
   }
+
+  // We may get multiple responses for some commands,
+  // see http://crbug.com/18036.
+  while (ctrl_response_buffer_.ResponseAvailable() && rv == OK) {
+    response = ctrl_response_buffer_.PopResponse();
+
+    switch (command_sent_) {
+      case COMMAND_RETR:
+        rv = ProcessResponseRETR(response);
+        break;
+      default:
+        // Multiple responses for other commands are invalid.
+        return ERR_INVALID_RESPONSE;
+    }
+  }
+
   return rv;
 }
 
 void FtpNetworkTransaction::DoCallback(int rv) {
   DCHECK(rv != ERR_IO_PENDING);
   DCHECK(user_callback_);
 
   // Since Run may result in Read being called, clear callback_ up front.
   CompletionCallback* c = user_callback_;
   user_callback_ = NULL;
@@ skipping 521 matching lines @@
     command.append(" /");
   }
   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, COMMAND_RETR);
 }
 
 int FtpNetworkTransaction::ProcessResponseRETR(
     const FtpCtrlResponse& response) {
   switch (GetErrorClass(response.status_code)) {
     case ERROR_CLASS_INITIATED:
+      next_state_ = STATE_CTRL_READ;
       break;
     case ERROR_CLASS_OK:
       next_state_ = STATE_CTRL_WRITE_QUIT;
       break;
     case ERROR_CLASS_PENDING:
       next_state_ = STATE_CTRL_WRITE_PASV;
       break;
     case ERROR_CLASS_ERROR_RETRY:
       if (response.status_code == 421 || response.status_code == 425 ||
           response.status_code == 426)
@@ skipping 171 matching lines @@
   read_data_buf_->data()[0] = 0;
   return data_socket_->Read(read_data_buf_, read_data_buf_len_,
                             &io_callback_);
 }
 
 int FtpNetworkTransaction::DoDataReadComplete(int result) {
   return result;
 }
 
 }  // namespace net