Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-6838467

JSR199 FileObjects don't obey general contract of equals.

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: P4 P4
    • 7
    • 6, 7
    • tools
    • b75
    • generic
    • generic
    • Verified

        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

              jjg Jonathan Gibbons
              jjg Jonathan Gibbons
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

                Created:
                Updated:
                Resolved:
                Imported:
                Indexed: