Unescape FTP URL paths, Firefox-compatible.

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 e1a4114caf6ac340966f4bc2605381e979460ee1..db84b51cd32275445d317718e0ff9a758525c220 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -7,6 +7,7 @@
 #include "base/compiler_specific.h"
 #include "base/string_util.h"
 #include "net/base/connection_type_histograms.h"
+#include "net/base/escape.h"
 #include "net/base/load_log.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
@@ -28,10 +29,6 @@ namespace {
 
 // Returns true if |input| can be safely used as a part of FTP command.
 bool IsValidFTPCommandString(const std::string& input) {
-  // RFC 959 only allows ASCII strings.

[eroman, 2009/09/18 22:51:23]

Please add a comment in IsValidFTPComment() that we allow non-ASCII, and the
empirical data that led to that decision.

-  if (!IsStringASCII(input))
-    return false;
-
   // Protect agains newline injection attack.
   if (input.find_first_of("\r\n") != std::string::npos)
     return false;
@@ -322,6 +319,15 @@ void FtpNetworkTransaction::OnIOComplete(int result) {
     DoCallback(rv);
 }
 
+std::string FtpNetworkTransaction::GetRequestPathForFtpCommand() const {
+  std::string path = (request_->url.has_path() ? request_->url.path() : "/");
+  UnescapeRule::Type unescape_rules = UnescapeRule::SPACES |
+                                      UnescapeRule::URL_SPECIAL_CHARS;
+  path = UnescapeURLComponent(path, unescape_rules);

[eroman, 2009/09/18 22:51:23]

 Please add a comment that escaping to non-ASCII is OK.

+  DCHECK(IsValidFTPCommandString(path));
+  return path;
+}
+
 int FtpNetworkTransaction::DoLoop(int result) {
   DCHECK(next_state_ != STATE_NONE);
 
@@ -788,11 +794,7 @@ int FtpNetworkTransaction::ProcessResponsePASV(
 
 // SIZE command
 int FtpNetworkTransaction::DoCtrlWriteSIZE() {
-  std::string command = "SIZE";
-  if (request_->url.has_path()) {
-    command.append(" ");
-    command.append(request_->url.path());
-  }
+  std::string command = "SIZE " + GetRequestPathForFtpCommand();
   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, COMMAND_SIZE);
 }
@@ -826,13 +828,7 @@ int FtpNetworkTransaction::ProcessResponseSIZE(
 
 // RETR command
 int FtpNetworkTransaction::DoCtrlWriteRETR() {
-  std::string command = "RETR";
-  if (request_->url.has_path()) {
-    command.append(" ");
-    command.append(request_->url.path());
-  } else {
-    command.append(" /");
-  }
+  std::string command = "RETR " + GetRequestPathForFtpCommand();
   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, COMMAND_RETR);
 }
@@ -879,13 +875,7 @@ int FtpNetworkTransaction::ProcessResponseRETR(
 
 // MDMT command
 int FtpNetworkTransaction::DoCtrlWriteMDTM() {
-  std::string command = "MDTM";
-  if (request_->url.has_path()) {
-    command.append(" ");
-    command.append(request_->url.path());
-  } else {
-    command.append(" /");
-  }
+  std::string command = "MDTM " + GetRequestPathForFtpCommand();
   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, COMMAND_MDTM);
 }
@@ -915,13 +905,7 @@ int FtpNetworkTransaction::ProcessResponseMDTM(
 
 // CWD command
 int FtpNetworkTransaction::DoCtrlWriteCWD() {
-  std::string command = "CWD";
-  if (request_->url.has_path()) {
-    command.append(" ");
-    command.append(request_->url.path());
-  } else {
-    command.append(" /");
-  }
+  std::string command = "CWD " + GetRequestPathForFtpCommand();
   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, COMMAND_CWD);
 }