-
Bug
-
Resolution: Fixed
-
P4
-
6, 7
-
b75
-
generic
-
generic
-
Verified
Issue | Fix Version | Assignee | Priority | Status | Resolution | Resolved In Build |
---|---|---|---|---|---|---|
JDK-2194731 | OpenJDK6 | Jonathan Gibbons | P4 | Resolved | Fixed | b21 |
From http://mail.openjdk.java.net/pipermail/compiler-dev/2007-June/000062.html
We have a problem. Actually, we have two, one for RegularFileObject and
one for ZipFileObject.
In JavacFileManager, RegularFileObject.equals does not object the
general contract of equals. Here is the relevant code:
@Override
public boolean equals(Object other) {
if (!(other instanceof RegularFileObject))
return false;
RegularFileObject o = (RegularFileObject) other;
try {
return f.equals(o.f)
|| f.getCanonicalFile().equals(o.f.getCanonicalFile());
} catch (IOException e) {
return false;
}
}
@Override
public int hashCode() {
return f.hashCode();
}
The contract for Object.equals says that:
"Note that it is generally necessary to override the hashCode method
whenever this method is overridden, so as to maintain the general
contract for the hashCode method, which states that equal objects must
have equal hash codes."
The problem is that getCanonicalFile will equate a relative file ("foo")
with its absolute equivalent ("/home/jjg/foo"), even though they have
different hashcodes.
There are two possible solutions. For maximum compatibility, hashCode()
should use getCanonicalFile for the hashCode (and hope that you don't
get an IOException.)
Arguably a better solution for both equals and hashCode would be to use
getAbsoluteFile instead of getCanonical file, but that would potentially
and subtly change the semantics so that symbolic links would not be
considered equal.
From a historical perspective, the getCanonicalFile code in
RegularFileObject can be traced back to the need to do case-equivalent
fillename matching on Windows. (See 1.5.0_10 ClassReader.java, line 1819
and following.) However, that code was already outdated, since 1.5.0_10
Win32FileSystem.java shows that filename matching ignores case.
The other issue is with ZipFileObject, which is related but different.
This has a novel and probably incorrect definition of equality, which
also violates the general contract of equals. Here's the code from
ZipFileObject.
@Override
public boolean equals(Object other) {
if (!(other instanceof ZipFileObject))
return false;
ZipFileObject o = (ZipFileObject) other;
return zdir.equals(o.zdir) || name.equals(o.name);
}
@Override
public int hashCode() {
return zdir.hashCode() + name.hashCode();
}
The novelty is the '||', which means that two ZipFileObjects are the
same if they share the identical same zip file container, or (yes, or)
they have the same path within a zip file. Even if you change the ||
to &&, we still have an additional but relatively minor issue -- that
the condition for equality of zipfiles is inconsistent with the
condition for equality of regular files. (ie. zip file equality doesn't
compare paths and they don't get the same canonical treatment.)
So the proposal is the following
- fix ZipFileObject equality to use && not ||
- fix ZipFileObject to compare zip files the same way we compare regular
files (whatever we decide that should be)
- consider using absolute paths, and not canonical paths for
FileObject.equals, at a risk of a minor behavioral change
- there is already a method StandardFileManager.IsSameFile; this should
use canonical paths to determine same-ness
-- Jon
We have a problem. Actually, we have two, one for RegularFileObject and
one for ZipFileObject.
In JavacFileManager, RegularFileObject.equals does not object the
general contract of equals. Here is the relevant code:
@Override
public boolean equals(Object other) {
if (!(other instanceof RegularFileObject))
return false;
RegularFileObject o = (RegularFileObject) other;
try {
return f.equals(o.f)
|| f.getCanonicalFile().equals(o.f.getCanonicalFile());
} catch (IOException e) {
return false;
}
}
@Override
public int hashCode() {
return f.hashCode();
}
The contract for Object.equals says that:
"Note that it is generally necessary to override the hashCode method
whenever this method is overridden, so as to maintain the general
contract for the hashCode method, which states that equal objects must
have equal hash codes."
The problem is that getCanonicalFile will equate a relative file ("foo")
with its absolute equivalent ("/home/jjg/foo"), even though they have
different hashcodes.
There are two possible solutions. For maximum compatibility, hashCode()
should use getCanonicalFile for the hashCode (and hope that you don't
get an IOException.)
Arguably a better solution for both equals and hashCode would be to use
getAbsoluteFile instead of getCanonical file, but that would potentially
and subtly change the semantics so that symbolic links would not be
considered equal.
From a historical perspective, the getCanonicalFile code in
RegularFileObject can be traced back to the need to do case-equivalent
fillename matching on Windows. (See 1.5.0_10 ClassReader.java, line 1819
and following.) However, that code was already outdated, since 1.5.0_10
Win32FileSystem.java shows that filename matching ignores case.
The other issue is with ZipFileObject, which is related but different.
This has a novel and probably incorrect definition of equality, which
also violates the general contract of equals. Here's the code from
ZipFileObject.
@Override
public boolean equals(Object other) {
if (!(other instanceof ZipFileObject))
return false;
ZipFileObject o = (ZipFileObject) other;
return zdir.equals(o.zdir) || name.equals(o.name);
}
@Override
public int hashCode() {
return zdir.hashCode() + name.hashCode();
}
The novelty is the '||', which means that two ZipFileObjects are the
same if they share the identical same zip file container, or (yes, or)
they have the same path within a zip file. Even if you change the ||
to &&, we still have an additional but relatively minor issue -- that
the condition for equality of zipfiles is inconsistent with the
condition for equality of regular files. (ie. zip file equality doesn't
compare paths and they don't get the same canonical treatment.)
So the proposal is the following
- fix ZipFileObject equality to use && not ||
- fix ZipFileObject to compare zip files the same way we compare regular
files (whatever we decide that should be)
- consider using absolute paths, and not canonical paths for
FileObject.equals, at a risk of a minor behavioral change
- there is already a method StandardFileManager.IsSameFile; this should
use canonical paths to determine same-ness
-- Jon
- backported by
-
JDK-2194731 JSR199 FileObjects don't obey general contract of equals.
- Resolved
- duplicates
-
JDK-6572968 equals and hashcode incorrect for file objects returned from StandardFileManager
- Closed