Treat absolute paths in zip files as relative

Absolute paths in zip files are invalid, as per the APPNOTE:

4.4.17 file name: (Variable)
       4.4.17.1 The name of the file, with optional relative path.
       The path stored MUST not contain a drive or
       device letter, or a leading slash.  All slashes
       MUST be forward slashes '/' as opposed to
       backwards slashes '\' for compatibility with Amiga
       and UNIX file systems etc.  If input came from standard
       input, there is no file name field.

However, zip files in the wild and ones generated by some services do
have them, so they should be handled in a reasonable manner, as other
platform (including Google Drive) do. Stripping the leading slash and
treating the path as relative seems reasonable. This won't work if there
are two files with the same path, but one being relative and the other
absolute. But in this case, the archive was already invalid, so any
attempt to handle it is best effort.

BUG=646850

Change-Id: I7681e888bb78d7f4f53c7bd3a5ab318bffb8c527
Reviewed-on: https://chromium-review.googlesource.com/c/1301094Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603005}

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -260,6 +260,7 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
     ::testing::Values(ZipCase("zipFileOpenDownloads").InGuestMode(),
                       ZipCase("zipFileOpenDownloads"),
                       ZipCase("zipFileOpenDownloadsShiftJIS"),
+                      ZipCase("zipFileOpenDownloadsWithAbsolutePaths"),
                       ZipCase("zipFileOpenDrive").DisableDriveFs(),
                       ZipCase("zipFileOpenDrive").EnableDriveFs(),
                       ZipCase("zipFileOpenUsb"),

chrome/browser/resources/chromeos/zip_archiver/cpp/volume.cc

@@ -351,13 +351,23 @@ void Volume::ReadMetadataCallback(int32_t /*result*/,
       display_name = Cp437ToUtf8(path_name);
     }
 
-    ConstructMetadata(index, display_name.c_str(), path_name, size,
-                      is_directory, modification_time, is_encoded_in_utf8,
-                      &root_metadata);
-
-    index_to_pathname_[index] = path_name;
+    // If the path is absolute, which is technically a violation of the ZIP
+    // spec, strip the leading '/' and treat the path as relative.
+    // TODO(amistry): Handle other cases of ill-formed paths such as "//", ".",
+    // and ".."
+    if (display_name[0] == '/') {
+      display_name = display_name.substr(1);
+    }
 
-    ++index;
+    if (!display_name.empty()) {
+      // Some archives have a "/" entry, which turns into the empty string when
+      // the leading '/' is stripped.
+      ConstructMetadata(index, display_name.c_str(), path_name, size,
+                        is_directory, modification_time, is_encoded_in_utf8,
+                        &root_metadata);
+      index_to_pathname_[index] = path_name;
+      ++index;
+    }
 
     int return_value = volume_archive_->GoToNextFile();
     if (return_value == VolumeArchive::RESULT_FAIL) {

ui/file_manager/integration_tests/file_manager/zip_files.js

@@ -43,6 +43,26 @@ function getUnzippedFileListRowEntriesSjisSubdir() {
   ];
 }
 
+/**
+ * Returns the expected file list row entries after opening (unzipping) the
+ * ENTRIES.zipArchiveWithAbsolutePaths file list entry.
+ */
+function getUnzippedFileListRowEntriesAbsolutePathsRoot() {
+  return [
+    ['foo', '--', 'Folder', 'Oct 11, 2018, 9:44 AM'],
+    ['hello.txt', '13 bytes', 'Plain text', 'Oct 11, 2018, 9:44 AM']
+  ];
+}
+
+/**
+ * Returns the expected file list row entries after opening (unzipping) the
+ * ENTRIES.zipArchiveWithAbsolutePaths file list entry and moving into the
+ * subdirectory.
+ */
+function getUnzippedFileListRowEntriesAbsolutePathsSubdir() {
+  return [['bye.txt', '9 bytes', 'Plain text', 'Oct 11, 2018, 9:44 AM']];
+}
+
 /**
  * Tests zip file open (aka unzip) from Downloads.
  */
@@ -76,6 +96,57 @@ testcase.zipFileOpenDownloads = function() {
   ]);
 };
 
+/**
+ * Tests zip file, with absolute paths, open (aka unzip) from Downloads.
+ */
+testcase.zipFileOpenDownloadsWithAbsolutePaths = function() {
+  let appId;
+
+  StepsRunner.run([
+    // Open Files app on Downloads containing a zip file.
+    function() {
+      setupAndWaitUntilReady(
+          null, RootPath.DOWNLOADS, this.next,
+          [ENTRIES.zipArchiveWithAbsolutePaths], []);
+    },
+    // Select the zip file.
+    function(result) {
+      appId = result.windowId;
+      remoteCall.callRemoteTestUtil('selectFile', appId, ['absolute_paths.zip'])
+          .then(this.next);
+    },
+    // Press the Enter key.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'selectFile failed');
+      const key = ['#file-list', 'Enter', false, false, false];
+      remoteCall.callRemoteTestUtil('fakeKeyDown', appId, key, this.next);
+    },
+    // Check: the zip file content should be shown (unzip).
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeKeyDown failed');
+      const files = getUnzippedFileListRowEntriesAbsolutePathsRoot();
+      remoteCall.waitForFiles(appId, files).then(this.next);
+    },
+    // Select the directory in the ZIP file.
+    function(result) {
+      remoteCall.callRemoteTestUtil('selectFile', appId, ['foo'])
+          .then(this.next);
+    },
+    // Press the Enter key.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'selectFile failed');
+      const key = ['#file-list', 'Enter', false, false, false];
+      remoteCall.callRemoteTestUtil('fakeKeyDown', appId, key, this.next);
+    },
+    // Check: the zip file content should be shown (unzip).
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeKeyDown failed');
+      const files = getUnzippedFileListRowEntriesAbsolutePathsSubdir();
+      remoteCall.waitForFiles(appId, files).then(this.next);
+    },
+  ]);
+};
+
 /**
  * Tests zip file open (aka unzip) from Google Drive.
  */

ui/file_manager/integration_tests/test_util.js

@@ -643,6 +643,17 @@ var ENTRIES = {
     typeText: 'Zip archive'
   }),
 
+  zipArchiveWithAbsolutePaths: new TestEntryInfo({
+    type: EntryType.FILE,
+    sourceFileName: 'absolute_paths.zip',
+    targetPath: 'absolute_paths.zip',
+    mimeType: 'application/x-zip',
+    lastModifiedTime: 'Jan 1, 2014, 1:00 AM',
+    nameText: 'absolute_paths.zip',
+    sizeText: '400 bytes',
+    typeText: 'Zip archive'
+  }),
+
   debPackage: new TestEntryInfo({
     type: EntryType.FILE,
     sourceFileName: 'package.deb',