8339874: Avoid duplicate checking of trailing slash in ZipFile.getZipEntry

Reviewed-by: lancea, redestad

Author: Eirik Bjørsnøs

src/java.base/share/classes/java/util/zip/ZipCoder.java

@@ -158,13 +158,6 @@ static int hash(String name) {
         return hsh;
     }
 
-    boolean hasTrailingSlash(byte[] a, int end) {
-        byte[] slashBytes = slashBytes();
-        return end >= slashBytes.length &&
-            Arrays.mismatch(a, end - slashBytes.length, end, slashBytes, 0, slashBytes.length) == -1;
-    }
-
-    private byte[] slashBytes;
     private final Charset cs;
     protected CharsetDecoder dec;
     private CharsetEncoder enc;
@@ -191,23 +184,6 @@ private CharsetEncoder encoder() {
         return enc;
     }
 
-    // This method produces an array with the bytes that will correspond to a
-    // trailing '/' in the chosen character encoding.
-    //
-    // While in most charsets a trailing slash will be encoded as the byte
-    // value of '/', this does not hold in the general case. E.g., in charsets
-    // such as UTF-16 and UTF-32 it will be represented by a sequence of 2 or 4
-    // bytes, respectively.
-    private byte[] slashBytes() {
-        if (slashBytes == null) {
-            // Take into account charsets that produce a BOM, e.g., UTF-16
-            byte[] slash = "/".getBytes(cs);
-            byte[] doubleSlash = "//".getBytes(cs);
-            slashBytes = Arrays.copyOfRange(doubleSlash, slash.length, doubleSlash.length);
-        }
-        return slashBytes;
-    }
-
     /**
      * This method is used by ZipFile.Source.getEntryPos when comparing the
      * name being looked up to candidate names encoded in the CEN byte
@@ -297,8 +273,7 @@ int checkedHash(byte[] a, int off, int len) throws Exception {
             return h;
         }
 
-        @Override
-        boolean hasTrailingSlash(byte[] a, int end) {
+        private boolean hasTrailingSlash(byte[] a, int end) {
             return end > 0 && a[end - 1] == '/';
         }
 

src/java.base/share/classes/java/util/zip/ZipFile.java

@@ -347,9 +347,12 @@ public ZipEntry getEntry(String name) {
         ZipEntry entry = null;
         synchronized (this) {
             ensureOpen();
-            int pos = res.zsrc.getEntryPos(name, true);
-            if (pos != -1) {
-                entry = getZipEntry(name, pos);
+            // Look up the name and CEN header position of the entry.
+            // The resolved name may include a trailing slash.
+            // See Source::getEntryPos for details.
+            EntryPos pos = res.zsrc.getEntryPos(name, true);
+            if (pos != null) {
+                entry = getZipEntry(pos.name, pos.pos);
             }
         }
         return entry;
@@ -387,7 +390,12 @@ public InputStream getInputStream(ZipEntry entry) throws IOException {
             if (Objects.equals(lastEntryName, entry.name)) {
                 pos = lastEntryPos;
             } else {
-                pos = zsrc.getEntryPos(entry.name, false);
+                EntryPos entryPos = zsrc.getEntryPos(entry.name, false);
+                if (entryPos != null) {
+                    pos = entryPos.pos;
+                } else {
+                    pos = -1;
+                }
             }
             if (pos == -1) {
                 return null;
@@ -540,7 +548,8 @@ public T next() {
                     throw new NoSuchElementException();
                 }
                 // each "entry" has 3 ints in table entries
-                return (T)getZipEntry(null, res.zsrc.getEntryPos(i++ * 3));
+                int pos = res.zsrc.getEntryPos(i++ * 3);
+                return (T)getZipEntry(getEntryName(pos), pos);
             }
         }
 
@@ -612,7 +621,7 @@ public Stream<? extends ZipEntry> stream() {
         synchronized (this) {
             ensureOpen();
             return StreamSupport.stream(new EntrySpliterator<>(0, res.zsrc.total,
-                pos -> getZipEntry(null, pos)), false);
+                pos -> getZipEntry(getEntryName(pos), pos)), false);
        }
     }
 
@@ -655,7 +664,7 @@ private Stream<JarEntry> jarStream() {
         synchronized (this) {
             ensureOpen();
             return StreamSupport.stream(new EntrySpliterator<>(0, res.zsrc.total,
-                pos -> (JarEntry)getZipEntry(null, pos)), false);
+                pos -> (JarEntry)getZipEntry(getEntryName(pos), pos)), false);
         }
     }
 
@@ -665,30 +674,10 @@ private Stream<JarEntry> jarStream() {
     /* Check ensureOpen() before invoking this method */
     private ZipEntry getZipEntry(String name, int pos) {
         byte[] cen = res.zsrc.cen;
-        int nlen = CENNAM(cen, pos);
-        int elen = CENEXT(cen, pos);
-        int clen = CENCOM(cen, pos);
+        ZipEntry e = this instanceof JarFile jarFile
+                ? Source.JUJA.entryFor(jarFile, name)
+                : new ZipEntry(name);
 
-        ZipCoder zc = res.zsrc.zipCoderForPos(pos);
-        if (name != null) {
-            // only need to check for mismatch of trailing slash
-            if (nlen > 0 &&
-                !name.isEmpty() &&
-                zc.hasTrailingSlash(cen, pos + CENHDR + nlen) &&
-                !name.endsWith("/"))
-            {
-                name += '/';
-            }
-        } else {
-            // invoked from iterator, use the entry name stored in cen
-            name = zc.toString(cen, pos + CENHDR, nlen);
-        }
-        ZipEntry e;
-        if (this instanceof JarFile) {
-            e = Source.JUJA.entryFor((JarFile)this, name);
-        } else {
-            e = new ZipEntry(name);
-        }
         e.flag = CENFLG(cen, pos);
         e.xdostime = CENTIM(cen, pos);
         e.crc = CENCRC(cen, pos);
@@ -700,12 +689,17 @@ private ZipEntry getZipEntry(String name, int pos) {
             e.externalFileAttributes = CENATX_PERMS(cen, pos) & 0xFFFF;
         }
 
+        int nlen = CENNAM(cen, pos);
+        int elen = CENEXT(cen, pos);
+        int clen = CENCOM(cen, pos);
+
         if (elen != 0) {
             int start = pos + CENHDR + nlen;
             e.setExtra0(Arrays.copyOfRange(cen, start, start + elen), true, false);
         }
         if (clen != 0) {
             int start = pos + CENHDR + nlen + elen;
+            ZipCoder zc = res.zsrc.zipCoderForPos(pos);
             e.comment = zc.toString(cen, start, clen);
         }
         lastEntryName = e.name;
@@ -1176,6 +1170,8 @@ public void setExternalFileAttributes(ZipEntry ze, int externalFileAttributes) {
              }
         );
     }
