Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(373)

Unified Diff: net/ftp/ftp_network_transaction.cc

Issue 903273002: Update from https://crrev.com/315085 (Closed) Base URL: git@github.com:domokit/mojo.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/ftp/ftp_network_transaction.h ('k') | net/ftp/ftp_network_transaction_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/ftp/ftp_network_transaction.cc
diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc
index 2b308c7eed4cddf225b90b270d4554a89bd03f8e..ef6adb8bf759124f1bb14462ded90c72dd1de596 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -239,11 +239,6 @@ int FtpNetworkTransaction::Stop(int error) {
return OK;
}
-int FtpNetworkTransaction::RestartIgnoringLastError(
- const CompletionCallback& callback) {
- return ERR_NOT_IMPLEMENTED;
-}
-
int FtpNetworkTransaction::Start(const FtpRequestInfo* request_info,
const CompletionCallback& callback,
const BoundNetLog& net_log) {
@@ -1033,16 +1028,18 @@ int FtpNetworkTransaction::DoCtrlWriteRETR() {
int FtpNetworkTransaction::ProcessResponseRETR(
const FtpCtrlResponse& response) {
+ // Resource type should be either filled in by DetectTypecode() or
+ // detected with CWD. RETR is sent only when the resource is a file.
+ DCHECK_EQ(RESOURCE_TYPE_FILE, resource_type_);
+
switch (GetErrorClass(response.status_code)) {
case ERROR_CLASS_INITIATED:
// We want the client to start reading the response at this point.
// It got here either through Start or RestartWithAuth. We want that
// method to complete. Not setting next state here will make DoLoop exit
// and in turn make Start/RestartWithAuth complete.
- resource_type_ = RESOURCE_TYPE_FILE;
break;
case ERROR_CLASS_OK:
- resource_type_ = RESOURCE_TYPE_FILE;
next_state_ = STATE_CTRL_WRITE_QUIT;
break;
case ERROR_CLASS_INFO_NEEDED:
@@ -1056,10 +1053,6 @@ int FtpNetworkTransaction::ProcessResponseRETR(
return Stop(ERR_UNEXPECTED);
}
- // We should be sure about our resource type now. Otherwise we risk
- // an infinite loop (RETR can later send CWD, and CWD can later send RETR).
- DCHECK_NE(RESOURCE_TYPE_UNKNOWN, resource_type_);
-
return OK;
}
@@ -1095,6 +1088,7 @@ int FtpNetworkTransaction::ProcessResponseSIZE(
break;
case ERROR_CLASS_PERMANENT_ERROR:
// It's possible that SIZE failed because the path is a directory.
+ // TODO(xunjieli): Add a test for this case.
if (resource_type_ == RESOURCE_TYPE_UNKNOWN &&
response.status_code != 550) {
return Stop(GetNetErrorCodeForFtpResponseCode(response.status_code));
@@ -1104,6 +1098,9 @@ int FtpNetworkTransaction::ProcessResponseSIZE(
NOTREACHED();
return Stop(ERR_UNEXPECTED);
}
+
+ // If the resource is known beforehand to be a file, RETR should be issued,
+ // otherwise do CWD which will detect the resource type.
if (resource_type_ == RESOURCE_TYPE_FILE)
EstablishDataConnection(STATE_CTRL_WRITE_RETR);
else
@@ -1119,13 +1116,14 @@ int FtpNetworkTransaction::DoCtrlWriteCWD() {
}
int FtpNetworkTransaction::ProcessResponseCWD(const FtpCtrlResponse& response) {
- // We should never issue CWD if we know the target resource is a file.
+ // CWD should be invoked only when the resource is not a file.
DCHECK_NE(RESOURCE_TYPE_FILE, resource_type_);
switch (GetErrorClass(response.status_code)) {
case ERROR_CLASS_INITIATED:
return Stop(ERR_INVALID_RESPONSE);
case ERROR_CLASS_OK:
+ resource_type_ = RESOURCE_TYPE_DIRECTORY;
EstablishDataConnection(STATE_CTRL_WRITE_LIST);
break;
case ERROR_CLASS_INFO_NEEDED:
@@ -1158,9 +1156,7 @@ int FtpNetworkTransaction::ProcessResponseCWDNotADirectory() {
return Stop(ERR_FILE_NOT_FOUND);
}
- // We are here because SIZE failed and we are not sure what the resource
- // 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.
+ // If it is not a directory, it is probably a file.
resource_type_ = RESOURCE_TYPE_FILE;
EstablishDataConnection(STATE_CTRL_WRITE_RETR);
@@ -1182,6 +1178,10 @@ int FtpNetworkTransaction::DoCtrlWriteLIST() {
int FtpNetworkTransaction::ProcessResponseLIST(
const FtpCtrlResponse& response) {
+ // Resource type should be either filled in by DetectTypecode() or
+ // detected with CWD. LIST is sent only when the resource is a directory.
+ DCHECK_EQ(RESOURCE_TYPE_DIRECTORY, resource_type_);
+
switch (GetErrorClass(response.status_code)) {
case ERROR_CLASS_INITIATED:
// We want the client to start reading the response at this point.
« no previous file with comments | « net/ftp/ftp_network_transaction.h ('k') | net/ftp/ftp_network_transaction_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698