FTP: fix compatibility with VMS server kednos.com

- weird mix of UNIX emulation and VMS output;
  switch completely to VMS-speak

- recognize more errors correctly

- recognize "unknown" file sizes (all asterisks)

BUG=none

Review URL: https://codereview.chromium.org/11570034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@173511 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 66 insertions, 15 deletions

diff --git a/net/ftp/ftp_directory_listing_parser_unittest.cc b/net/ftp/ftp_directory_listing_parser_unittest.cc
index 434cdfa..069acd9 100644
--- a/net/ftp/ftp_directory_listing_parser_unittest.cc
+++ b/net/ftp/ftp_directory_listing_parser_unittest.cc
@@ -151,6 +151,9 @@ const char* kTestFiles[] = {
   "dir-listing-vms-3",
   "dir-listing-vms-4",
   "dir-listing-vms-5",
+  "dir-listing-vms-6",
+  "dir-listing-vms-7",
+  "dir-listing-vms-8",
   "dir-listing-windows-1",
   "dir-listing-windows-2",
 };
diff --git a/net/ftp/ftp_directory_listing_parser_vms.cc b/net/ftp/ftp_directory_listing_parser_vms.cc
index ce940c9..4604415 100644
--- a/net/ftp/ftp_directory_listing_parser_vms.cc
+++ b/net/ftp/ftp_directory_listing_parser_vms.cc
@@ -53,12 +53,20 @@ bool ParseVmsFilename(const string16& raw_filename, string16* parsed_filename,
 }
 
 bool ParseVmsFilesize(const string16& input, int64* size) {
+  if (ContainsOnlyChars(input, ASCIIToUTF16("*"))) {
+    // Response consisting of asterisks means unknown size.
+    *size = -1;
+    return true;
+  }
+
   // VMS's directory listing gives us file size in blocks. We assume that
   // the block size is 512 bytes. It doesn't give accurate file size, but is the
   // best information we have.
   const int kBlockSize = 512;
 
   if (base::StringToInt64(input, size)) {
+    if (*size < 0)
+      return false;
     *size *= kBlockSize;
     return true;
   }
@@ -75,6 +83,8 @@ bool ParseVmsFilesize(const string16& input, int64* size) {
     return false;
   if (blocks_used > blocks_allocated)
     return false;
+  if (blocks_used < 0 || blocks_allocated < 0)
+    return false;
 
   *size = blocks_used * kBlockSize;
   return true;
@@ -121,9 +131,10 @@ bool LooksLikeVmsUserIdentificationCode(const string16& input) {
   return input[0] == '[' && input[input.length() - 1] == ']';
 }
 
-bool LooksLikePermissionDeniedError(const string16& text) {
+bool LooksLikeVMSError(const string16& text) {
   static const char* kPermissionDeniedMessages[] = {
-    "%RMS-E-PRV",
+    "%RMS-E-FNF",  // File not found.
+    "%RMS-E-PRV",  // Access denied.
     "%SYSTEM-F-NOPRIV",
     "privilege",
   };
@@ -188,6 +199,11 @@ bool ParseFtpDirectoryListingVms(
   // seing the header.
   bool seen_header = false;
 
+  // Sometimes the listing doesn't end with a "Total" line, but
+  // it's only okay when it contains some errors (it's needed
+  // to distinguish it from "ls -l" format).
+  bool seen_error = false;
+
   for (size_t i = 0; i < lines.size(); i++) {
     if (lines[i].empty())
       continue;
@@ -206,8 +222,10 @@ bool ParseFtpDirectoryListingVms(
       continue;
     }
 
-    if (LooksLikePermissionDeniedError(lines[i]))
+    if (LooksLikeVMSError(lines[i])) {
+      seen_error = true;
       continue;
+    }
 
     std::vector<string16> columns;
     base::SplitString(CollapseWhitespace(lines[i], false), ' ', &columns);
@@ -217,13 +235,21 @@ bool ParseFtpDirectoryListingVms(
       if (i == lines.size() - 1)
         return false;
 
+      // Skip the next line.
+      i++;
+
+      // This refers to the continuation line.
+      if (LooksLikeVMSError(lines[i])) {
+        seen_error = true;
+        continue;
+      }
+
       // Join the current and next line and split them into columns.
       base::SplitString(
-          CollapseWhitespace(lines[i] + ASCIIToUTF16(" ") + lines[i + 1],
+          CollapseWhitespace(lines[i - 1] + ASCIIToUTF16(" ") + lines[i],
                              false),
           ' ',
           &columns);
-      i++;
     }
 
     FtpDirectoryListingEntry entry;
@@ -248,8 +274,6 @@ bool ParseFtpDirectoryListingVms(
 
     if (!ParseVmsFilesize(columns[1], &entry.size))
       return false;
-    if (entry.size < 0)
-      return false;
     if (entry.type != FtpDirectoryListingEntry::FILE)
       entry.size = -1;
     if (!VmsDateListingToTime(columns, &entry.last_modified))
@@ -259,8 +283,9 @@ bool ParseFtpDirectoryListingVms(
   }
 
   // The only place where we return true is after receiving the "Total" line,
-  // that should be present in every VMS listing.
-  return false;
+  // that should be present in every VMS listing. Alternatively, if the listing
+  // contains error messages, it's OK not to have the "Total" line.
+  return seen_error;
 }
 
 }  // namespace net
diff --git a/net/ftp/ftp_directory_listing_parser_vms_unittest.cc b/net/ftp/ftp_directory_listing_parser_vms_unittest.cc
index 6643e16..a8401a7 100644
--- a/net/ftp/ftp_directory_listing_parser_vms_unittest.cc
+++ b/net/ftp/ftp_directory_listing_parser_vms_unittest.cc
@@ -46,6 +46,9 @@ TEST_F(FtpDirectoryListingParserVmsTest, Good) {
     { "ANNOUNCE.TXT;2 1 12-MAR-2005 08:44:57 [X] (ED,RED,WD,WED)",
       FtpDirectoryListingEntry::FILE, "announce.txt", 512,
       2005, 3, 12, 8, 44 },
+    { "VMS721.ISO;2 ******  6-MAY-2008 09:29 [ANONY,ANONYMOUS] (RE,RWED,RE,RE)",
+      FtpDirectoryListingEntry::FILE, "vms721.iso", -1,
+      2008, 5, 6, 9, 29 },
   };
   for (size_t i = 0; i < arraysize(good_cases); i++) {
     SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]: %s", i,
diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc
index 9b8cb41..2076e9e 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -834,19 +834,26 @@ int FtpNetworkTransaction::ProcessResponseSYST(
       // as unknown.
       if (IsStringASCII(line)) {
         line = StringToLowerASCII(line);
+
+        // Remove all whitespace, to correctly handle cases like fancy "V M S"
+        // response instead of "VMS".
+        RemoveChars(line, kWhitespaceASCII, &line);
+
         // The "magic" strings we test for below have been gathered by an
-        // empirical study.
-        if (line.find("l8") != std::string::npos ||
-            line.find("unix") != std::string::npos ||
-            line.find("bsd") != std::string::npos) {
+        // empirical study. VMS needs to come first because some VMS systems
+        // also respond with "UNIX emulation", which is not perfect. It is much
+        // more reliable to talk to these servers in their native language.
+        if (line.find("vms") != std::string::npos) {
+          system_type_ = SYSTEM_TYPE_VMS;
+        } else if (line.find("l8") != std::string::npos ||
+                   line.find("unix") != std::string::npos ||
+                   line.find("bsd") != std::string::npos) {
           system_type_ = SYSTEM_TYPE_UNIX;
         } else if (line.find("win32") != std::string::npos ||
                    line.find("windows") != std::string::npos) {
           system_type_ = SYSTEM_TYPE_WINDOWS;
         } else if (line.find("os/2") != std::string::npos) {
           system_type_ = SYSTEM_TYPE_OS2;
-        } else if (line.find("vms") != std::string::npos) {
-          system_type_ = SYSTEM_TYPE_VMS;
         }
       }
       next_state_ = STATE_CTRL_WRITE_PWD;
diff --git a/net/ftp/ftp_util.cc b/net/ftp/ftp_util.cc
index febb350..dda9471 100644
--- a/net/ftp/ftp_util.cc
+++ b/net/ftp/ftp_util.cc
@@ -95,6 +95,12 @@ std::string FtpUtil::VMSPathToUnix(const std::string& vms_path) {
   if (vms_path.empty())
     return ".";
 
+  if (vms_path[0] == '/') {
+    // This is not really a VMS path. Most likely the server is emulating UNIX.
+    // Return path as-is.
+    return vms_path;
+  }
+
   if (vms_path == "[]")
     return "/";
 
diff --git a/net/ftp/ftp_util_unittest.cc b/net/ftp/ftp_util_unittest.cc
index fa7213e..0f34bf5 100644
--- a/net/ftp/ftp_util_unittest.cc
+++ b/net/ftp/ftp_util_unittest.cc
@@ -96,6 +96,13 @@ TEST(FtpUtilTest, VMSPathToUnix) {
     { "[.a.b.c]d",   "a/b/c/d"    },
     { "[.a.b.c.d]",  "a/b/c/d"    },
     { "[.",          ""           },
+
+    // UNIX emulation:
+    { "/",           "/"          },
+    { "/a",          "/a"         },
+    { "/a/b",        "/a/b"       },
+    { "/a/b/c",      "/a/b/c"     },
+    { "/a/b/c/d",    "/a/b/c/d"   },
   };
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTestCases); i++) {
     EXPECT_EQ(kTestCases[i].expected_output,