+    // Represents the resolved name and position of a CEN record
+    static record EntryPos(String name, int pos) {}
 
     private static class Source {
         // While this is only used from ZipFile, defining it there would cause
@@ -1849,12 +1845,12 @@ private static void zerror(String msg) throws ZipException {
         }
 
         /*
-         * Returns the {@code pos} of the ZIP cen entry corresponding to the
-         * specified entry name, or -1 if not found.
+         * Returns the resolved name and position of the ZIP cen entry corresponding
+         *  to the specified entry name, or {@code null} if not found.
          */
-        private int getEntryPos(String name, boolean addSlash) {
+        private EntryPos getEntryPos(String name, boolean addSlash) {
             if (total == 0) {
-                return -1;
+                return null;
             }
 
             int hsh = ZipCoder.hash(name);
@@ -1877,7 +1873,7 @@ private int getEntryPos(String name, boolean addSlash) {
                     switch (zc.compare(name, cen, noff, nlen, addSlash)) {
                         case EXACT_MATCH:
                             // We found an exact match for "name"
-                            return pos;
+                            return new EntryPos(name, pos);
                         case DIRECTORY_MATCH:
                             // We found the directory "name/"
                             // Track its position, then continue the search for "name"
@@ -1892,10 +1888,10 @@ private int getEntryPos(String name, boolean addSlash) {
             // Reaching this point means we did not find "name".
             // Return the position of "name/" if we found it
             if (dirPos != -1) {
-                return dirPos;
+                return new EntryPos(name + "/", dirPos);
             }
             // No entry found
-            return -1;
+            return null;
         }
 
         private ZipCoder zipCoderForPos(int pos) {