Avoid signed integer overflow when calculating filesizes for VMS's ftp listings.

net/ftp/ftp_directory_listing_parser_vms_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/ftp/ftp_directory_listing_parser_unittest.h"
 
 #include "base/format_macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/string_split.h"
@@ skipping 58 matching lines @@
 
     std::vector<FtpDirectoryListingEntry> entries;
     EXPECT_TRUE(ParseFtpDirectoryListingVms(lines,
                                             &entries));
     VerifySingleLineTestCase(good_cases[i], entries);
   }
 }
 
 TEST_F(FtpDirectoryListingParserVmsTest, Bad) {
   const char* const bad_cases[] = {
-    "garbage",
+      "garbage",
 
-    // Missing file version number.
-    "README.TXT 2 18-APR-2000 10:40:39",
+      // Missing file version number.
+      "README.TXT 2 18-APR-2000 10:40:39",
 
-    // Missing extension.
-    "README;1 2 18-APR-2000 10:40:39",
+      // Missing extension.
+      "README;1 2 18-APR-2000 10:40:39",
 
-    // Malformed file size.
-    "README.TXT;1 garbage 18-APR-2000 10:40:39",
-    "README.TXT;1 -2 18-APR-2000 10:40:39",
+      // Malformed file size.
+      "README.TXT;1 garbage 18-APR-2000 10:40:39",
+      "README.TXT;1 -2 18-APR-2000 10:40:39",
 
-    // Malformed date.
-    "README.TXT;1 2 APR-2000 10:40:39",
-    "README.TXT;1 2 -18-APR-2000 10:40:39",
-    "README.TXT;1 2 18-APR 10:40:39",
-    "README.TXT;1 2 18-APR-2000 10",
-    "README.TXT;1 2 18-APR-2000 10:40.25",
-    "README.TXT;1 2 18-APR-2000 10:40.25.25",
+      // Malformed date.
+      "README.TXT;1 2 APR-2000 10:40:39",
+      "README.TXT;1 2 -18-APR-2000 10:40:39", "README.TXT;1 2 18-APR 10:40:39",
+      "README.TXT;1 2 18-APR-2000 10", "README.TXT;1 2 18-APR-2000 10:40.25",
+      "README.TXT;1 2 18-APR-2000 10:40.25.25",
 
-    // Malformed security information.
-    "X.TXT;2 1 12-MAR-2005 08:44:57 (RWED,RWED,RE,RE)",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 [SYSTEM]",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 (SYSTEM) (RWED,RWED,RE,RE)",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 [SYSTEM] [RWED,RWED,RE,RE]",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED)",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,RWED,RE,RE,RE)",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,RWEDRWED,RE,RE)",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,DEWR,RE,RE)",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,RWED,Q,RE)",
-    "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,RRWWEEDD,RE,RE)",
+      // Malformed security information.
+      "X.TXT;2 1 12-MAR-2005 08:44:57 (RWED,RWED,RE,RE)",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 [SYSTEM]",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 (SYSTEM) (RWED,RWED,RE,RE)",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 [SYSTEM] [RWED,RWED,RE,RE]",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED)",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,RWED,RE,RE,RE)",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,RWEDRWED,RE,RE)",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,DEWR,RE,RE)",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,RWED,Q,RE)",
+      "X.TXT;2 1 12-MAR-2005 08:44:57 [X] (RWED,RRWWEEDD,RE,RE)",
+
+      // Block size is too large.
+      "README.TXT;1  9223372036854775807  18-APR-2000 10:40:39.90",
+      "README.TXT;1  9223372036854775807/9223372036854775807  18-APR-2000 "

[mmenke, 2017/01/04 18:46:41]

Shouldn't the first number be smaller here, if you want to test something new
and exciting?

[mmenke, 2017/01/04 18:46:41]

I'd also suggest something like 9223372036854775806/9223372036854775806 (only
overflows after multiplication)

[eroman, 2017/01/04 18:55:49]

My current test is INT64_MAX/INT64_MAX.

What additional edge does INT64_MAX-1 / INT64_MAX-1 test ?

[eroman, 2017/01/04 18:55:49]

The first number is the blocks_used, the second is the blocks_allocated.

It is sufficient for blocks_used <= blocks_allocated for the test to exercises
the overflow condition

I confirmed that as written the test triggers the overflow case.

[mmenke, 2017/01/04 19:00:26]

Oops, I misread the code...What I really want is a case where blocks used does
not overflow, and blocks allocated does, since we parse both (I hadn't realized
that x and x/y follow different blocks used overflow paths).

So I suggest an additional test with #/9223372036854775807, where # is less than
9223372036854775807 / 512.

[mmenke, 2017/01/04 19:00:26]

Multiplying by 512 (block size) causes an overflow, rather than parsing as
string.

[mmenke, 2017/01/04 19:01:33]

Rather than parsing the string as an int64, rather

[eroman, 2017/01/04 19:04:31]

I have updated the test description to clarify, and added a new test.

- The existing test was verifying what happens when block_count * 512 overflows
- The new tests verifies when parsing block_count itself overflows

+      "10:40:39.90",
   };
   for (size_t i = 0; i < arraysize(bad_cases); i++) {
     SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]: %s", i, bad_cases[i]));
 
     std::vector<base::string16> lines(GetSingleLineTestCase(bad_cases[i]));
 
     // The parser requires a directory header before accepting regular input.
     lines.insert(lines.begin(),
                  ASCIIToUTF16("Directory ANONYMOUS_ROOT:[000000]"));
 
@@ skipping 66 matching lines @@
 
   lines.push_back(ASCIIToUTF16("   "));
 
   std::vector<FtpDirectoryListingEntry> entries;
   EXPECT_FALSE(ParseFtpDirectoryListingVms(lines, &entries));
 }
 
 }  // namespace
 
 }  // namespace net