Harmony
  1. Harmony
  2. HARMONY-4754

[classlib] FilePermission incorrect canonical path

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Classlib
    • Labels:
      None

      Description

      If called with a path ending in "" or "", java.io.FilePermission calls File.getCanonicalPath with the "" or "" still appended. This is incorrect (and produces very odd results if "-" or "*" exists - perhaps as a symlink to some completely different tree).

      For example, if you do:

      mkdir test
      ln s /tmp test/

      then (new FilePermission("test/-", "read")).implies(new FilePermission("/tmp/file", "read")
      is true and (new FilePermission("test/", "read")).implies(new FilePermission("test/file", "read") is false where as on the RI it is the opposite way around since the '' symlink is (correctly) ignored.

        Activity

        Hide
        Mark Hindess added a comment -

        On sun 50 I see:

        lstat64("/path/to/foobartest", ...);

        on Harmony I see:

        readlink("/path/to/foobartest/-", ...);

        (Incidentally, on J9 I see neither and what I originally suggested about the canonical path being found at implies time stands. And at implies time it only stats foobartest and not foobartest/-. At least that tells use that lazy evaluation is okay in order to pass the TCK.)

        I think it is the constructor that is incorrect.

        Show
        Mark Hindess added a comment - On sun 50 I see: lstat64("/path/to/foobartest", ...); on Harmony I see: readlink("/path/to/foobartest/-", ...); (Incidentally, on J9 I see neither and what I originally suggested about the canonical path being found at implies time stands. And at implies time it only stats foobartest and not foobartest/-. At least that tells use that lazy evaluation is okay in order to pass the TCK.) I think it is the constructor that is incorrect.
        Hide
        Tim Ellison added a comment -

        Ok, now if I create the directory "foobartest/" in the cwd, and then touch the file "-" in that directory, when I run the same snippet

        new FilePermission("foobartest/-", "read")

        on RI / Harmony I see (edited for brevity) :

        DIRECTORY ...\Tim test\ SUCCESS FileBothDirectoryInformation: foobartest
        OPEN ...\Tim test\foobartest\ SUCCESS Options: Open Directory Access: 00100001
        DIRECTORY ...\Tim test\foobartest\ SUCCESS FileBothDirectoryInformation: -
        CLOSE ...\Tim test\foobartest\ SUCCESS

        i.e. the RI and harmony both stat the file called "-", even though it is not intended to represent an actual file, just the foobartest directory recursively.

        Are you saying that you don't see that stat? I'm just trying to figure out where the difference lies, whether it is in fact in the constructor or in the implementation of #implies.

        Thanks

        Show
        Tim Ellison added a comment - Ok, now if I create the directory "foobartest/" in the cwd, and then touch the file "-" in that directory, when I run the same snippet new FilePermission("foobartest/-", "read") on RI / Harmony I see (edited for brevity) : DIRECTORY ...\Tim test\ SUCCESS FileBothDirectoryInformation: foobartest OPEN ...\Tim test\foobartest\ SUCCESS Options: Open Directory Access: 00100001 DIRECTORY ...\Tim test\foobartest\ SUCCESS FileBothDirectoryInformation: - CLOSE ...\Tim test\foobartest\ SUCCESS i.e. the RI and harmony both stat the file called "-", even though it is not intended to represent an actual file, just the foobartest directory recursively. Are you saying that you don't see that stat? I'm just trying to figure out where the difference lies, whether it is in fact in the constructor or in the implementation of #implies. Thanks
        Hide
        Mark Hindess added a comment -

        Oops. Ignore that comment. Doing something silly (the RI forks (or clones a new thread) and I had omitted the -f option on strace.)

        Show
        Mark Hindess added a comment - Oops. Ignore that comment. Doing something silly (the RI forks (or clones a new thread) and I had omitted the -f option on strace.)
        Hide
        Tim Ellison added a comment -

        > I also notice that the RI doesn't evaluate the canonical
        > path in the constructor but only when it is required

        Really? I see it behave differently on Windows, using the Sysinternals tool and simply running
        new FilePermission("foobartest/-", "read")

        produces (apologies for poor formatting) on the RI a file system request

        Process Request Path Result Other
        javaw.exe:2104 DIRECTORY C:\Eclipse\workspace\Tim test\ NO SUCH FILE FileBothDirectoryInformation: foobartest

        Show
        Tim Ellison added a comment - > I also notice that the RI doesn't evaluate the canonical > path in the constructor but only when it is required Really? I see it behave differently on Windows, using the Sysinternals tool and simply running new FilePermission("foobartest/-", "read") produces (apologies for poor formatting) on the RI a file system request Process Request Path Result Other javaw.exe:2104 DIRECTORY C:\Eclipse\workspace\Tim test\ NO SUCH FILE FileBothDirectoryInformation: foobartest
        Hide
        Mark Hindess added a comment -

        I also notice that the RI doesn't evaluate the canonical path in the constructor but only when it is required ... for example when the implies method is called. This might result in different behaviour if the filesystem changes between the call to new and the call to implies. We should probably try to behave more like the RI.

        Show
        Mark Hindess added a comment - I also notice that the RI doesn't evaluate the canonical path in the constructor but only when it is required ... for example when the implies method is called. This might result in different behaviour if the filesystem changes between the call to new and the call to implies. We should probably try to behave more like the RI.

          People

          • Assignee:
            Tim Ellison
            Reporter:
            Mark Hindess
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development