diff options
author | phajdan.jr@chromium.org <phajdan.jr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-24 19:55:41 +0000 |
---|---|---|
committer | phajdan.jr@chromium.org <phajdan.jr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-24 19:55:41 +0000 |
commit | 630d1b047c8c4ea54eef06fa24a89ad516457875 (patch) | |
tree | 531ebb2fc14ba3ce52b41bb01bb711776f9b1304 /net/ftp | |
parent | 7c79d2454c5a26b9c6723008aecb32fa871edd14 (diff) | |
download | chromium_src-630d1b047c8c4ea54eef06fa24a89ad516457875.zip chromium_src-630d1b047c8c4ea54eef06fa24a89ad516457875.tar.gz chromium_src-630d1b047c8c4ea54eef06fa24a89ad516457875.tar.bz2 |
Respect typecode in the FTP network transaction.
See RFC 1738.
BUG=31819
TEST=net_unittests
Review URL: http://codereview.chromium.org/1170003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42518 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/ftp')
-rw-r--r-- | net/ftp/ftp_network_transaction.cc | 68 | ||||
-rw-r--r-- | net/ftp/ftp_network_transaction.h | 27 | ||||
-rw-r--r-- | net/ftp/ftp_network_transaction_unittest.cc | 69 |
3 files changed, 145 insertions, 19 deletions
diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc index d9d2b88..651340a 100644 --- a/net/ftp/ftp_network_transaction.cc +++ b/net/ftp/ftp_network_transaction.cc @@ -62,7 +62,10 @@ FtpNetworkTransaction::FtpNetworkTransaction( read_data_buf_len_(0), last_error_(OK), system_type_(SYSTEM_TYPE_UNKNOWN), - retr_failed_(false), + // Use image (binary) transfer by default. It should always work, + // whereas the ascii transfer may damage binary data. + data_type_(DATA_TYPE_IMAGE), + resource_type_(RESOURCE_TYPE_UNKNOWN), data_connection_port_(0), socket_factory_(socket_factory), next_state_(STATE_NONE) { @@ -84,6 +87,8 @@ int FtpNetworkTransaction::Start(const FtpRequestInfo* request_info, password_ = L"chrome@example.com"; } + DetectTypecode(); + next_state_ = STATE_CTRL_INIT; int rv = DoLoop(OK); if (rv == ERR_IO_PENDING) @@ -311,7 +316,6 @@ void FtpNetworkTransaction::ResetStateForRestart() { if (write_buf_) write_buf_->SetOffset(0); last_error_ = OK; - retr_failed_ = false; data_connection_port_ = 0; ctrl_socket_.reset(); data_socket_.reset(); @@ -337,8 +341,16 @@ void FtpNetworkTransaction::OnIOComplete(int result) { std::string FtpNetworkTransaction::GetRequestPathForFtpCommand( bool is_directory) const { std::string path(current_remote_directory_); - if (request_->url.has_path()) - path.append(request_->url.path()); + if (request_->url.has_path()) { + std::string gurl_path(request_->url.path()); + + // Get rid of the typecode, see RFC 1738 section 3.2.2. FTP url-path. + std::string::size_type pos = gurl_path.rfind(';'); + if (pos != std::string::npos) + gurl_path.resize(pos); + + path.append(gurl_path); + } // Make sure that if the path is expected to be a file, it won't end // with a trailing slash. if (!is_directory && path.length() > 1 && path[path.length() - 1] == '/') @@ -360,6 +372,27 @@ std::string FtpNetworkTransaction::GetRequestPathForFtpCommand( return path; } +void FtpNetworkTransaction::DetectTypecode() { + if (!request_->url.has_path()) + return; + std::string gurl_path(request_->url.path()); + + // Extract the typecode, see RFC 1738 section 3.2.2. FTP url-path. + std::string::size_type pos = gurl_path.rfind(';'); + if (pos == std::string::npos) + return; + std::string typecode_string(gurl_path.substr(pos)); + if (typecode_string == ";type=a") { + data_type_ = DATA_TYPE_ASCII; + resource_type_ = RESOURCE_TYPE_FILE; + } else if (typecode_string == ";type=i") { + data_type_ = DATA_TYPE_IMAGE; + resource_type_ = RESOURCE_TYPE_FILE; + } else if (typecode_string == ";type=d") { + resource_type_ = RESOURCE_TYPE_DIRECTORY; + } +} + int FtpNetworkTransaction::DoLoop(int result) { DCHECK(next_state_ != STATE_NONE); @@ -760,7 +793,15 @@ int FtpNetworkTransaction::ProcessResponsePWD(const FtpCtrlResponse& response) { // TYPE command. int FtpNetworkTransaction::DoCtrlWriteTYPE() { - std::string command = "TYPE I"; + std::string command = "TYPE "; + if (data_type_ == DATA_TYPE_ASCII) { + command += "A"; + } else if (data_type_ == DATA_TYPE_IMAGE) { + command += "I"; + } else { + NOTREACHED(); + return Stop(ERR_UNEXPECTED); + } next_state_ = STATE_CTRL_READ; return SendFtpCommand(command, COMMAND_TYPE); } @@ -945,10 +986,9 @@ int FtpNetworkTransaction::ProcessResponseRETR( if (response.status_code != 550) return Stop(ERR_FAILED); - DCHECK(!retr_failed_); // Should not get here twice. - retr_failed_ = true; - // It's possible that RETR failed because the path is a directory. + resource_type_ = RESOURCE_TYPE_DIRECTORY; + // We're going to try CWD next, but first send a PASV one more time, // because some FTP servers, including FileZilla, require that. // See http://crbug.com/25316. @@ -1010,11 +1050,11 @@ int FtpNetworkTransaction::ProcessResponseCWD(const FtpCtrlResponse& response) { case ERROR_CLASS_TRANSIENT_ERROR: return Stop(ERR_FAILED); case ERROR_CLASS_PERMANENT_ERROR: - if (retr_failed_ && response.status_code == 550) { - // Both RETR and CWD failed with codes 550. That means that the path - // we're trying to access is not a file, and not a directory. The most - // probable interpretation is that it doesn't exist (with FTP we can't - // be sure). + if (resource_type_ == RESOURCE_TYPE_DIRECTORY && + response.status_code == 550) { + // We're assuming that the resource is a directory, but the server says + // it's not true. The most probable interpretation is that it doesn't + // exist (with FTP we can't be sure). return Stop(ERR_FILE_NOT_FOUND); } return Stop(ERR_FAILED); @@ -1124,7 +1164,7 @@ int FtpNetworkTransaction::DoDataConnect() { int FtpNetworkTransaction::DoDataConnectComplete(int result) { RecordDataConnectionError(result); - if (retr_failed_) { + if (resource_type_ == RESOURCE_TYPE_DIRECTORY) { next_state_ = STATE_CTRL_WRITE_CWD; } else { next_state_ = STATE_CTRL_WRITE_SIZE; diff --git a/net/ftp/ftp_network_transaction.h b/net/ftp/ftp_network_transaction.h index 4fbd204..b74f39a 100644 --- a/net/ftp/ftp_network_transaction.h +++ b/net/ftp/ftp_network_transaction.h @@ -96,6 +96,22 @@ class FtpNetworkTransaction : public FtpTransaction { SYSTEM_TYPE_VMS, }; + // Data representation type, see RFC 959 section 3.1.1. Data Types. + // We only support the two most popular data types. + enum DataType { + DATA_TYPE_ASCII, + DATA_TYPE_IMAGE, + }; + + // In FTP we need to issue different commands depending on whether a resource + // is a file or directory. If we don't know that, we're going to autodetect + // it. + enum ResourceType { + RESOURCE_TYPE_UNKNOWN, + RESOURCE_TYPE_FILE, + RESOURCE_TYPE_DIRECTORY, + }; + // Resets the members of the transaction so it can be restarted. void ResetStateForRestart(); @@ -116,6 +132,9 @@ class FtpNetworkTransaction : public FtpTransaction { // will be used as a directory, |is_directory| should be true. std::string GetRequestPathForFtpCommand(bool is_directory) const; + // See if the request URL contains a typecode and make us respect it. + void DetectTypecode(); + // Runs the state transition loop. int DoLoop(int result); @@ -203,6 +222,12 @@ class FtpNetworkTransaction : public FtpTransaction { SystemType system_type_; + // Data type to be used for the TYPE command. + DataType data_type_; + + // Detected resource type (file or directory). + ResourceType resource_type_; + // We get username and password as wstrings in RestartWithAuth, so they are // also kept as wstrings here. std::wstring username_; @@ -212,8 +237,6 @@ class FtpNetworkTransaction : public FtpTransaction { // with any trailing slash removed. std::string current_remote_directory_; - bool retr_failed_; - int data_connection_port_; ClientSocketFactory* socket_factory_; diff --git a/net/ftp/ftp_network_transaction_unittest.cc b/net/ftp/ftp_network_transaction_unittest.cc index bb9e132..5ed8add 100644 --- a/net/ftp/ftp_network_transaction_unittest.cc +++ b/net/ftp/ftp_network_transaction_unittest.cc @@ -50,7 +50,8 @@ class FtpSocketDataProvider : public DynamicSocketDataProvider { FtpSocketDataProvider() : failure_injection_state_(NONE), - multiline_welcome_(false) { + multiline_welcome_(false), + data_type_('I') { Init(); } @@ -74,8 +75,8 @@ class FtpSocketDataProvider : public DynamicSocketDataProvider { return Verify("PWD\r\n", data, PRE_TYPE, "257 \"/\" is your current location\r\n"); case PRE_TYPE: - return Verify("TYPE I\r\n", data, PRE_PASV, - "200 TYPE is now 8-bit binary\r\n"); + return Verify(std::string("TYPE ") + data_type_ + "\r\n", data, + PRE_PASV, "200 TYPE set successfully\r\n"); case PRE_PASV: return Verify("PASV\r\n", data, PRE_SIZE, "227 Entering Passive Mode 127,0,0,1,123,456\r\n"); @@ -114,6 +115,10 @@ class FtpSocketDataProvider : public DynamicSocketDataProvider { multiline_welcome_ = multiline; } + void set_data_type(char data_type) { + data_type_ = data_type; + } + protected: void Init() { state_ = PRE_USER; @@ -152,6 +157,9 @@ class FtpSocketDataProvider : public DynamicSocketDataProvider { // If true, we will send multiple 230 lines as response after PASS. bool multiline_welcome_; + // Data type to be used for TYPE command. + char data_type_; + DISALLOW_COPY_AND_ASSIGN(FtpSocketDataProvider); }; @@ -191,6 +199,36 @@ class FtpSocketDataProviderDirectoryListing : public FtpSocketDataProvider { DISALLOW_COPY_AND_ASSIGN(FtpSocketDataProviderDirectoryListing); }; +class FtpSocketDataProviderDirectoryListingWithTypecode + : public FtpSocketDataProvider { + public: + FtpSocketDataProviderDirectoryListingWithTypecode() { + } + + virtual MockWriteResult OnWrite(const std::string& data) { + if (InjectFault()) + return MockWriteResult(true, data.length()); + switch (state()) { + case PRE_PASV: + return Verify("PASV\r\n", data, PRE_CWD, + "227 Entering Passive Mode 127,0,0,1,123,456\r\n"); + case PRE_CWD: + return Verify("CWD /\r\n", data, PRE_MLSD, "200 OK\r\n"); + case PRE_MLSD: + return Verify("MLSD\r\n", data, PRE_QUIT, + "150 Accepted data connection\r\n" + "226 MLSD complete\r\n"); + case PRE_LIST: + return Verify("LIST\r\n", data, PRE_QUIT, "200 OK\r\n"); + default: + return FtpSocketDataProvider::OnWrite(data); + } + } + + private: + DISALLOW_COPY_AND_ASSIGN(FtpSocketDataProviderDirectoryListingWithTypecode); +}; + class FtpSocketDataProviderVMSDirectoryListing : public FtpSocketDataProvider { public: FtpSocketDataProviderVMSDirectoryListing() { @@ -647,6 +685,14 @@ TEST_F(FtpNetworkTransactionTest, DirectoryTransaction) { EXPECT_EQ(-1, transaction_.GetResponseInfo()->expected_content_size); } +TEST_F(FtpNetworkTransactionTest, DirectoryTransactionWithTypecode) { + FtpSocketDataProviderDirectoryListingWithTypecode ctrl_socket; + ExecuteTransaction(&ctrl_socket, "ftp://host;type=d", OK); + + EXPECT_TRUE(transaction_.GetResponseInfo()->is_directory_listing); + EXPECT_EQ(-1, transaction_.GetResponseInfo()->expected_content_size); +} + TEST_F(FtpNetworkTransactionTest, DirectoryTransactionMultilineWelcome) { FtpSocketDataProviderDirectoryListing ctrl_socket; ctrl_socket.set_multiline_welcome(true); @@ -698,6 +744,23 @@ TEST_F(FtpNetworkTransactionTest, DownloadTransaction) { EXPECT_EQ(18, transaction_.GetResponseInfo()->expected_content_size); } +TEST_F(FtpNetworkTransactionTest, DownloadTransactionWithTypecodeA) { + FtpSocketDataProviderFileDownload ctrl_socket; + ctrl_socket.set_data_type('A'); + ExecuteTransaction(&ctrl_socket, "ftp://host/file;type=a", OK); + + // We pass an artificial value of 18 as a response to the SIZE command. + EXPECT_EQ(18, transaction_.GetResponseInfo()->expected_content_size); +} + +TEST_F(FtpNetworkTransactionTest, DownloadTransactionWithTypecodeI) { + FtpSocketDataProviderFileDownload ctrl_socket; + ExecuteTransaction(&ctrl_socket, "ftp://host/file;type=i", OK); + + // We pass an artificial value of 18 as a response to the SIZE command. + EXPECT_EQ(18, transaction_.GetResponseInfo()->expected_content_size); +} + TEST_F(FtpNetworkTransactionTest, DownloadTransactionMultilineWelcome) { FtpSocketDataProviderFileDownload ctrl_socket; ctrl_socket.set_multiline_welcome(true); |