Do not send PASV until RETR or LIST

net/ftp/ftp_network_transaction_unittest.cc

Index: net/ftp/ftp_network_transaction_unittest.cc
diff --git a/net/ftp/ftp_network_transaction_unittest.cc b/net/ftp/ftp_network_transaction_unittest.cc
index 4ee812dff413c43eedbcc068b23d883023c3bbc8..0492850cb96f4af17eaba6dbfea9c7e5ea3d8ef0 100644
--- a/net/ftp/ftp_network_transaction_unittest.cc
+++ b/net/ftp/ftp_network_transaction_unittest.cc
@@ -41,14 +41,12 @@ class FtpSocketDataProvider : public DynamicSocketDataProvider {
     PRE_PWD,
     PRE_TYPE,
     PRE_SIZE,
-    PRE_EPSV,
-    PRE_PASV,
+    PRE_LIST_EPSV,
+    PRE_LIST_PASV,
     PRE_LIST,
     PRE_RETR,
     PRE_RETR_EPSV,
     PRE_RETR_PASV,
-    PRE_CWD_EPSV,
-    PRE_CWD_PASV,
     PRE_CWD,
     PRE_QUIT,
     PRE_NOPASV,
@@ -84,26 +82,19 @@ class FtpSocketDataProvider : public DynamicSocketDataProvider {
                       "257 \"/\" is your current location\r\n");
       case PRE_TYPE:
         return Verify(std::string("TYPE ") + data_type_ + "\r\n", data,
-                      use_epsv_ ? PRE_EPSV : PRE_PASV,
-                      "200 TYPE set successfully\r\n");
-      case PRE_EPSV:
-        return Verify("EPSV\r\n", data, PRE_SIZE,
-                      "227 Entering Extended Passive Mode (|||31744|)\r\n");
-      case PRE_CWD_EPSV:
-        return Verify("EPSV\r\n", data, PRE_CWD,
+                      PRE_SIZE, "200 TYPE set successfully\r\n");
+      case PRE_LIST_EPSV:
+        return Verify("EPSV\r\n", data, PRE_LIST,
                       "227 Entering Extended Passive Mode (|||31744|)\r\n");
+      case PRE_LIST_PASV:
+        return Verify("PASV\r\n", data, PRE_LIST,
+                      "227 Entering Passive Mode 127,0,0,1,123,456\r\n");
       case PRE_RETR_EPSV:
         return Verify("EPSV\r\n", data, PRE_RETR,
                       "227 Entering Extended Passive Mode (|||31744|)\r\n");
-      case PRE_CWD_PASV:
-        return Verify("PASV\r\n", data, PRE_CWD,
-                      "227 Entering Passive Mode 127,0,0,1,123,456\r\n");
       case PRE_RETR_PASV:
         return Verify("PASV\r\n", data, PRE_RETR,
                       "227 Entering Passive Mode 127,0,0,1,123,456\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");
       case PRE_NOPASV:
         // Use unallocated 599 FTP error code to make sure it falls into the
         // generic ERR_FTP_FAILED bucket.
@@ -210,11 +201,11 @@ class FtpSocketDataProviderDirectoryListing : public FtpSocketDataProvider {
       return MockWriteResult(ASYNC, data.length());
     switch (state()) {
       case PRE_SIZE:
-        return Verify("SIZE /\r\n", data,
-                      use_epsv() ? PRE_CWD_EPSV : PRE_CWD_PASV,
+        return Verify("SIZE /\r\n", data, PRE_CWD,
                       "550 I can only retrieve regular files\r\n");
       case PRE_CWD:
-        return Verify("CWD /\r\n", data, PRE_LIST, "200 OK\r\n");
+        return Verify("CWD /\r\n", data,
+                      use_epsv() ? PRE_LIST_EPSV : PRE_LIST_PASV, "200 OK\r\n");

[eroman, 2015/01/26 23:00:38]

Is there a test which covers the circumstances of 450687 ?

[xunjieli, 2015/01/27 14:42:02]

Unfortunately there isn't one. I considered adding one, but I don't think we can
emulate the server behavior, which is no longer relevant. In 450687, we receive
a not-directory error, and we throw away the first data connection and establish
a second data connection by sending a second PASV. Then the server got confused
and closed the second data connection. 
But after this change, there's no longer a second data connection. We establish
the first data connection after we know whether it's a file or directory. We
never attempt to send PASV again. So I think the situation in 450687 can no
longer be simulated in the test. With the removal of the second data connection,
the situation in 450687 is just a normal file retrieval covered in
FtpNetworkTransactionTest.DownloadTransaction.

[davidben, 2015/01/27 19:59:00]

I think we're implicitly testing it by asserting that only one PASV is sent.
(And we were arguably simulating the old situation before by asserting our old
trace. We just didn't account for some server barfing on it.)

       case PRE_LIST:
         return Verify("LIST -l\r\n", data, PRE_QUIT, "200 OK\r\n");
       default:
@@ -236,11 +227,11 @@ class FtpSocketDataProviderDirectoryListingWithPasvFallback
     if (InjectFault())
       return MockWriteResult(ASYNC, data.length());
     switch (state()) {
-      case PRE_EPSV:
-        return Verify("EPSV\r\n", data, PRE_PASV,
+      case PRE_LIST_EPSV:
+        return Verify("EPSV\r\n", data, PRE_LIST_PASV,
                       "500 no EPSV for you\r\n");
       case PRE_SIZE:
-        return Verify("SIZE /\r\n", data, PRE_CWD_PASV,
+        return Verify("SIZE /\r\n", data, PRE_CWD,
                       "550 I can only retrieve regular files\r\n");
       default:
         return FtpSocketDataProviderDirectoryListing::OnWrite(data);
@@ -287,14 +278,15 @@ class FtpSocketDataProviderVMSDirectoryListing : public FtpSocketDataProvider {
       case PRE_PWD:
         return Verify("PWD\r\n", data, PRE_TYPE,
                       "257 \"ANONYMOUS_ROOT:[000000]\"\r\n");
-      case PRE_EPSV:
-        return Verify("EPSV\r\n", data, PRE_PASV, "500 Invalid command\r\n");
+      case PRE_LIST_EPSV:
+        return Verify("EPSV\r\n", data, PRE_LIST_PASV,
+                      "500 Invalid command\r\n");
       case PRE_SIZE:
-        return Verify("SIZE ANONYMOUS_ROOT:[000000]dir\r\n", data, PRE_CWD_PASV,
+        return Verify("SIZE ANONYMOUS_ROOT:[000000]dir\r\n", data, PRE_CWD,
                       "550 I can only retrieve regular files\r\n");
       case PRE_CWD:
-        return Verify("CWD ANONYMOUS_ROOT:[dir]\r\n", data, PRE_LIST,
-                      "200 OK\r\n");
+        return Verify("CWD ANONYMOUS_ROOT:[dir]\r\n", data,
+                      use_epsv() ? PRE_LIST_EPSV : PRE_LIST_PASV, "200 OK\r\n");
       case PRE_LIST:
         return Verify("LIST *.*;0\r\n", data, PRE_QUIT, "200 OK\r\n");
       default:
@@ -321,15 +313,15 @@ class FtpSocketDataProviderVMSDirectoryListingRootDirectory
       case PRE_PWD:
         return Verify("PWD\r\n", data, PRE_TYPE,
                       "257 \"ANONYMOUS_ROOT:[000000]\"\r\n");
-      case PRE_EPSV:
-        return Verify("EPSV\r\n", data, PRE_PASV,
+      case PRE_LIST_EPSV:
+        return Verify("EPSV\r\n", data, PRE_LIST_PASV,
                       "500 EPSV command unknown\r\n");
       case PRE_SIZE:
-        return Verify("SIZE ANONYMOUS_ROOT\r\n", data, PRE_CWD_PASV,
+        return Verify("SIZE ANONYMOUS_ROOT\r\n", data, PRE_CWD,
                       "550 I can only retrieve regular files\r\n");
       case PRE_CWD:
-        return Verify("CWD ANONYMOUS_ROOT:[000000]\r\n", data, PRE_LIST,
-                      "200 OK\r\n");
+        return Verify("CWD ANONYMOUS_ROOT:[000000]\r\n", data,
+                      use_epsv() ? PRE_LIST_EPSV : PRE_LIST_PASV, "200 OK\r\n");
       case PRE_LIST:
         return Verify("LIST *.*;0\r\n", data, PRE_QUIT, "200 OK\r\n");
       default:
@@ -353,8 +345,8 @@ class FtpSocketDataProviderFileDownloadWithFileTypecode
       return MockWriteResult(ASYNC, data.length());
     switch (state()) {
       case PRE_SIZE:
-        return Verify("SIZE /file\r\n", data, PRE_RETR,
-                      "213 18\r\n");
+        return Verify("SIZE /file\r\n", data,
+                      use_epsv() ? PRE_RETR_EPSV : PRE_RETR_PASV, "213 18\r\n");
       case PRE_RETR:
         return Verify("RETR /file\r\n", data, PRE_QUIT, "200 OK\r\n");
       default:
@@ -376,8 +368,7 @@ class FtpSocketDataProviderFileDownload : public FtpSocketDataProvider {
       return MockWriteResult(ASYNC, data.length());
     switch (state()) {
       case PRE_SIZE:
-        return Verify("SIZE /file\r\n", data, PRE_CWD,
-                      "213 18\r\n");
+        return Verify("SIZE /file\r\n", data, PRE_CWD, "213 18\r\n");
       case PRE_CWD:
         return Verify("CWD /file\r\n", data,
                       use_epsv() ? PRE_RETR_EPSV : PRE_RETR_PASV,
@@ -403,8 +394,7 @@ class FtpSocketDataProviderFileNotFound : public FtpSocketDataProvider {
       return MockWriteResult(ASYNC, data.length());
     switch (state()) {
       case PRE_SIZE:
-        return Verify("SIZE /file\r\n", data,
-                      use_epsv() ? PRE_CWD_EPSV : PRE_CWD_PASV,
+        return Verify("SIZE /file\r\n", data, PRE_CWD,
                       "550 File Not Found\r\n");
       case PRE_CWD:
         return Verify("CWD /file\r\n", data,
@@ -432,11 +422,11 @@ class FtpSocketDataProviderFileDownloadWithPasvFallback
     if (InjectFault())
       return MockWriteResult(ASYNC, data.length());
     switch (state()) {
-      case PRE_EPSV:
-        return Verify("EPSV\r\n", data, PRE_PASV,
-                      "500 No can do\r\n");
+      case PRE_RETR_EPSV:
+        return Verify("EPSV\r\n", data, PRE_RETR_PASV, "500 No can do\r\n");
       case PRE_CWD:
-        return Verify("CWD /file\r\n", data, PRE_RETR_PASV,
+        return Verify("CWD /file\r\n", data,
+                      use_epsv() ? PRE_RETR_EPSV : PRE_RETR_PASV,
                       "550 Not a directory\r\n");
       default:
         return FtpSocketDataProviderFileDownload::OnWrite(data);
@@ -510,14 +500,15 @@ class FtpSocketDataProviderVMSFileDownload : public FtpSocketDataProvider {
       case PRE_PWD:
         return Verify("PWD\r\n", data, PRE_TYPE,
                       "257 \"ANONYMOUS_ROOT:[000000]\"\r\n");
-      case PRE_EPSV:
-        return Verify("EPSV\r\n", data, PRE_PASV,
+      case PRE_LIST_EPSV:
+        return Verify("EPSV\r\n", data, PRE_LIST_PASV,
                       "500 EPSV command unknown\r\n");
       case PRE_SIZE:
         return Verify("SIZE ANONYMOUS_ROOT:[000000]file\r\n", data, PRE_CWD,
                       "213 18\r\n");
       case PRE_CWD:
-        return Verify("CWD ANONYMOUS_ROOT:[file]\r\n", data, PRE_RETR_PASV,
+        return Verify("CWD ANONYMOUS_ROOT:[file]\r\n", data,
+                      use_epsv() ? PRE_RETR_EPSV : PRE_RETR_PASV,
                       "550 Not a directory\r\n");
       case PRE_RETR:
         return Verify("RETR ANONYMOUS_ROOT:[000000]file\r\n", data, PRE_QUIT,
@@ -652,7 +643,7 @@ class FtpSocketDataProviderEvilEpsv : public FtpSocketDataProviderFileDownload {
     if (InjectFault())
       return MockWriteResult(ASYNC, data.length());
     switch (state()) {
-      case PRE_EPSV:
+      case PRE_RETR_EPSV:
         return Verify("EPSV\r\n", data, expected_state_,
                       epsv_response_, epsv_response_length_);
       default:
@@ -680,7 +671,7 @@ class FtpSocketDataProviderEvilPasv
     if (InjectFault())
       return MockWriteResult(ASYNC, data.length());
     switch (state()) {
-      case PRE_PASV:
+      case PRE_RETR_PASV:
         return Verify("PASV\r\n", data, expected_state_, pasv_response_);
       default:
         return FtpSocketDataProviderFileDownloadWithPasvFallback::OnWrite(data);
@@ -871,7 +862,7 @@ class FtpNetworkTransactionTest
                              const char* response,
                              int expected_result) {
     ctrl_socket->InjectFailure(state, next_state, response);
-    ExecuteTransaction(ctrl_socket, request, 1, expected_result);
+    ExecuteTransaction(ctrl_socket, request, 0, expected_result);
   }
 
   scoped_ptr<MockHostResolver> host_resolver_;
@@ -917,7 +908,7 @@ TEST_P(FtpNetworkTransactionTest, StripBracketsFromIPv6Literals) {
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransaction) {
   FtpSocketDataProviderDirectoryListing ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host", 0, OK);
 
   EXPECT_TRUE(transaction_.GetResponseInfo()->is_directory_listing);
   EXPECT_EQ(-1, transaction_.GetResponseInfo()->expected_content_size);
@@ -928,7 +919,7 @@ TEST_P(FtpNetworkTransactionTest, DirectoryTransaction) {
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionWithPasvFallback) {
   FtpSocketDataProviderDirectoryListingWithPasvFallback ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host", 0, OK);
 
   EXPECT_TRUE(transaction_.GetResponseInfo()->is_directory_listing);
   EXPECT_EQ(-1, transaction_.GetResponseInfo()->expected_content_size);
@@ -936,7 +927,7 @@ TEST_P(FtpNetworkTransactionTest, DirectoryTransactionWithPasvFallback) {
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionWithTypecode) {
   FtpSocketDataProviderDirectoryListing ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host;type=d", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host;type=d", 0, OK);
 
   EXPECT_TRUE(transaction_.GetResponseInfo()->is_directory_listing);
   EXPECT_EQ(-1, transaction_.GetResponseInfo()->expected_content_size);
@@ -945,19 +936,19 @@ TEST_P(FtpNetworkTransactionTest, DirectoryTransactionWithTypecode) {
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionMultilineWelcome) {
   FtpSocketDataProviderDirectoryListing ctrl_socket;
   ctrl_socket.set_multiline_welcome(true);
-  ExecuteTransaction(&ctrl_socket, "ftp://host", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionShortReads2) {
   FtpSocketDataProviderDirectoryListing ctrl_socket;
   ctrl_socket.set_short_read_limit(2);
-  ExecuteTransaction(&ctrl_socket, "ftp://host", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionShortReads5) {
   FtpSocketDataProviderDirectoryListing ctrl_socket;
   ctrl_socket.set_short_read_limit(5);
-  ExecuteTransaction(&ctrl_socket, "ftp://host", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionMultilineWelcomeShort) {
@@ -967,7 +958,7 @@ TEST_P(FtpNetworkTransactionTest, DirectoryTransactionMultilineWelcomeShort) {
   ctrl_socket.allow_unconsumed_reads(true);
   ctrl_socket.set_multiline_welcome(true);
   ctrl_socket.set_short_read_limit(5);
-  ExecuteTransaction(&ctrl_socket, "ftp://host", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host", 0, OK);
 }
 
 // Regression test for http://crbug.com/60555.
@@ -978,22 +969,22 @@ TEST_P(FtpNetworkTransactionTest, DirectoryTransactionZeroSize) {
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionVMS) {
   FtpSocketDataProviderVMSDirectoryListing ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/dir", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/dir", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionVMSRootDirectory) {
   FtpSocketDataProviderVMSDirectoryListingRootDirectory ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionTransferStarting) {
   FtpSocketDataProviderDirectoryListingTransferStarting ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransaction) {
   FtpSocketDataProviderFileDownload ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 
   // We pass an artificial value of 18 as a response to the SIZE command.
   EXPECT_EQ(18, transaction_.GetResponseInfo()->expected_content_size);
@@ -1004,7 +995,7 @@ TEST_P(FtpNetworkTransactionTest, DownloadTransaction) {
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionWithPasvFallback) {
   FtpSocketDataProviderFileDownloadWithPasvFallback ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 
   // We pass an artificial value of 18 as a response to the SIZE command.
   EXPECT_EQ(18, transaction_.GetResponseInfo()->expected_content_size);
@@ -1030,82 +1021,82 @@ TEST_P(FtpNetworkTransactionTest, DownloadTransactionWithTypecodeI) {
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionMultilineWelcome) {
   FtpSocketDataProviderFileDownload ctrl_socket;
   ctrl_socket.set_multiline_welcome(true);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionShortReads2) {
   FtpSocketDataProviderFileDownload ctrl_socket;
   ctrl_socket.set_short_read_limit(2);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionShortReads5) {
   FtpSocketDataProviderFileDownload ctrl_socket;
   ctrl_socket.set_short_read_limit(5);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionZeroSize) {
   FtpSocketDataProviderFileDownloadZeroSize ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionCWD451) {
   FtpSocketDataProviderFileDownloadCWD451 ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionVMS) {
   FtpSocketDataProviderVMSFileDownload ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionTransferStarting) {
   FtpSocketDataProviderFileDownloadTransferStarting ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionInvalidResponse) {
   FtpSocketDataProviderFileDownloadInvalidResponse ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, ERR_INVALID_RESPONSE);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, ERR_INVALID_RESPONSE);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilPasvReallyBadFormat) {
   FtpSocketDataProviderEvilPasv ctrl_socket("227 Portscan (127,0,0,\r\n",
                                             FtpSocketDataProvider::PRE_QUIT);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, ERR_INVALID_RESPONSE);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, ERR_INVALID_RESPONSE);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilPasvUnsafePort1) {
   FtpSocketDataProviderEvilPasv ctrl_socket("227 Portscan (127,0,0,1,0,22)\r\n",
                                             FtpSocketDataProvider::PRE_QUIT);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, ERR_UNSAFE_PORT);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, ERR_UNSAFE_PORT);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilPasvUnsafePort2) {
   // Still unsafe. 1 * 256 + 2 = 258, which is < 1024.
   FtpSocketDataProviderEvilPasv ctrl_socket("227 Portscan (127,0,0,1,1,2)\r\n",
                                             FtpSocketDataProvider::PRE_QUIT);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, ERR_UNSAFE_PORT);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, ERR_UNSAFE_PORT);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilPasvUnsafePort3) {
   // Still unsafe. 3 * 256 + 4 = 772, which is < 1024.
   FtpSocketDataProviderEvilPasv ctrl_socket("227 Portscan (127,0,0,1,3,4)\r\n",
                                             FtpSocketDataProvider::PRE_QUIT);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, ERR_UNSAFE_PORT);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, ERR_UNSAFE_PORT);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilPasvUnsafePort4) {
   // Unsafe. 8 * 256 + 1 = 2049, which is used by nfs.
   FtpSocketDataProviderEvilPasv ctrl_socket("227 Portscan (127,0,0,1,8,1)\r\n",
                                             FtpSocketDataProvider::PRE_QUIT);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, ERR_UNSAFE_PORT);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, ERR_UNSAFE_PORT);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilPasvUnsafeHost) {
   FtpSocketDataProviderEvilPasv ctrl_socket(
-      "227 Portscan (10,1,2,3,123,456)\r\n", FtpSocketDataProvider::PRE_SIZE);
+      "227 Portscan (10,1,2,3,123,456)\r\n", FtpSocketDataProvider::PRE_RETR);
   ctrl_socket.set_use_epsv(GetFamily() != AF_INET);
   std::string mock_data("mock-data");
   MockRead data_reads[] = {
@@ -1239,8 +1230,8 @@ TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilEpsvWeirdSep) {
     return;
 
   FtpSocketDataProviderEvilEpsv ctrl_socket("227 Portscan ($$$31744$)\r\n",
-                                            FtpSocketDataProvider::PRE_SIZE);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+                                            FtpSocketDataProvider::PRE_RETR);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest,
@@ -1266,22 +1257,22 @@ TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilEpsvIllegalHost) {
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilLoginBadUsername) {
   FtpSocketDataProviderEvilLogin ctrl_socket("hello%0Aworld", "test");
-  ExecuteTransaction(&ctrl_socket, "ftp://hello%0Aworld:test@host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://hello%0Aworld:test@host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionEvilLoginBadPassword) {
   FtpSocketDataProviderEvilLogin ctrl_socket("test", "hello%0Dworld");
-  ExecuteTransaction(&ctrl_socket, "ftp://test:hello%0Dworld@host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://test:hello%0Dworld@host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionSpaceInLogin) {
   FtpSocketDataProviderEvilLogin ctrl_socket("hello world", "test");
-  ExecuteTransaction(&ctrl_socket, "ftp://hello%20world:test@host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://hello%20world:test@host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionSpaceInPassword) {
   FtpSocketDataProviderEvilLogin ctrl_socket("test", "hello world");
-  ExecuteTransaction(&ctrl_socket, "ftp://test:hello%20world@host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://test:hello%20world@host/file", 0, OK);
 }
 
 TEST_P(FtpNetworkTransactionTest, EvilRestartUser) {
@@ -1355,8 +1346,8 @@ TEST_P(FtpNetworkTransactionTest, EvilRestartPassword) {
 
 TEST_P(FtpNetworkTransactionTest, Escaping) {
   FtpSocketDataProviderEscaping ctrl_socket;
-  ExecuteTransaction(&ctrl_socket, "ftp://host/%20%21%22%23%24%25%79%80%81",
-                     1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/%20%21%22%23%24%25%79%80%81", 0,
+                     OK);
 }
 
 // Test for http://crbug.com/23794.
@@ -1374,7 +1365,7 @@ TEST_P(FtpNetworkTransactionTest, DownloadTransactionBigSize) {
   FtpSocketDataProviderEvilSize ctrl_socket(
       "213 3204427776\r\n",
       FtpSocketDataProvider::PRE_CWD);
-  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 1, OK);
+  ExecuteTransaction(&ctrl_socket, "ftp://host/file", 0, OK);
   EXPECT_EQ(3204427776LL,
             transaction_.GetResponseInfo()->expected_content_size);
 }
@@ -1462,12 +1453,9 @@ TEST_P(FtpNetworkTransactionTest, DirectoryTransactionFailEpsv) {
   FtpSocketDataProviderDirectoryListing ctrl_socket;
   // Use unallocated 599 FTP error code to make sure it falls into the generic
   // ERR_FTP_FAILED bucket.
-  TransactionFailHelper(&ctrl_socket,
-                        "ftp://host",
-                        FtpSocketDataProvider::PRE_EPSV,
-                        FtpSocketDataProvider::PRE_NOPASV,
-                        "599 fail\r\n",
-                        ERR_FTP_FAILED);
+  TransactionFailHelper(
+      &ctrl_socket, "ftp://host", FtpSocketDataProvider::PRE_LIST_EPSV,
+      FtpSocketDataProvider::PRE_NOPASV, "599 fail\r\n", ERR_FTP_FAILED);
 }
 
 TEST_P(FtpNetworkTransactionTest, DirectoryTransactionFailCwd) {
@@ -1560,12 +1548,9 @@ TEST_P(FtpNetworkTransactionTest, DownloadTransactionFailEpsv) {
   FtpSocketDataProviderFileDownload ctrl_socket;
   // Use unallocated 599 FTP error code to make sure it falls into the generic
   // ERR_FTP_FAILED bucket.
-  TransactionFailHelper(&ctrl_socket,
-                        "ftp://host/file",
-                        FtpSocketDataProvider::PRE_EPSV,
-                        FtpSocketDataProvider::PRE_NOPASV,
-                        "599 fail\r\n",
-                        ERR_FTP_FAILED);
+  TransactionFailHelper(
+      &ctrl_socket, "ftp://host/file", FtpSocketDataProvider::PRE_RETR_EPSV,
+      FtpSocketDataProvider::PRE_NOPASV, "599 fail\r\n", ERR_FTP_FAILED);
 }
 
 TEST_P(FtpNetworkTransactionTest, DownloadTransactionFailRetr) {