Uploaded image for project: 'Derby'
  1. Derby
  2. DERBY-6503

Starting network server on a network drive fails with JDK 7 on Windows

    Details

    • Urgency:
      Normal
    • Issue & fix info:
      Workaround attached

      Description

      Starting a network server on a network drive with JDK 7 on Windows fails. The reported exception is a ClassCastException, but the underlying exception is the following:

      java.nio.file.AccessDeniedException: \\host\path\derby.log
      at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:83)
      at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
      at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
      at sun.nio.fs.WindowsAclFileAttributeView.setAcl(WindowsAclFileAttributeView.java:221)
      at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
      at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      at java.lang.reflect.Method.invoke(Method.java:601)
      at org.apache.derby.iapi.services.io.FileUtil.limitAccessToOwnerViaACLs(FileUtil.java:897)
      at org.apache.derby.iapi.services.io.FileUtil.limitAccessToOwner(FileUtil.java:747)
      at org.apache.derby.impl.services.stream.SingleStream.PBmakeFileHPW(SingleStream.java:205)
      at org.apache.derby.impl.services.stream.SingleStream.run(SingleStream.java:401)
      at org.apache.derby.impl.services.stream.SingleStream.run(SingleStream.java:72)
      at java.security.AccessController.doPrivileged(Native Method)
      at org.apache.derby.impl.services.stream.SingleStream.makeFileHPW(SingleStream.java:394)
      at org.apache.derby.impl.services.stream.SingleStream.createDefaultStream(SingleStream.java:356)
      at org.apache.derby.impl.services.stream.SingleStream.makeStream(SingleStream.java:132)
      at org.apache.derby.impl.services.stream.SingleStream.boot(SingleStream.java:92)
      at org.apache.derby.impl.services.monitor.BaseMonitor.boot(BaseMonitor.java:1991)
      at org.apache.derby.impl.services.monitor.TopService.bootModule(TopService.java:334)
      at org.apache.derby.impl.services.monitor.BaseMonitor.startModule(BaseMonitor.java:541)
      at org.apache.derby.impl.services.monitor.FileMonitor.startModule(FileMonitor.java:44)
      at org.apache.derby.iapi.services.monitor.Monitor.startSystemModule(Monitor.java:362)
      at org.apache.derby.impl.services.monitor.BaseMonitor.runWithState(BaseMonitor.java:343)
      at org.apache.derby.impl.services.monitor.FileMonitor.<init>(FileMonitor.java:58)
      at org.apache.derby.iapi.services.monitor.Monitor.startMonitor(Monitor.java:285)
      at org.apache.derby.iapi.jdbc.JDBCBoot.boot(JDBCBoot.java:67)
      at org.apache.derby.jdbc.EmbeddedDriver.boot(EmbeddedDriver.java:199)
      at org.apache.derby.jdbc.EmbeddedDriver.<clinit>(EmbeddedDriver.java:95)
      at java.lang.Class.forName0(Native Method)
      at java.lang.Class.forName(Class.java:188)
      at org.apache.derby.impl.drda.NetworkServerControlImpl.startNetworkServer(NetworkServerControlImpl.java:1032)
      at org.apache.derby.impl.drda.NetworkServerControlImpl.blockingStart(NetworkServerControlImpl.java:732)
      at org.apache.derby.impl.drda.NetworkServerControlImpl.executeWork(NetworkServerControlImpl.java:2277)
      at org.apache.derby.drda.NetworkServerControl.main(NetworkServerControl.java:353)

      1. d6503-1a.diff
        43 kB
        Knut Anders Hatlen
      2. DERBY6503_backport_patch1_diff.txt
        36 kB
        Mamta A. Satoor

        Issue Links

          Activity

          Hide
          knutanders Knut Anders Hatlen added a comment -

          [ Automatically closing all resolved issues that haven't been updated in more than six months. ]

          Show
          knutanders Knut Anders Hatlen added a comment - [ Automatically closing all resolved issues that haven't been updated in more than six months. ]
          Hide
          myrna Myrna van Lunteren added a comment -

          Re-resolving and re-assigning to original fixer.

          Show
          myrna Myrna van Lunteren added a comment - Re-resolving and re-assigning to original fixer.
          Hide
          mamtas Mamta A. Satoor added a comment -

          I agree it is wise for us to not do this backport considering how involved the FileUtil changes are.

          The issue is that FileUtil has gotten very out of sync between 10.10 and trunk. The reason for this is that we have had to backport changes out of order from trunk to 10.10 for FileUtil(this is because there are few changes in trunk which are not suitable for bacport to 10.10). This makes backport to FileUtil much harder if the FileUtil is touched a lot by a specific checkin in trunk(which is what happened with this jira).

          I will label this jira as reject for backport to 10.10

          Show
          mamtas Mamta A. Satoor added a comment - I agree it is wise for us to not do this backport considering how involved the FileUtil changes are. The issue is that FileUtil has gotten very out of sync between 10.10 and trunk. The reason for this is that we have had to backport changes out of order from trunk to 10.10 for FileUtil(this is because there are few changes in trunk which are not suitable for bacport to 10.10). This makes backport to FileUtil much harder if the FileUtil is touched a lot by a specific checkin in trunk(which is what happened with this jira). I will label this jira as reject for backport to 10.10
          Hide
          myrna Myrna van Lunteren added a comment -

          It seems this backport is too involved for the benefits.

          Show
          myrna Myrna van Lunteren added a comment - It seems this backport is too involved for the benefits.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          I wouldn't recommend spending a lot of time backporting this fix. All the fix does, is changing a ClassCastException to an IOException. It makes it easier to see what the underlying problem is, but the cases that failed before the fix, will still fail after the fix.

          Show
          knutanders Knut Anders Hatlen added a comment - I wouldn't recommend spending a lot of time backporting this fix. All the fix does, is changing a ClassCastException to an IOException. It makes it easier to see what the underlying problem is, but the cases that failed before the fix, will still fail after the fix.
          Hide
          mamtas Mamta A. Satoor added a comment -

          Here is my attempt at backporting the 2 commits from trunk to 10.10. This patch(DERBY6503_backport_patch1_diff) is not ready for commit. It has been a non-trivial backport with quite a bit of hand changes(specifically in FileUtil.java). I will appreciate if another pair of eyes can see if the hand backport has missed anything. Also, with the patch, I am getting following error during compilation on 10.10(I haven't debugged the build failure yet to see what the problem might be)
          compile_iapi_error:

          compile_iapi_services_jsr169:

          BUILD FAILED
          C:\p4clients\svn10.10\client1\10.10\build.xml:623: The following error occurred while executing this line:
          C:\p4clients\svn10.10\client1\10.10\java\engine\build.xml:59: The following error occurred while executing this line:
          C:\p4clients\svn10.10\client1\10.10\java\engine\org\apache\derby\iapi\build.xml:39: The following error occurred while executing this line:
          C:\p4clients\svn10.10\client1\10.10\java\engine\org\apache\derby\iapi\services\build.xml:61: The following error occurred while executing this line:
          C:\p4clients\svn10.10\client1\10.10\java\engine\org\apache\derby\iapi\services\io\build.xml:19: Content is not allowed in prolog.

          Total time: 1 second

          Show
          mamtas Mamta A. Satoor added a comment - Here is my attempt at backporting the 2 commits from trunk to 10.10. This patch(DERBY6503_backport_patch1_diff) is not ready for commit. It has been a non-trivial backport with quite a bit of hand changes(specifically in FileUtil.java). I will appreciate if another pair of eyes can see if the hand backport has missed anything. Also, with the patch, I am getting following error during compilation on 10.10(I haven't debugged the build failure yet to see what the problem might be) compile_iapi_error: compile_iapi_services_jsr169: BUILD FAILED C:\p4clients\svn10.10\client1\10.10\build.xml:623: The following error occurred while executing this line: C:\p4clients\svn10.10\client1\10.10\java\engine\build.xml:59: The following error occurred while executing this line: C:\p4clients\svn10.10\client1\10.10\java\engine\org\apache\derby\iapi\build.xml:39: The following error occurred while executing this line: C:\p4clients\svn10.10\client1\10.10\java\engine\org\apache\derby\iapi\services\build.xml:61: The following error occurred while executing this line: C:\p4clients\svn10.10\client1\10.10\java\engine\org\apache\derby\iapi\services\io\build.xml:19: Content is not allowed in prolog. Total time: 1 second
          Hide
          mamtas Mamta A. Satoor added a comment -

          After checking with Mike, I am doing the backport to 10.10

          Show
          mamtas Mamta A. Satoor added a comment - After checking with Mike, I am doing the backport to 10.10
          Hide
          mikem Mike Matrigali added a comment -

          reopening issue, and temp assigning it to myself for 10.10 backport effort.

          Show
          mikem Mike Matrigali added a comment - reopening issue, and temp assigning it to myself for 10.10 backport effort.
          Hide
          mikem Mike Matrigali added a comment -

          taking temp ownership while backporting to 10.10

          Show
          mikem Mike Matrigali added a comment - taking temp ownership while backporting to 10.10
          Hide
          knutanders Knut Anders Hatlen added a comment -

          I haven't seen failures that look related to this issue in the nightly tests after the last commit, so I'm marking the issue as resolved.

          Show
          knutanders Knut Anders Hatlen added a comment - I haven't seen failures that look related to this issue in the nightly tests after the last commit, so I'm marking the issue as resolved.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1580845 from Knut Anders Hatlen in branch 'code/trunk'
          [ https://svn.apache.org/r1580845 ]

          DERBY-6503: Ignore LinkageErrors when loading FilePermissionServiceImpl

          When attempting to load the class on Java 6, an
          UnsupportedClassVersionError is (correctly) raised, but it is not
          among the exceptions that are caught and ignored. Add a catch clause
          for its superclass LinkageError to handle it.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1580845 from Knut Anders Hatlen in branch 'code/trunk' [ https://svn.apache.org/r1580845 ] DERBY-6503 : Ignore LinkageErrors when loading FilePermissionServiceImpl When attempting to load the class on Java 6, an UnsupportedClassVersionError is (correctly) raised, but it is not among the exceptions that are caught and ignored. Add a catch clause for its superclass LinkageError to handle it.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          Thanks, Dag. I've committed the patch, but I'll keep this issue open for a few days to see if the nightly tests pass on all platforms. Since different platforms take different paths through this code, some corner cases may have been overlooked.

          Show
          knutanders Knut Anders Hatlen added a comment - Thanks, Dag. I've committed the patch, but I'll keep this issue open for a few days to see if the nightly tests pass on all platforms. Since different platforms take different paths through this code, some corner cases may have been overlooked.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1580789 from Knut Anders Hatlen in branch 'code/trunk'
          [ https://svn.apache.org/r1580789 ]

          DERBY-6503: ClassCastException when network server cannot restrict file permissions

          Make sure the underlying IOException is exposed if the network server
          fails to restrict file permissions. The original exception used to be
          shadowed by a ClassCastException.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1580789 from Knut Anders Hatlen in branch 'code/trunk' [ https://svn.apache.org/r1580789 ] DERBY-6503 : ClassCastException when network server cannot restrict file permissions Make sure the underlying IOException is exposed if the network server fails to restrict file permissions. The original exception used to be shadowed by a ClassCastException.
          Hide
          dagw Dag H. Wanvik added a comment -

          Looks like a good improvement as well as simplification! +1

          Show
          dagw Dag H. Wanvik added a comment - Looks like a good improvement as well as simplification! +1
          Hide
          knutanders Knut Anders Hatlen added a comment -

          Attaching a patch (d6503-1a.diff) which fixes the error handling so
          that it doesn't result in a ClassCastException that hides the
          underlying IOException. This means the IOException is propagated up
          from FileUtil.limitAccessToOwner(), so some calling code that was not
          prepared to see IOExceptions had to be changed as well.

          Description of the changes:

          java/engine/org/apache/derby/iapi/services/io/FilePermissionService.java
          java/engine/org/apache/derby/iapi/services/io/FilePermissionServiceImpl.java

          Add a new interface for changing the file permissions, and an
          implementation class that uses the java.nio.file.attribute package.
          This abstraction layer allows us to compile the code that accesses
          java.nio.file.attribute separately using JDK 7, so that it doesn't
          have to do everything through reflection. This makes the code easier
          to read and modify, and also makes the error handling simpler.

          build.xml
          tools/jar/extraDBMSclasses.properties
          java/engine/org/apache/derby/iapi/services/io/build.xml

          Only build FilePermissionServiceImpl.java when using JDK 7 or higher.

          java/engine/org/apache/derby/iapi/services/io/FileUtil.java

          Use the new FilePermissionService instead of reflection to call the
          java.nio.file.attribute functionality.

          Add "throws IOException" to limitAccessToOwner(), so that callers will
          have to handle it.

          Make the copyDirectory() methods catch IOException from
          limitAccessToOwner() and return false (which is the same way as they
          handle all other IOExceptions, see DERBY-1981).

          java/drda/org/apache/derby/impl/drda/DssTrace.java

          Add "throws IOException" to method declaration. IOExceptions are
          already handled by the caller.

          java/engine/org/apache/derby/impl/io/DirFile.java

          Wrap the new IOException thrown by limitAccessToOwner() in a
          FileNotFoundException to satisfy the StorageFile.getOutputStream()
          interface (which says any error that prevents the file from being
          opened should result in a FileNotFoundException).

          java/engine/org/apache/derby/impl/services/monitor/FileMonitor.java

          Catch IOException and return false if the permissions cannot be
          changed for the newly created system home directory (which is how this
          piece of code handles other exceptions too).

          java/engine/org/apache/derby/impl/store/raw/RawStore.java
          java/engine/org/apache/derby/impl/store/raw/data/RAFContainer.java
          java/engine/org/apache/derby/impl/store/raw/log/LogToFile.java

          Catch the new IOException and wrap it in a StandardException with a
          high-level description of the operation that failed.

          java/engine/org/apache/derby/impl/store/raw/data/StreamFileContainer.java

          Handle all IOExceptions, not only FileNotFoundExceptions. These
          exceptions end up wrapped in a StandardException by the callers.

          java/engine/org/apache/derby/io/StorageFile.java
          java/testing/org/apache/derbyTesting/functionTests/util/corruptio/CorruptFile.java

          Add "throws IOException" to the signature of limitAccessToOwner().
          Callers will handle the exception.

          Show
          knutanders Knut Anders Hatlen added a comment - Attaching a patch (d6503-1a.diff) which fixes the error handling so that it doesn't result in a ClassCastException that hides the underlying IOException. This means the IOException is propagated up from FileUtil.limitAccessToOwner(), so some calling code that was not prepared to see IOExceptions had to be changed as well. Description of the changes: java/engine/org/apache/derby/iapi/services/io/FilePermissionService.java java/engine/org/apache/derby/iapi/services/io/FilePermissionServiceImpl.java Add a new interface for changing the file permissions, and an implementation class that uses the java.nio.file.attribute package. This abstraction layer allows us to compile the code that accesses java.nio.file.attribute separately using JDK 7, so that it doesn't have to do everything through reflection. This makes the code easier to read and modify, and also makes the error handling simpler. build.xml tools/jar/extraDBMSclasses.properties java/engine/org/apache/derby/iapi/services/io/build.xml Only build FilePermissionServiceImpl.java when using JDK 7 or higher. java/engine/org/apache/derby/iapi/services/io/FileUtil.java Use the new FilePermissionService instead of reflection to call the java.nio.file.attribute functionality. Add "throws IOException" to limitAccessToOwner(), so that callers will have to handle it. Make the copyDirectory() methods catch IOException from limitAccessToOwner() and return false (which is the same way as they handle all other IOExceptions, see DERBY-1981 ). java/drda/org/apache/derby/impl/drda/DssTrace.java Add "throws IOException" to method declaration. IOExceptions are already handled by the caller. java/engine/org/apache/derby/impl/io/DirFile.java Wrap the new IOException thrown by limitAccessToOwner() in a FileNotFoundException to satisfy the StorageFile.getOutputStream() interface (which says any error that prevents the file from being opened should result in a FileNotFoundException). java/engine/org/apache/derby/impl/services/monitor/FileMonitor.java Catch IOException and return false if the permissions cannot be changed for the newly created system home directory (which is how this piece of code handles other exceptions too). java/engine/org/apache/derby/impl/store/raw/RawStore.java java/engine/org/apache/derby/impl/store/raw/data/RAFContainer.java java/engine/org/apache/derby/impl/store/raw/log/LogToFile.java Catch the new IOException and wrap it in a StandardException with a high-level description of the operation that failed. java/engine/org/apache/derby/impl/store/raw/data/StreamFileContainer.java Handle all IOExceptions, not only FileNotFoundExceptions. These exceptions end up wrapped in a StandardException by the callers. java/engine/org/apache/derby/io/StorageFile.java java/testing/org/apache/derbyTesting/functionTests/util/corruptio/CorruptFile.java Add "throws IOException" to the signature of limitAccessToOwner(). Callers will handle the exception.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          Thanks, everyone, for the feedback. I think I'll limit the scope of this bug report to only fixing the ClassCastException that hides the underlying IOException (which turned out to be a valid error situation, since the user was not allowed by the operating system to change file permissions on that drive, not even its own files). I've filed DERBY-6521 to track the discussion on whether Derby should raise exceptions also on posix file systems, and whether exceptions should only be raised if the user had explicitly turned on the feature.

          Show
          knutanders Knut Anders Hatlen added a comment - Thanks, everyone, for the feedback. I think I'll limit the scope of this bug report to only fixing the ClassCastException that hides the underlying IOException (which turned out to be a valid error situation, since the user was not allowed by the operating system to change file permissions on that drive, not even its own files). I've filed DERBY-6521 to track the discussion on whether Derby should raise exceptions also on posix file systems, and whether exceptions should only be raised if the user had explicitly turned on the feature.
          Hide
          myrna Myrna van Lunteren added a comment -

          I like the idea of returning an error if the user has explicitly requested it and we cannot deliver, but to only raise a warning if we're defaulting to this behavior.

          Show
          myrna Myrna van Lunteren added a comment - I like the idea of returning an error if the user has explicitly requested it and we cannot deliver, but to only raise a warning if we're defaulting to this behavior.
          Hide
          rhillegas Rick Hillegas added a comment -

          I think that we should at least raise a warning if the user has explicitly requested this extra security but we find that we can't deliver it. Thanks.

          Show
          rhillegas Rick Hillegas added a comment - I think that we should at least raise a warning if the user has explicitly requested this extra security but we find that we can't deliver it. Thanks.
          Hide
          dagw Dag H. Wanvik added a comment -

          I think we should fail, but give a reasonable error message. If not, the user could be given a sense of false security, no?

          Show
          dagw Dag H. Wanvik added a comment - I think we should fail, but give a reasonable error message. If not, the user could be given a sense of false security, no?
          Hide
          knutanders Knut Anders Hatlen added a comment -

          And just as I said we didn't have any reports about this causing any trouble, a customer reported another variant of the ClassCastException when starting the network server:

          java.lang.ClassCastException: java.nio.file.NoSuchFileException cannot be cast to java.lang.RuntimeException

          (I don't have the stack trace of the original NoSuchFileException that gets shadowed by the ClassCastException.)

          So I'm beginning to wonder if a better solution would be to fail only if the user has explicitly requested the new functionality by setting derby.storage.useDefaultFilePermissions=false. In the environments where Derby now defaults to restricting the file permissions (network server started from the command line when Java >= 7 and underlying file system is either posix or supports ACL), and the user hasn't requested the functionality, we should still try to restrict the permissions, but not fail if the permissions could not be changed. I think that would combine security out of the box and backwards compatibility in a good way.

          Show
          knutanders Knut Anders Hatlen added a comment - And just as I said we didn't have any reports about this causing any trouble, a customer reported another variant of the ClassCastException when starting the network server: java.lang.ClassCastException: java.nio.file.NoSuchFileException cannot be cast to java.lang.RuntimeException (I don't have the stack trace of the original NoSuchFileException that gets shadowed by the ClassCastException.) So I'm beginning to wonder if a better solution would be to fail only if the user has explicitly requested the new functionality by setting derby.storage.useDefaultFilePermissions=false. In the environments where Derby now defaults to restricting the file permissions (network server started from the command line when Java >= 7 and underlying file system is either posix or supports ACL), and the user hasn't requested the functionality, we should still try to restrict the permissions, but not fail if the permissions could not be changed. I think that would combine security out of the box and backwards compatibility in a good way.
          Hide
          dagw Dag H. Wanvik added a comment -

          Knut, I guess it would make sense to report errors on a UNIXy platform as well; it does have the merit of alerting us to any such use case, were it happen. If it never does, well, its not much extra code..

          Show
          dagw Dag H. Wanvik added a comment - Knut, I guess it would make sense to report errors on a UNIXy platform as well; it does have the merit of alerting us to any such use case, were it happen. If it never does, well, its not much extra code..
          Hide
          rhillegas Rick Hillegas added a comment -

          The workaround for this issue is to turn off the extra file access controls for Windows platforms which are available on Java SE 7 and higher. To do that, set...

          derby.storage.useDefaultFilePermissions=true

          ...as described here: http://db.apache.org/derby/docs/10.10/ref/rrefproperdefaultfileperms.html

          Show
          rhillegas Rick Hillegas added a comment - The workaround for this issue is to turn off the extra file access controls for Windows platforms which are available on Java SE 7 and higher. To do that, set... derby.storage.useDefaultFilePermissions=true ...as described here: http://db.apache.org/derby/docs/10.10/ref/rrefproperdefaultfileperms.html
          Hide
          knutanders Knut Anders Hatlen added a comment -

          Thanks, Dag. I'll give that a try. The limitAccessToOwner method is also used by the copy methods mentioned in DERBY-1981, and details about IOExceptions thrown there will probably still be lost. I don't plan to address that problem in this issue, though.

          If we start reporting failure to change permissions on NTFS & co, does that mean we should also check the return value from File.setReadable/Writable/Executable that are used on the other platforms and report if it's false? Currently we only have an assert there to check that it never returns false on a non-Windows platform. I don't think that assert has ever been triggered.

          Show
          knutanders Knut Anders Hatlen added a comment - Thanks, Dag. I'll give that a try. The limitAccessToOwner method is also used by the copy methods mentioned in DERBY-1981 , and details about IOExceptions thrown there will probably still be lost. I don't plan to address that problem in this issue, though. If we start reporting failure to change permissions on NTFS & co, does that mean we should also check the return value from File.setReadable/Writable/Executable that are used on the other platforms and report if it's false? Currently we only have an assert there to check that it never returns false on a non-Windows platform. I don't think that assert has ever been triggered.
          Hide
          dagw Dag H. Wanvik added a comment -

          Right. I think your last suggestion is an acceptable solution.

          Show
          dagw Dag H. Wanvik added a comment - Right. I think your last suggestion is an acceptable solution.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          There is some trickiness involved in writing a warning to derby.log when such an error occurs. For example, the exception in the bug description happens when derby.log is being created, so we'd have to store that information somewhere until the engine is up and ready to write to the log.

          This feature has been available for a while now, and has been enabled by default in the network server, but we haven't seen any reports about IOExceptions causing problems (except DERBY-6410, which is a JVM bug). So these exceptions don't appear to be a big problem for users currently, and that makes me think it might be OK to continue failing when IOExceptions are thrown here. Of course they would have to be reported to the user as IOExceptions and not as ClassCastExceptions as they are today. Users experiencing such failures could either turn off the feature or fix the underlying problem.

          At least I think that would be a reasonable first step: Fail in the same situations as today, but show the underlying IOException instead of the ClassCastException. If we start getting reports from users about such failures causing problems, we could consider if we should add some kind of automatic fallback logic as the next step.

          Show
          knutanders Knut Anders Hatlen added a comment - There is some trickiness involved in writing a warning to derby.log when such an error occurs. For example, the exception in the bug description happens when derby.log is being created, so we'd have to store that information somewhere until the engine is up and ready to write to the log. This feature has been available for a while now, and has been enabled by default in the network server, but we haven't seen any reports about IOExceptions causing problems (except DERBY-6410 , which is a JVM bug). So these exceptions don't appear to be a big problem for users currently, and that makes me think it might be OK to continue failing when IOExceptions are thrown here. Of course they would have to be reported to the user as IOExceptions and not as ClassCastExceptions as they are today. Users experiencing such failures could either turn off the feature or fix the underlying problem. At least I think that would be a reasonable first step: Fail in the same situations as today, but show the underlying IOException instead of the ClassCastException. If we start getting reports from users about such failures causing problems, we could consider if we should add some kind of automatic fallback logic as the next step.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          Thanks, Dag. The code that changes the permissions has some platform specific quirks, and it's hard to say for sure if all OS/filesystem combinations are handled, so only guaranteeing "best effort" and falling back to the old behaviour if the permissions cannot be changed sounds like a reasonable approach to me. At least in the case where the new behaviour is chosen by default (that is, when the network server is started from the command line). In the case where the user has explicitly requested the new behaviour by setting the derby.storage.useDefaultFilePermissions property to false, one might argue that a failure is more appropriate, so that the user can choose what the correct course of action is (either fix the permissions or remove the property).

          Show
          knutanders Knut Anders Hatlen added a comment - Thanks, Dag. The code that changes the permissions has some platform specific quirks, and it's hard to say for sure if all OS/filesystem combinations are handled, so only guaranteeing "best effort" and falling back to the old behaviour if the permissions cannot be changed sounds like a reasonable approach to me. At least in the case where the new behaviour is chosen by default (that is, when the network server is started from the command line). In the case where the user has explicitly requested the new behaviour by setting the derby.storage.useDefaultFilePermissions property to false, one might argue that a failure is more appropriate, so that the user can choose what the correct course of action is (either fix the permissions or remove the property).
          Hide
          dagw Dag H. Wanvik added a comment -

          If we can't change permissions (as per normal) for network drive files, I guess we could just report that fact in derby.log as a warning, and otherwise just ignore the errors. The docs should warn about this possibly unwanted behavior, though, so that the user can take other steps to secure the files.

          Show
          dagw Dag H. Wanvik added a comment - If we can't change permissions (as per normal) for network drive files, I guess we could just report that fact in derby.log as a warning, and otherwise just ignore the errors. The docs should warn about this possibly unwanted behavior, though, so that the user can take other steps to secure the files.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          In the network directory where I see this failure, I'm not allowed to change the permissions of derby.log (or any other file for that matter) as the same user using the normal Windows mechanism either (right-click file in Windows Explorer, choose Properties, then Security, and Edit). So it sounds correct that Java raises an exception when the network server tries to do that operation.

          The code comments seem to acknowledge that an IOException is a possibility, but it sounds like it was not believed to be something that could happen in practice. See this code in FileUtil.limitAccessToOwnerViaACLs():

                  } catch (InvocationTargetException e) {
                      // java.security.AccessControlException: access denied
                      // ("java.lang.RuntimePermission" "accessUserInformation") can
                      // happen, so throw.
                      //
                      // Should we get an IOException from getOwner, the cast below
                      // would throw which is fine, since it should not happen.
                      throw (RuntimeException)e.getCause();
                  }
          

          At the very least this code needs to be improved so that it doesn't report the IOException as a ClassCastException.

          I'm not quite sure how errors are supposed to be handled in this code, though. Some exceptions are handled by falling back to using the less capable File.setReadable(), setWritable(), setExecutable() methods. Others are ignored (unless you're running a debug build). Only RuntimeExceptions will be reported currently.

          Show
          knutanders Knut Anders Hatlen added a comment - In the network directory where I see this failure, I'm not allowed to change the permissions of derby.log (or any other file for that matter) as the same user using the normal Windows mechanism either (right-click file in Windows Explorer, choose Properties, then Security, and Edit). So it sounds correct that Java raises an exception when the network server tries to do that operation. The code comments seem to acknowledge that an IOException is a possibility, but it sounds like it was not believed to be something that could happen in practice. See this code in FileUtil.limitAccessToOwnerViaACLs(): } catch (InvocationTargetException e) { // java.security.AccessControlException: access denied // ( "java.lang.RuntimePermission" "accessUserInformation" ) can // happen, so throw . // // Should we get an IOException from getOwner, the cast below // would throw which is fine, since it should not happen. throw (RuntimeException)e.getCause(); } At the very least this code needs to be improved so that it doesn't report the IOException as a ClassCastException. I'm not quite sure how errors are supposed to be handled in this code, though. Some exceptions are handled by falling back to using the less capable File.setReadable(), setWritable(), setExecutable() methods. Others are ignored (unless you're running a debug build). Only RuntimeExceptions will be reported currently.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          This problem was found while investigating DERBY-6410.

          Show
          knutanders Knut Anders Hatlen added a comment - This problem was found while investigating DERBY-6410 .

            People

            • Assignee:
              knutanders Knut Anders Hatlen
              Reporter:
              knutanders Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development