8197398: (zipfs) Files.walkFileTree walk indefinitelly while processing JAR file with "/" as a directory inside.
Reviewed-by: alanb
--- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java Tue Sep 04 14:35:59 2018 -0700
+++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java Tue Sep 04 17:04:10 2018 -0700
@@ -317,7 +317,7 @@
if (inode == null)
return null;
e = new Entry(inode.name, inode.isdir); // pseudo directory
- e.method = METHOD_STORED; // STORED for dir
+ e.method = METHOD_STORED; // STORED for dir
e.mtime = e.atime = e.ctime = zfsDefaultTimeStamp;
}
} finally {
@@ -1087,9 +1087,8 @@
if (pos + CENHDR + nlen > limit) {
zerror("invalid CEN header (bad header size)");
}
- IndexNode inode = new IndexNode(cen, pos + CENHDR, nlen, pos);
+ IndexNode inode = new IndexNode(cen, nlen, pos);
inodes.put(inode, inode);
-
// skip ext and comment
pos += (CENHDR + nlen + elen + clen);
}
@@ -1173,9 +1172,15 @@
size = 16;
}
// read loc, use the original loc.elen/nlen
- if (readFullyAt(buf, 0, LOCHDR , locoff) != LOCHDR)
+ //
+ // an extra byte after loc is read, which should be the first byte of the
+ // 'name' field of the loc. if this byte is '/', which means the original
+ // entry has an absolute path in original zip/jar file, the e.writeLOC()
+ // is used to output the loc, in which the leading "/" will be removed
+ if (readFullyAt(buf, 0, LOCHDR + 1 , locoff) != LOCHDR + 1)
throw new ZipException("loc: reading failed");
- if (updateHeader) {
+
+ if (updateHeader || LOCNAM(buf) > 0 && buf[LOCHDR] == '/') {
locoff += LOCHDR + LOCNAM(buf) + LOCEXT(buf); // skip header
size += e.csize;
written = e.writeLOC(os) + size;
@@ -1275,6 +1280,10 @@
if (inode.pos == -1) {
continue; // pseudo directory node
}
+ if (inode.name.length == 1 && inode.name[0] == '/') {
+ continue; // no root '/' directory even it
+ // exits in original zip/jar file.
+ }
e = Entry.readCEN(this, inode);
try {
written += copyLOCEntry(e, false, os, written, buf);
@@ -1796,15 +1805,20 @@
this.pos = pos;
}
- // constructor for cenInit()
- IndexNode(byte[] cen, int noff, int nlen, int pos) {
+ // constructor for cenInit() (1) remove tailing '/' (2) pad leading '/'
+ IndexNode(byte[] cen, int nlen, int pos) {
+ int noff = pos + CENHDR;
if (cen[noff + nlen - 1] == '/') {
isdir = true;
nlen--;
}
- name = new byte[nlen + 1];
- System.arraycopy(cen, pos + CENHDR, name, 1, nlen);
- name[0] = '/';
+ if (nlen > 0 && cen[noff] == '/') {
+ name = Arrays.copyOfRange(cen, noff, noff + nlen);
+ } else {
+ name = new byte[nlen + 1];
+ System.arraycopy(cen, noff, name, 1, nlen);
+ name[0] = '/';
+ }
name(name);
this.pos = pos;
}
@@ -2505,7 +2519,12 @@
private void buildNodeTree() throws IOException {
beginWrite();
try {
- IndexNode root = new IndexNode(ROOTPATH, true);
+ IndexNode root = inodes.get(LOOKUPKEY.as(ROOTPATH));
+ if (root == null) {
+ root = new IndexNode(ROOTPATH, true);
+ } else {
+ inodes.remove(root);
+ }
IndexNode[] nodes = inodes.keySet().toArray(new IndexNode[0]);
inodes.put(root, root);
ParentLookup lookup = new ParentLookup();
--- a/test/jdk/jdk/nio/zipfs/ZFSTests.java Tue Sep 04 14:35:59 2018 -0700
+++ b/test/jdk/jdk/nio/zipfs/ZFSTests.java Tue Sep 04 17:04:10 2018 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -22,7 +22,7 @@
*/
/* @test
- * @bug 7156873 8040059 8028480 8034773 8153248 8061777
+ * @bug 7156873 8040059 8028480 8034773 8153248 8061777 8197398
* @summary ZipFileSystem regression tests
*
* @modules jdk.zipfs
@@ -31,22 +31,155 @@
*/
+import java.io.IOException;
import java.io.OutputStream;
import java.net.URI;
import java.nio.ByteBuffer;
import java.nio.channels.*;
import java.nio.file.*;
+import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.spi.*;
import java.util.*;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+import java.util.zip.ZipOutputStream;
public class ZFSTests {
public static void main(String[] args) throws Throwable {
+ test8197398();
test7156873();
test8061777();
tests();
}
+ static void test8197398() throws Throwable {
+
+ // root entry "/"
+ Path path = Paths.get("rootdir.zip");
+ URI uri = URI.create("jar:" + path.toUri());
+
+ Set<String> dirs = new HashSet<>();
+ Set<String> files = new HashSet<>();
+ try (OutputStream os = Files.newOutputStream(path);
+ ZipOutputStream zos = new ZipOutputStream(os)) {
+ zos.putNextEntry(new ZipEntry("/")); dirs.add("/");
+ zos.putNextEntry(new ZipEntry("foo")); files.add("/foo");
+ zos.write("/foo".getBytes());
+ zos.putNextEntry(new ZipEntry("bar")); files.add("/bar");
+ zos.write("/bar".getBytes());
+ }
+ AtomicInteger cnt = new AtomicInteger();
+ int max = 3;
+ try (FileSystem fs = FileSystems.newFileSystem(uri, Collections.emptyMap())) {
+ Files.walkFileTree(fs.getRootDirectories().iterator().next(),
+ new SimpleFileVisitor<Path>() {
+
+ @Override
+ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+ throws IOException {
+ if (cnt.incrementAndGet() > max)
+ throw new RuntimeException("visited too many files/dirs");
+ files.remove(file.toString());
+ if (!Arrays.equals(Files.readAllBytes(file), file.toString().getBytes()))
+ throw new RuntimeException("visited files has wrong content: " + file);
+ return FileVisitResult.CONTINUE;
+ }
+
+ @Override
+ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
+ throws IOException {
+ if (cnt.incrementAndGet() > max)
+ throw new RuntimeException("visited too many files/dirs");
+ dirs.remove(dir.toString());
+ return FileVisitResult.CONTINUE;
+ }
+ });
+ if (cnt.get() != max || dirs.size() != 0 || files.size() != 0)
+ throw new RuntimeException("walk files/dirs failed");
+
+ } finally {
+ Files.deleteIfExists(path);
+ }
+
+ // absolute file/dir, no need to test cnt again..
+ dirs.clear();
+ files.clear();
+ try (OutputStream os = Files.newOutputStream(path);
+ ZipOutputStream zos = new ZipOutputStream(os)) {
+ zos.putNextEntry(new ZipEntry("/")); dirs.add("/");
+ zos.putNextEntry(new ZipEntry("/fooo/")); dirs.add("/fooo");
+ zos.putNextEntry(new ZipEntry("/foo")); files.add("/foo");
+ zos.write("/foo".getBytes());
+ zos.putNextEntry(new ZipEntry("/bar")); files.add("/bar");
+ zos.write("/bar".getBytes());
+ zos.putNextEntry(new ZipEntry("/fooo/bar")); files.add("/fooo/bar");
+ zos.write("/fooo/bar".getBytes());
+ }
+
+ try (FileSystem fs = FileSystems.newFileSystem(uri, Collections.emptyMap())) {
+ Files.walkFileTree(fs.getRootDirectories().iterator().next(),
+ new SimpleFileVisitor<Path>() {
+ @Override
+ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+ throws IOException {
+ files.remove(file.toString());
+ if (!Arrays.equals(Files.readAllBytes(file), file.toString().getBytes()))
+ throw new RuntimeException("visited files has wrong content: " + file);
+ return FileVisitResult.CONTINUE;
+ }
+ @Override
+ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
+ throws IOException {
+ dirs.remove(dir.toString());
+ return FileVisitResult.CONTINUE;
+ }
+ });
+ if (dirs.size() != 0 || files.size() != 0)
+ throw new RuntimeException("walk files/dirs failed");
+
+ // for next test: updated any entry, the result zipfs file should have no
+ // absolute path entry
+ Files.write(fs.getPath("/foo"), "/foo".getBytes());
+ }
+
+ // updated zfs should have the same dirs/files (path is not absolute though)
+ dirs.add("/");
+ dirs.add("/fooo");
+ files.add("/foo");
+ files.add("/bar");
+ files.add("/fooo/bar");
+ try (FileSystem fs = FileSystems.newFileSystem(uri, Collections.emptyMap())) {
+ Files.walk(fs.getPath("/")).forEach( p -> {
+ if (Files.isDirectory(p)) {
+ dirs.remove(p.toString());
+ } else {
+ files.remove(p.toString());
+ try {
+ if (!Arrays.equals(Files.readAllBytes(p), p.toString().getBytes()))
+ throw new RuntimeException("visited files has wrong content: " + p);
+ } catch (IOException x) {
+ throw new RuntimeException(x);
+ }
+ }
+ });
+ if (dirs.size() != 0 || files.size() != 0)
+ throw new RuntimeException("walk files/dirs failed");
+
+ }
+ // updated zip file should not have "/" and entry with absolute path
+ String[] entries = new ZipFile(path.toFile()).stream()
+ .map(ZipEntry::toString)
+ .sorted()
+ .toArray(String[]::new);
+ if (!Arrays.equals(entries, new String[] {"bar", "foo", "fooo/", "fooo/bar" })) {
+ System.out.println("unexpeded: " + Arrays.toString(entries));
+ throw new RuntimeException("unexpected entreis in updated zipfs file");
+ }
+ Files.deleteIfExists(path);
+ }
+
static void test7156873() throws Throwable {
String DIRWITHSPACE = "testdir with spaces";
Path dir = Paths.get(DIRWITHSPACE);
@@ -95,6 +228,7 @@
Path path = Paths.get("file.zip");
try {
URI uri = URI.create("jar:" + path.toUri());
+
Map<String, Object> env = new HashMap<String, Object>();
env.put("create", "true");
try (FileSystem fs = FileSystems.newFileSystem(uri, env)) {