7006576: (zipfs) Path.exists() always returns false on dirs when zip/JAR file built without dirs

7009092: (zipfs) ZipPath.isSameFile() should always return true if this Path and the given Path are equal
7009085: (zipfs) ZipPath.normalize("/./.") returns null
7009102: (zipfs) ZipPath.toRealPath() should always return absolute path

Zip filesystem provider update

Reviewed-by: alanb
This commit is contained in:
Xueming Shen 2011-01-24 11:47:06 -08:00
parent 75f781fa99
commit 8bce4225f6
5 changed files with 178 additions and 169 deletions

View File

@ -78,7 +78,6 @@ public class ZipFileSystem extends FileSystem {
// configurable by env map
private final String defaultDir; // default dir for the file system
private final String nameEncoding; // default encoding for name/comment
private final boolean buildDirTree; // build a dir tree for directoryStream ops
private final boolean useTempFile; // use a temp file for newOS, default
// is to use BAOS for better performance
private final boolean createNew; // create a new zip if not exists
@ -94,7 +93,6 @@ public class ZipFileSystem extends FileSystem {
this.createNew = "true".equals(env.get("create"));
this.nameEncoding = env.containsKey("encoding") ?
(String)env.get("encoding") : "UTF-8";
this.buildDirTree = TRUE.equals(env.get("buildDirTreea"));
this.useTempFile = TRUE.equals(env.get("useTempFile"));
this.defaultDir = env.containsKey("default.dir") ?
(String)env.get("default.dir") : "/";
@ -290,28 +288,16 @@ public class ZipFileSystem extends FileSystem {
try {
ensureOpen();
e = getEntry0(path);
} finally {
endRead();
}
if (e == null) {
if (path.length == 0) {
e = new Entry(new byte[0]); // root
} else if (buildDirTree) {
IndexNode inode = getDirs().get(IndexNode.keyOf(path));
if (e == null) {
IndexNode inode = getInode(path);
if (inode == null)
return null;
e = new Entry(inode.name);
} else {
return null;
e = new Entry(inode.name); // pseudo directory
e.method = METHOD_STORED; // STORED for dir
e.mtime = e.atime = e.ctime = -1;// -1 for all times
}
e.method = METHOD_STORED; // STORED for dir
BasicFileAttributes bfas = Attributes.readBasicFileAttributes(zfpath);
if (bfas.lastModifiedTime() != null)
e.mtime = bfas.lastModifiedTime().toMillis();
if (bfas.lastAccessTime() != null)
e.atime = bfas.lastAccessTime().toMillis();
if (bfas.creationTime() != null)
e.ctime = bfas.creationTime().toMillis();
} finally {
endRead();
}
return new ZipFileAttributes(e);
}
@ -346,7 +332,7 @@ public class ZipFileSystem extends FileSystem {
beginRead();
try {
ensureOpen();
return getEntry0(path) != null;
return getInode(path) != null;
} finally {
endRead();
}
@ -355,13 +341,10 @@ public class ZipFileSystem extends FileSystem {
boolean isDirectory(byte[] path)
throws IOException
{
if (buildDirTree)
return getDirs().containsKey(IndexNode.keyOf(path));
beginRead();
try {
Entry e = getEntry0(path);
return (e != null && e.isDir()) || path.length == 0;
IndexNode n = getInode(path);
return n != null && n.isDir();
} finally {
endRead();
}
@ -383,39 +366,16 @@ public class ZipFileSystem extends FileSystem {
beginWrite(); // iteration of inodes needs exclusive lock
try {
ensureOpen();
if (buildDirTree) {
IndexNode inode = getDirs().get(IndexNode.keyOf(path));
if (inode == null)
throw new NotDirectoryException(getString(path));
List<Path> list = new ArrayList<>();
IndexNode child = inode.child;
while (child != null) {
ZipPath zp = toZipPath(child.name);
if (filter == null || filter.accept(zp))
list.add(zp);
child = child.sibling;
}
return list.iterator();
}
if (!isDirectory(path))
IndexNode inode = getInode(path);
if (inode == null)
throw new NotDirectoryException(getString(path));
List<Path> list = new ArrayList<>();
path = toDirectoryPath(path);
for (IndexNode key : inodes.keySet()) {
if (!isParentOf(path, key.name)) // is "path" the parent of "name"
continue;
int off = path.length;
while (off < key.name.length) {
if (key.name[off] == '/')
break;
off++;
}
if (off < (key.name.length - 1))
continue;
ZipPath zp = toZipPath(key.name);
IndexNode child = inode.child;
while (child != null) {
ZipPath zp = toZipPath(child.name);
if (filter == null || filter.accept(zp))
list.add(zp);
child = child.sibling;
}
return list.iterator();
} finally {
@ -433,7 +393,6 @@ public class ZipFileSystem extends FileSystem {
ensureOpen();
if (dir.length == 0 || exists(dir)) // root dir, or exiting dir
throw new FileAlreadyExistsException(getString(dir));
checkParents(dir);
Entry e = new Entry(dir, Entry.NEW);
e.method = METHOD_STORED; // STORED for dir
@ -476,7 +435,7 @@ public class ZipFileSystem extends FileSystem {
checkParents(dst);
}
Entry u = new Entry(eSrc, Entry.COPY); // copy eSrc entry
u.name = dst; // change name
u.name(dst); // change name
if (eSrc.type == Entry.NEW || eSrc.type == Entry.FILECH)
{
u.type = eSrc.type; // make it the same type
@ -518,7 +477,7 @@ public class ZipFileSystem extends FileSystem {
if (opt == APPEND)
hasAppend = true;
}
beginRead(); // only need a readlock, the "update()" will
beginRead(); // only need a readlock, the "update()" will
try { // try to obtain a writelock when the os is
ensureOpen(); // being closed.
Entry e = getEntry0(path);
@ -869,43 +828,27 @@ public class ZipFileSystem extends FileSystem {
private void checkParents(byte[] path) throws IOException {
beginRead();
try {
while ((path = getParent(path)) != null) {
if (!inodes.containsKey(IndexNode.keyOf(path)))
while ((path = getParent(path)) != null && path.length != 0) {
if (!inodes.containsKey(IndexNode.keyOf(path))) {
throw new NoSuchFileException(getString(path));
}
}
} finally {
endRead();
}
}
private static byte[] ROOTPATH = new byte[0];
private static byte[] getParent(byte[] path) {
int off = path.length - 1;
if (off > 0 && path[off] == '/') // isDirectory
off--;
while (off > 0 && path[off] != '/') { off--; }
if (off == 0)
return null; // top entry
if (off <= 0)
return ROOTPATH;
return Arrays.copyOf(path, off + 1);
}
// If "starter" is the parent directory of "path"
private static boolean isParentOf(byte[] p, byte[] c) {
final int plen = p.length;
if (plen == 0) // root dir
return true;
if (plen >= c.length)
return false;
int n = 0;
while (n < plen) {
if (p[n] != c[n])
return false;
n++;
}
if (p[n - 1] != '/' && (c[n] != '/' || n == c.length - 1))
return false;
return true;
}
private final void beginWrite() {
rwlock.writeLock().lock();
}
@ -926,7 +869,7 @@ public class ZipFileSystem extends FileSystem {
private volatile boolean isOpen = true;
private final SeekableByteChannel ch; // channel to the zipfile
final byte[] cen; // CEN & ENDHDR
final byte[] cen; // CEN & ENDHDR
private END end;
private long locpos; // position of first LOC header (usually 0)
@ -1058,6 +1001,7 @@ public class ZipFileSystem extends FileSystem {
if (end.endpos == 0) {
inodes = new LinkedHashMap<>(10);
locpos = 0;
buildNodeTree();
return null; // only END header present
}
if (end.cenlen > end.endpos)
@ -1101,6 +1045,7 @@ public class ZipFileSystem extends FileSystem {
if (pos + ENDHDR != cen.length) {
zerror("invalid CEN header (bad header size)");
}
buildNodeTree();
return cen;
}
@ -1125,12 +1070,15 @@ public class ZipFileSystem extends FileSystem {
private boolean hasUpdate = false;
// shared key. consumer guarantees the "writeLock" before use it.
private final IndexNode LOOKUPKEY = IndexNode.keyOf(null);
private void updateDelete(Entry e) {
beginWrite();
try {
inodes.remove(IndexNode.keyOf(e.name)); //inodes.remove(e.name);
removeFromTree(e);
inodes.remove(e);
hasUpdate = true;
dirs = null;
} finally {
endWrite();
}
@ -1139,9 +1087,16 @@ public class ZipFileSystem extends FileSystem {
private void update(Entry e) {
beginWrite();
try {
inodes.put(IndexNode.keyOf(e.name), e); //inodes.put(e, e);
IndexNode old = inodes.put(e, e);
if (old != null) {
removeFromTree(old);
}
if (e.type == Entry.NEW || e.type == Entry.FILECH) {
IndexNode parent = inodes.get(LOOKUPKEY.as(getParent(e.name)));
e.sibling = parent.child;
parent.child = e;
}
hasUpdate = true;
dirs = null;
} finally {
endWrite();
}
@ -1229,7 +1184,7 @@ public class ZipFileSystem extends FileSystem {
// ext) without enflating/deflating from the old zip
// file LOC entry.
written += copyLOCEntry(e, true, os, written, buf);
} else { // NEW or FILECH
} else { // NEW, FILECH or CEN
e.locoff = written;
written += e.writeLOC(os); // write loc header
if (e.bytes != null) { // in-memory, deflated
@ -1265,7 +1220,10 @@ public class ZipFileSystem extends FileSystem {
} catch (IOException x) {
x.printStackTrace(); // skip any in-accurate entry
}
} else { // unchanged inode
} else { // unchanged inode
if (inode.pos == -1) {
continue; // pseudo directory node
}
e = Entry.readCEN(this, inode.pos);
try {
written += copyLOCEntry(e, false, os, written, buf);
@ -1318,34 +1276,28 @@ public class ZipFileSystem extends FileSystem {
//System.out.printf("->sync(%s) done!%n", toString());
}
private Entry getEntry0(byte[] path) throws IOException {
private IndexNode getInode(byte[] path) {
if (path == null)
throw new NullPointerException("path");
if (path.length == 0)
return null;
IndexNode inode = null;
IndexNode key = IndexNode.keyOf(path);
if ((inode = inodes.get(key)) == null) {
if (path[path.length -1] == '/') // already has a slash
return null;
IndexNode inode = inodes.get(key);
if (inode == null &&
(path.length == 0 || path[path.length -1] != '/')) {
// if does not ends with a slash
path = Arrays.copyOf(path, path.length + 1);
path[path.length - 1] = '/';
if ((inode = inodes.get(key.as(path))) == null)
return null;
inode = inodes.get(key.as(path));
}
if (inode instanceof Entry)
return (Entry)inode;
return Entry.readCEN(this, inode.pos);
return inode;
}
// Test if the "name" a parent directory of any entry (dir empty)
boolean isAncestor(byte[] name) {
for (Map.Entry<IndexNode, IndexNode> entry : inodes.entrySet()) {
byte[] ename = entry.getKey().name;
if (isParentOf(name, ename))
return true;
}
return false;
private Entry getEntry0(byte[] path) throws IOException {
IndexNode inode = getInode(path);
if (inode instanceof Entry)
return (Entry)inode;
if (inode == null || inode.pos == -1)
return null;
return Entry.readCEN(this, inode.pos);
}
public void deleteFile(byte[] path, boolean failIfNotExists)
@ -1359,7 +1311,7 @@ public class ZipFileSystem extends FileSystem {
if (failIfNotExists)
throw new NoSuchFileException(getString(path));
} else {
if (e.isDir() && isAncestor(path))
if (e.isDir() && e.child != null)
throw new DirectoryNotEmptyException(getString(path));
updateDelete(e);
}
@ -1539,7 +1491,6 @@ public class ZipFileSystem extends FileSystem {
public int available() {
return rem > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) rem;
}
public long size() {
return size;
}
@ -1762,7 +1713,7 @@ public class ZipFileSystem extends FileSystem {
int pos = -1; // postion in cen table, -1 menas the
// entry does not exists in zip file
IndexNode(byte[] name, int pos) {
as(name);
name(name);
this.pos = pos;
}
@ -1770,15 +1721,25 @@ public class ZipFileSystem extends FileSystem {
return new IndexNode(name, -1);
}
final IndexNode as(byte[] name) { // reuse the node, mostly
this.name = name; // as a lookup "key"
final void name(byte[] name) {
this.name = name;
this.hashcode = Arrays.hashCode(name);
}
final IndexNode as(byte[] name) { // reuse the node, mostly
name(name); // as a lookup "key"
return this;
}
boolean isDir() {
return name != null &&
(name.length == 0 || name[name.length - 1] == '/');
}
public boolean equals(Object other) {
if (!(other instanceof IndexNode))
if (!(other instanceof IndexNode)) {
return false;
}
return Arrays.equals(name, ((IndexNode)other).name);
}
@ -1798,6 +1759,7 @@ public class ZipFileSystem extends FileSystem {
static final int FILECH = 3; // fch update in "file"
static final int COPY = 4; // copy of a CEN entry
byte[] bytes; // updated content bytes
Path file; // use tmp file to store bytes;
int type = CEN; // default is the entry read from cen
@ -1825,7 +1787,7 @@ public class ZipFileSystem extends FileSystem {
Entry() {}
Entry(byte[] name) {
this.name = name;
name(name);
this.mtime = System.currentTimeMillis();
this.crc = 0;
this.size = 0;
@ -1839,8 +1801,8 @@ public class ZipFileSystem extends FileSystem {
}
Entry (Entry e, int type) {
name(e.name);
this.version = e.version;
this.name = e.name;
this.ctime = e.ctime;
this.atime = e.atime;
this.mtime = e.mtime;
@ -1855,7 +1817,6 @@ public class ZipFileSystem extends FileSystem {
this.attrsEx = e.attrsEx;
this.locoff = e.locoff;
this.comment = e.comment;
this.type = type;
}
@ -1865,12 +1826,6 @@ public class ZipFileSystem extends FileSystem {
this.method = METHOD_STORED;
}
boolean isDir() {
return name != null &&
(name.length == 0 ||
name[name.length - 1] == '/');
}
int version() throws ZipException {
if (method == METHOD_DEFLATED)
return 20;
@ -1909,7 +1864,7 @@ public class ZipFileSystem extends FileSystem {
locoff = CENOFF(cen, pos);
pos += CENHDR;
name = Arrays.copyOfRange(cen, pos, pos + nlen);
name(Arrays.copyOfRange(cen, pos, pos + nlen));
pos += nlen;
if (elen > 0) {
@ -2132,7 +2087,8 @@ public class ZipFileSystem extends FileSystem {
}
writeShort(os, flag); // general purpose bit flag
writeShort(os, method); // compression method
writeInt(os, mtime); // last modification time
// last modification time
writeInt(os, (int)javaToDosTime(mtime));
writeInt(os, crc); // crc-32
if (elen64 != 0) {
writeInt(os, ZIP64_MINVAL);
@ -2318,50 +2274,56 @@ public class ZipFileSystem extends FileSystem {
// structure.
// A possible solution is to build the node tree ourself as
// implemented below.
private HashMap<IndexNode, IndexNode> dirs;
private IndexNode root;
private IndexNode addToDir(IndexNode child) {
IndexNode cinode = dirs.get(child);
if (cinode != null)
return cinode;
byte[] cname = child.name;
byte[] pname = getParent(cname);
IndexNode pinode;
if (pname != null)
pinode = addToDir(IndexNode.keyOf(pname));
else
pinode = root;
cinode = inodes.get(child);
if (cname[cname.length -1] != '/') { // not a dir
cinode.sibling = pinode.child;
pinode.child = cinode;
return null;
private void addToTree(IndexNode inode, HashSet<IndexNode> dirs) {
if (dirs.contains(inode)) {
return;
}
//cinode = dirs.get(child);
if (cinode == null) // pseudo directry entry
cinode = new IndexNode(cname, -1);
cinode.sibling = pinode.child;
pinode.child = cinode;
dirs.put(child, cinode);
return cinode;
IndexNode parent;
byte[] name = inode.name;
byte[] pname = getParent(name);
if (inodes.containsKey(LOOKUPKEY.as(pname))) {
parent = inodes.get(LOOKUPKEY);
} else { // pseudo directory entry
parent = new IndexNode(pname, -1);
inodes.put(parent, parent);
}
addToTree(parent, dirs);
inode.sibling = parent.child;
parent.child = inode;
if (name[name.length -1] == '/')
dirs.add(inode);
}
private HashMap<IndexNode, IndexNode> getDirs()
throws IOException
{
private void removeFromTree(IndexNode inode) {
IndexNode parent = inodes.get(LOOKUPKEY.as(getParent(inode.name)));
IndexNode child = parent.child;
if (child == inode) {
parent.child = child.sibling;
} else {
IndexNode last = child;
while ((child = child.sibling) != null) {
if (child == inode) {
last.sibling = child.sibling;
break;
} else {
last = child;
}
}
}
}
private void buildNodeTree() throws IOException {
beginWrite();
try {
if (dirs != null)
return dirs;
dirs = new HashMap<>();
root = new IndexNode(new byte[0], -1);
dirs.put(root, root);
for (IndexNode node : inodes.keySet())
addToDir(node);
return dirs;
HashSet<IndexNode> dirs = new HashSet<>();
IndexNode root = new IndexNode(ROOTPATH, -1);
inodes.put(root, root);
dirs.add(root);
for (IndexNode node : inodes.keySet().toArray(new IndexNode[0])) {
addToTree(node, dirs);
}
} finally {
endWrite();
}

View File

@ -157,7 +157,7 @@ public class ZipPath extends Path {
@Override
public ZipPath toRealPath(boolean resolveLinks) throws IOException {
ZipPath realPath = new ZipPath(zfs, getResolvedPath());
ZipPath realPath = new ZipPath(zfs, getResolvedPath()).toAbsolutePath();
realPath.checkAccess();
return realPath;
}
@ -472,8 +472,11 @@ public class ZipPath extends Path {
int n = offsets[i];
int len = (i == offsets.length - 1)?
(path.length - n):(offsets[i + 1] - n - 1);
if (len == 1 && path[n] == (byte)'.')
if (len == 1 && path[n] == (byte)'.') {
if (m == 0 && path[0] == '/') // absolute path
to[m++] = '/';
continue;
}
if (len == 2 && path[n] == '.' && path[n + 1] == '.') {
if (lastMOff >= 0) {
m = lastM[lastMOff--]; // retreat
@ -726,6 +729,8 @@ public class ZipPath extends Path {
@Override
public boolean isSameFile(Path other) throws IOException {
if (this.equals(other))
return true;
if (other == null ||
this.getFileSystem() != other.getFileSystem())
return false;

View File

@ -193,6 +193,17 @@ public class PathOps {
return this;
}
PathOps isSameFile(String target) {
try {
out.println("check two paths are same");
checkPath();
check(path.isSameFile(test(target).path()), true);
} catch (IOException ioe) {
fail();
}
return this;
}
PathOps invalid() {
if (!(exc instanceof InvalidPathException)) {
out.println("InvalidPathException not thrown as expected");
@ -344,7 +355,12 @@ public class PathOps {
.normalize("foo");
test("/foo/bar/gus/../..")
.normalize("/foo");
test("/./.")
.normalize("/");
test("/.")
.normalize("/");
test("/./abc")
.normalize("/abc");
// invalid
test("foo\u0000bar")
.invalid();
@ -365,6 +381,10 @@ public class PathOps {
.root("/")
.parent("/foo")
.name("bar");
// isSameFile
test("/fileDoesNotExist")
.isSameFile("/fileDoesNotExist");
}
static void npes() {

View File

@ -28,6 +28,7 @@ import java.nio.file.*;
import java.nio.file.attribute.*;
import java.net.*;
import java.util.*;
import java.util.zip.*;
import static java.nio.file.StandardOpenOption.*;
import static java.nio.file.StandardCopyOption.*;
@ -42,7 +43,8 @@ public class ZipFSTester {
FileSystem fs = null;
try {
fs = newZipFileSystem(Paths.get(args[0]), new HashMap<String, Object>());
test(fs);
test0(fs);
test1(fs);
test2(fs); // more tests
} finally {
if (fs != null)
@ -50,11 +52,31 @@ public class ZipFSTester {
}
}
static void test(FileSystem fs)
static void test0(FileSystem fs)
throws Exception
{
List<String> list = new LinkedList<>();
try (ZipFile zf = new ZipFile(fs.toString())) {
Enumeration<? extends ZipEntry> zes = zf.entries();
while (zes.hasMoreElements()) {
list.add(zes.nextElement().getName());
}
for (String pname : list) {
Path path = fs.getPath(pname);
if (!path.exists())
throw new RuntimeException("path existence check failed!");
while ((path = path.getParent()) != null) {
if (!path.exists())
throw new RuntimeException("parent existence check failed!");
}
}
}
}
static void test1(FileSystem fs)
throws Exception
{
Random rdm = new Random();
// clone a fs and test on it
Path tmpfsPath = getTempPath();
Map<String, Object> env = new HashMap<String, Object>();

View File

@ -21,7 +21,7 @@
# questions.
#
# @test
# @bug 6990846
# @bug 6990846 7009092 7009085
# @summary Test ZipFileSystem demo
# @build Basic PathOps ZipFSTester
# @run shell basic.sh