DO NOT MERGE: Normalize paths for files in archives.

Always use leading slashes for paths, for simpler logic.

Bug: 31783726
Test: All tests pass.
Change-Id: I41f03880234086b6da0b85e726414c3550c7ce1f
(cherry picked from commit de9ec006ad198e7e793d1666edd1d5d9778c51de)

2 files changed, 69 insertions, 57 deletions

diff --git a/src/com/android/documentsui/archives/Archive.java b/src/com/android/documentsui/archives/Archive.java
index e072b080a..4db6b4a66 100644
--- a/src/com/android/documentsui/archives/Archive.java
+++ b/src/com/android/documentsui/archives/Archive.java
@@ -109,26 +109,29 @@ public class Archive implements Closeable {
 
         // Build the tree structure in memory.
         mTree = new HashMap<String, List<ZipEntry>>();
-        mTree.put("/", new ArrayList<ZipEntry>());
 
         mEntries = new HashMap<String, ZipEntry>();
         ZipEntry entry;
         final List<? extends ZipEntry> entries = Collections.list(mZipFile.entries());
         final Stack<ZipEntry> stack = new Stack<>();
+        String entryPath;
         for (int i = entries.size() - 1; i >= 0; i--) {
             entry = entries.get(i);
             if (entry.isDirectory() != entry.getName().endsWith("/")) {
                 throw new IOException(
                         "Directories must have a trailing slash, and files must not.");
             }
-            if (mEntries.containsKey(entry.getName())) {
+            entryPath = getEntryPath(entry);
+            if (mEntries.containsKey(entryPath)) {
                 throw new IOException("Multiple entries with the same name are not supported.");
             }
-            mEntries.put(entry.getName(), entry);
+            mEntries.put(entryPath, entry);
             if (entry.isDirectory()) {
-                mTree.put(entry.getName(), new ArrayList<ZipEntry>());
+                mTree.put(entryPath, new ArrayList<ZipEntry>());
+            }
+            if (!"/".equals(entryPath)) { // Skip root, as it doesn't have a parent.
+                stack.push(entry);
             }
-            stack.push(entry);
         }
 
         int delimiterIndex;
@@ -136,27 +139,30 @@ public class Archive implements Closeable {
         ZipEntry parentEntry;
         List<ZipEntry> parentList;
 
+        // Go through all directories recursively and build a tree structure.
         while (stack.size() > 0) {
             entry = stack.pop();
 
-            delimiterIndex = entry.getName().lastIndexOf('/', entry.isDirectory()
-                    ? entry.getName().length() - 2 : entry.getName().length() - 1);
-            parentPath =
-                    delimiterIndex != -1 ? entry.getName().substring(0, delimiterIndex) + "/" : "/";
+            entryPath = getEntryPath(entry);
+            delimiterIndex = entryPath.lastIndexOf('/', entry.isDirectory()
+                    ? entryPath.length() - 2 : entryPath.length() - 1);
+            parentPath = entryPath.substring(0, delimiterIndex) + "/";
+
             parentList = mTree.get(parentPath);
 
             if (parentList == null) {
-                parentEntry = mEntries.get(parentPath);
-                if (parentEntry == null) {
-                    // The ZIP file doesn't contain all directories leading to the entry.
-                    // It's rare, but can happen in a valid ZIP archive. In such case create a
-                    // fake ZipEntry and add it on top of the stack to process it next.
-                    parentEntry = new ZipEntry(parentPath);
-                    parentEntry.setSize(0);
-                    parentEntry.setTime(entry.getTime());
-                    mEntries.put(parentPath, parentEntry);
+                // The ZIP file doesn't contain all directories leading to the entry.
+                // It's rare, but can happen in a valid ZIP archive. In such case create a
+                // fake ZipEntry and add it on top of the stack to process it next.
+                parentEntry = new ZipEntry(parentPath);
+                parentEntry.setSize(0);
+                parentEntry.setTime(entry.getTime());
+                mEntries.put(parentPath, parentEntry);
+
+                if (!"/".equals(parentPath)) {
                     stack.push(parentEntry);
                 }
+
                 parentList = new ArrayList<ZipEntry>();
                 mTree.put(parentPath, parentList);
             }
@@ -166,6 +172,19 @@ public class Archive implements Closeable {
     }
 
     /**
+     * Returns a valid, normalized path for an entry.
+     */
+    public static String getEntryPath(ZipEntry entry) {
+        Preconditions.checkArgument(entry.isDirectory() == entry.getName().endsWith("/"),
+                "Ill-formated ZIP-file.");
+        if (entry.getName().startsWith("/")) {
+            return entry.getName();
+        } else {
+            return "/" + entry.getName();
+        }
+    }
+
+    /**
      * Returns true if the file descriptor is seekable.
      * @param descriptor File descriptor to check.
      */
@@ -296,25 +315,18 @@ public class Archive implements Closeable {
             return false;
         }
 
-        // TODO: Include the fake '/' entry in mEntries, and remove this check.
-        // Fake entries are for directories which do not have entries in the ZIP
-        // archive, but are reachable. Eg. /a, /a/b and /a/b/c for a single-entry
-        // archive containing /a/b/c/file.txt.
-        if (parsedParentId.mPath.equals("/")) {
-            return true;
-        }
-
         final ZipEntry parentEntry = mEntries.get(parsedParentId.mPath);
         if (parentEntry == null || !parentEntry.isDirectory()) {
             return false;
         }
 
-        final String parentPath = entry.getName();
-
         // Add a trailing slash even if it's not a directory, so it's easy to check if the
         // entry is a descendant.
-        final String pathWithSlash = entry.isDirectory() ? entry.getName() : entry.getName() + "/";
-        return pathWithSlash.startsWith(parentPath) && !parentPath.equals(pathWithSlash);
+        String pathWithSlash = entry.isDirectory() ? getEntryPath(entry)
+                : getEntryPath(entry) + "/";
+
+        return pathWithSlash.startsWith(parsedParentId.mPath) &&
+                !parsedParentId.mPath.equals(pathWithSlash);
     }
 
     /**
@@ -489,7 +501,7 @@ public class Archive implements Closeable {
 
     private void addCursorRow(MatrixCursor cursor, ZipEntry entry) {
         final MatrixCursor.RowBuilder row = cursor.newRow();
-        final ArchiveId parsedId = new ArchiveId(mArchiveUri, entry.getName());
+        final ArchiveId parsedId = new ArchiveId(mArchiveUri, getEntryPath(entry));
         row.add(Document.COLUMN_DOCUMENT_ID, parsedId.toDocumentId());
 
         final File file = new File(entry.getName());
diff --git a/tests/unit/com/android/documentsui/archives/ArchiveTest.java b/tests/unit/com/android/documentsui/archives/ArchiveTest.java
index 39368d7ec..4ea3a016b 100644
--- a/tests/unit/com/android/documentsui/archives/ArchiveTest.java
+++ b/tests/unit/com/android/documentsui/archives/ArchiveTest.java
@@ -157,7 +157,7 @@ public class ArchiveTest extends AndroidTestCase {
                 new ArchiveId(ARCHIVE_URI, "/").toDocumentId(), null, null);
 
         assertTrue(cursor.moveToFirst());
-        assertEquals(new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId(),
+        assertEquals(new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId(),
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DOCUMENT_ID)));
         assertEquals("dir1",
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DISPLAY_NAME)));
@@ -168,7 +168,7 @@ public class ArchiveTest extends AndroidTestCase {
 
         assertTrue(cursor.moveToNext());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir2/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir2/").toDocumentId(),
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DOCUMENT_ID)));
         assertEquals("dir2",
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DISPLAY_NAME)));
@@ -179,7 +179,7 @@ public class ArchiveTest extends AndroidTestCase {
 
         assertTrue(cursor.moveToNext());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "file1.txt").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/file1.txt").toDocumentId(),
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DOCUMENT_ID)));
         assertEquals("file1.txt",
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DISPLAY_NAME)));
@@ -192,11 +192,11 @@ public class ArchiveTest extends AndroidTestCase {
 
         // Check if querying children works too.
         final Cursor childCursor = mArchive.queryChildDocuments(
-                new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId(), null, null);
+                new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId(), null, null);
 
         assertTrue(childCursor.moveToFirst());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir1/cherries.txt").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/cherries.txt").toDocumentId(),
                 childCursor.getString(childCursor.getColumnIndexOrThrow(
                         Document.COLUMN_DOCUMENT_ID)));
         assertEquals("cherries.txt",
@@ -216,7 +216,7 @@ public class ArchiveTest extends AndroidTestCase {
 
         assertTrue(cursor.moveToFirst());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId(),
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DOCUMENT_ID)));
         assertEquals("dir1",
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DISPLAY_NAME)));
@@ -227,11 +227,11 @@ public class ArchiveTest extends AndroidTestCase {
         assertFalse(cursor.moveToNext());
 
         final Cursor childCursor = mArchive.queryChildDocuments(
-                new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId(), null, null);
+                new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId(), null, null);
 
         assertTrue(childCursor.moveToFirst());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir1/dir2/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/dir2/").toDocumentId(),
                 childCursor.getString(childCursor.getColumnIndexOrThrow(
                         Document.COLUMN_DOCUMENT_ID)));
         assertEquals("dir2",
@@ -245,12 +245,12 @@ public class ArchiveTest extends AndroidTestCase {
         assertFalse(childCursor.moveToNext());
 
         final Cursor childCursor2 = mArchive.queryChildDocuments(
-                new ArchiveId(ARCHIVE_URI, "dir1/dir2/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/dir2/").toDocumentId(),
                 null, null);
 
         assertTrue(childCursor2.moveToFirst());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir1/dir2/cherries.txt").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/dir2/cherries.txt").toDocumentId(),
                 childCursor2.getString(childCursor.getColumnIndexOrThrow(
                         Document.COLUMN_DOCUMENT_ID)));
         assertFalse(childCursor2.moveToNext());
@@ -263,7 +263,7 @@ public class ArchiveTest extends AndroidTestCase {
 
         assertTrue(cursor.moveToFirst());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId(),
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DOCUMENT_ID)));
         assertEquals("dir1",
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DISPLAY_NAME)));
@@ -274,11 +274,11 @@ public class ArchiveTest extends AndroidTestCase {
         assertFalse(cursor.moveToNext());
 
         final Cursor childCursor = mArchive.queryChildDocuments(
-                new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId(), null, null);
+                new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId(), null, null);
 
         assertTrue(childCursor.moveToFirst());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir1/dir2/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/dir2/").toDocumentId(),
                 childCursor.getString(childCursor.getColumnIndexOrThrow(
                         Document.COLUMN_DOCUMENT_ID)));
         assertEquals("dir2",
@@ -292,7 +292,7 @@ public class ArchiveTest extends AndroidTestCase {
 
         assertTrue(childCursor.moveToNext());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir1/dir3/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/dir3/").toDocumentId(),
                 childCursor.getString(childCursor.getColumnIndexOrThrow(
                         Document.COLUMN_DOCUMENT_ID)));
         assertEquals("dir3",
@@ -306,12 +306,12 @@ public class ArchiveTest extends AndroidTestCase {
         assertFalse(cursor.moveToNext());
 
         final Cursor childCursor2 = mArchive.queryChildDocuments(
-                new ArchiveId(ARCHIVE_URI, "dir1/dir2/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/dir2/").toDocumentId(),
                 null, null);
         assertFalse(childCursor2.moveToFirst());
 
         final Cursor childCursor3 = mArchive.queryChildDocuments(
-                new ArchiveId(ARCHIVE_URI, "dir1/dir3/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/dir3/").toDocumentId(),
                 null, null);
         assertFalse(childCursor3.moveToFirst());
     }
@@ -319,34 +319,34 @@ public class ArchiveTest extends AndroidTestCase {
     public void testGetDocumentType() throws IOException {
         loadArchive(getNonSeekableDescriptor(R.raw.archive));
         assertEquals(Document.MIME_TYPE_DIR, mArchive.getDocumentType(
-                new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId()));
+                new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId()));
         assertEquals("text/plain", mArchive.getDocumentType(
-                new ArchiveId(ARCHIVE_URI, "file1.txt").toDocumentId()));
+                new ArchiveId(ARCHIVE_URI, "/file1.txt").toDocumentId()));
     }
 
     public void testIsChildDocument() throws IOException {
         loadArchive(getNonSeekableDescriptor(R.raw.archive));
         final String documentId = new ArchiveId(ARCHIVE_URI, "/").toDocumentId();
         assertTrue(mArchive.isChildDocument(documentId,
-                new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId()));
+                new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId()));
         assertFalse(mArchive.isChildDocument(documentId,
-                new ArchiveId(ARCHIVE_URI, "this-does-not-exist").toDocumentId()));
+                new ArchiveId(ARCHIVE_URI, "/this-does-not-exist").toDocumentId()));
         assertTrue(mArchive.isChildDocument(
-                new ArchiveId(ARCHIVE_URI, "dir1/").toDocumentId(),
-                new ArchiveId(ARCHIVE_URI, "dir1/cherries.txt").toDocumentId()));
+                new ArchiveId(ARCHIVE_URI, "/dir1/").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir1/cherries.txt").toDocumentId()));
         assertTrue(mArchive.isChildDocument(documentId,
-                new ArchiveId(ARCHIVE_URI, "dir1/cherries.txt").toDocumentId()));
+                new ArchiveId(ARCHIVE_URI, "/dir1/cherries.txt").toDocumentId()));
     }
 
     public void testQueryDocument() throws IOException {
         loadArchive(getNonSeekableDescriptor(R.raw.archive));
         final Cursor cursor = mArchive.queryDocument(
-                new ArchiveId(ARCHIVE_URI, "dir2/strawberries.txt").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir2/strawberries.txt").toDocumentId(),
                 null);
 
         assertTrue(cursor.moveToFirst());
         assertEquals(
-                new ArchiveId(ARCHIVE_URI, "dir2/strawberries.txt").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir2/strawberries.txt").toDocumentId(),
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DOCUMENT_ID)));
         assertEquals("strawberries.txt",
                 cursor.getString(cursor.getColumnIndexOrThrow(Document.COLUMN_DISPLAY_NAME)));
@@ -369,7 +369,7 @@ public class ArchiveTest extends AndroidTestCase {
     // Common part of testOpenDocument and testOpenDocument_NonSeekable.
     void commonTestOpenDocument() throws IOException {
         final ParcelFileDescriptor descriptor = mArchive.openDocument(
-                new ArchiveId(ARCHIVE_URI, "dir2/strawberries.txt").toDocumentId(),
+                new ArchiveId(ARCHIVE_URI, "/dir2/strawberries.txt").toDocumentId(),
                 "r", null /* signal */);
         try (final ParcelFileDescriptor.AutoCloseInputStream inputStream =
                 new ParcelFileDescriptor.AutoCloseInputStream(descriptor)) {