Implement RestartWithAuth for NewFtpTransaction.

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 ef896b8f312e41342eb8cd8657bd4ce945f9a431..0ff4eea7c839290bbb23852f988628ea25d57f7e 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -36,11 +36,11 @@ FtpNetworkTransaction::FtpNetworkTransaction(
       request_(NULL),
       resolver_(session->host_resolver()),
       read_ctrl_buf_(new IOBuffer(kCtrlBufLen)),
+      ctrl_response_buffer_(new FtpCtrlResponseBuffer()),
       read_data_buf_len_(0),
       file_data_len_(0),
       write_command_buf_written_(0),
       last_error_(OK),
-      is_anonymous_(false),
       retr_failed_(false),
       data_connection_port_(0),
       socket_factory_(socket_factory),
@@ -56,6 +56,17 @@ int FtpNetworkTransaction::Start(const FtpRequestInfo* request_info,
   load_log_ = load_log;
   request_ = request_info;
 
+  if (request_->url.has_username()) {
+    username_ = UTF8ToWide(request_->url.username());

[wtc, 2009/08/25 20:52:50]

Please ask Darin and Brett whether the embedded username
and password in URLs are ASCII or UTF-8, and whether they
need to be unescaped.

[eroman, 2009/09/01 07:12:37]

They definitely do need to be unescaped.

See HttpNetworkTransaction::GetIdentifyFromUrl(), where we do the unescaping for
use by HTTP auth.

I suggest moving that method and its associated unittest to net_util, and
re-using it in FTP code.

As a side-note, I mis-spelled that function. It should read GetIdentityFromURL()
and not GetIdentifyFromUrl()!

+    if (request_->url.has_password())

[eroman, 2009/09/01 07:12:37]

Don't actually need the guard, will just be empty string if there is none.

+      password_ = UTF8ToWide(request_->url.password());
+    else

[wtc, 2009/08/25 20:52:50]

Do we need the "else" here?  password_ is initialized
to an empty string.

+      password_ = L"";
+  } else {
+    username_ = L"anonymous";
+    password_ = L"chrome@example.com";
+  }
+
   next_state_ = STATE_CTRL_INIT;
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
@@ -75,7 +86,16 @@ int FtpNetworkTransaction::Stop(int error) {
 int FtpNetworkTransaction::RestartWithAuth(const std::wstring& username,
                                            const std::wstring& password,
                                            CompletionCallback* callback) {
-  return ERR_FAILED;
+  ResetStateForRestart();
+
+  username_ = username;
+  password_ = password;
+
+  next_state_ = STATE_CTRL_INIT;
+  int rv = DoLoop(OK);
+  if (rv == ERR_IO_PENDING)
+    user_callback_ = callback;
+  return rv;
 }
 
 int FtpNetworkTransaction::RestartIgnoringLastError(
@@ -134,7 +154,7 @@ int FtpNetworkTransaction::SendFtpCommand(const std::string& command,
   // 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(!ctrl_response_buffer_->ResponseAvailable());
 
   DCHECK(!write_command_buf_);
   DCHECK(!write_buf_);
@@ -176,7 +196,7 @@ FtpNetworkTransaction::ErrorClass FtpNetworkTransaction::GetErrorClass(
 }
 
 int FtpNetworkTransaction::ProcessCtrlResponse() {
-  FtpCtrlResponse response = ctrl_response_buffer_.PopResponse();
+  FtpCtrlResponse response = ctrl_response_buffer_->PopResponse();
 
   int rv = OK;
   switch (command_sent_) {
@@ -230,8 +250,8 @@ int FtpNetworkTransaction::ProcessCtrlResponse() {
 
   // 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();
+  while (ctrl_response_buffer_->ResponseAvailable() && rv == OK) {
+    response = ctrl_response_buffer_->PopResponse();
 
     switch (command_sent_) {
       case COMMAND_RETR:
@@ -246,6 +266,24 @@ int FtpNetworkTransaction::ProcessCtrlResponse() {
   return rv;
 }
 
+void FtpNetworkTransaction::ResetStateForRestart() {
+  command_sent_ = COMMAND_NONE;
+  user_callback_ = NULL;
+  response_ = FtpResponseInfo();
+  read_ctrl_buf_ = new IOBuffer(kCtrlBufLen);
+  ctrl_response_buffer_.reset(new FtpCtrlResponseBuffer());
+  read_data_buf_ = NULL;
+  read_data_buf_len_ = 0;
+  file_data_len_ = 0;
+  write_command_buf_written_ = 0;
+  last_error_ = OK;
+  retr_failed_ = false;
+  data_connection_port_ = 0;
+  ctrl_socket_.reset();
+  data_socket_.reset();
+  next_state_ = STATE_NONE;
+}
+
 void FtpNetworkTransaction::DoCallback(int rv) {
   DCHECK(rv != ERR_IO_PENDING);
   DCHECK(user_callback_);
@@ -443,9 +481,9 @@ int FtpNetworkTransaction::DoCtrlReadComplete(int result) {
   if (result < 0)
     return Stop(result);
 
-  ctrl_response_buffer_.ConsumeData(read_ctrl_buf_->data(), result);
+  ctrl_response_buffer_->ConsumeData(read_ctrl_buf_->data(), result);
 
-  if (!ctrl_response_buffer_.ResponseAvailable()) {
+  if (!ctrl_response_buffer_->ResponseAvailable()) {
     // Read more data from the control socket.
     next_state_ = STATE_CTRL_READ;
     return OK;
@@ -486,14 +524,7 @@ int FtpNetworkTransaction::DoCtrlWriteComplete(int result) {
 
 // USER Command.
 int FtpNetworkTransaction::DoCtrlWriteUSER() {
-  std::string command = "USER";
-  if (request_->url.has_username()) {
-    command.append(" ");
-    command.append(request_->url.username());
-  } else {
-    is_anonymous_ = true;
-    command.append(" anonymous");
-  }
+  std::string command = "USER " + WideToUTF8(username_);

[wtc, 2009/08/25 20:52:50]

Can you add a comment to the header to explain why we need
to convert the username from to a wide string and back to
a narrow string?  Is std::wstring required by the
FtpAuthCache?

   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, COMMAND_USER);
 }
@@ -522,14 +553,7 @@ int FtpNetworkTransaction::ProcessResponseUSER(
 
 // PASS command.
 int FtpNetworkTransaction::DoCtrlWritePASS() {
-  std::string command = "PASS";
-  if (request_->url.has_password()) {
-    command.append(" ");
-    command.append(request_->url.password());
-  } else {
-    command.append(" ");
-    command.append("chrome@example.com");
-  }
+  std::string command = "PASS " + WideToUTF8(password_);

[eroman, 2009/09/01 07:12:37]

PLEASE READ: This looks dangerous!

|username_| and |password_| must be sanitized after extracting them from the
URL, being very careful to strip away CR and LF... since those could be used to
escape the current command and issue other things, like deleting files.

ALSO, note that the RFC for FTP makes no mention of UTF8.
The username and password are expected to be ASCII, so you should do a lossy
conversion here from wide to ASCII :(

   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, COMMAND_PASS);
 }
@@ -552,7 +576,7 @@ int FtpNetworkTransaction::ProcessResponsePASS(
       if (response.status_code == 503) {
         next_state_ = STATE_CTRL_WRITE_USER;
       } else {
-        // TODO(ibrar): Retry here.
+        response_.auth_needed = true;

[eroman, 2009/09/01 07:12:37]

Hm, I dont know about this... will have to look more closely. My first reaction
is there are other error codes not covered by this which relate to login (530,
331, 332).

         return Stop(ERR_FAILED);
       }
       break;