|
[
Permlink
| « Hide
]
Suran Jayathilaka added a comment - 27/Mar/09 07:58 PM
Linking as the issues might be related.
I think there is a real bug here. It seems to be trying to make the directory even when the trace directory is not specified and we need to put the trace file in the default directory. That is not correct. I don't think it is a show stopper for the release, but it sure would be nice to get it fixed, so as not to regress the product. I will post when I have a fix.
I discovered that the change to the policy file and addition of the new permission requirement came later than
The reason for the permission change in
Well, I can fix it so that it won't need read permission, just write permission if the trace directory is in user.dir or derby.system.home, because we know they exists, but for other directories, even if they exist, we will have to add the requirement that the user grant read as well as write permission to the trace directory to derbynet.jar.
Is this acceptable, or should I just revert back to the old behavior where it only needs write permission and will just fail if the directory does not exist? The downside is that the user will have to create the trace directory themselves instead of Derby doing it for them. If I understand correctly, we're talking about a bit of an edge case - the case where a user has got traceDirectory set, the directory exists, they've got read and write permissions for derby.jar to the directory, but they've got write, but not read permissions in the policy file for derbynet.jar to the directory?
I think it would be acceptable to introduce the incompatibility of requiring the users to modify the policy files...It will only be for as long as they have traceDirectory on, which you probably wouldn't do for a production environment. I think forcing a user to create the trace directory is a bigger incompatibility. But, do you need read permission to create a directory? Or would that overwrite what's in the directory if it exists? Does the attached patch d4128.diff help on this problem? It attempts to create the directory with mkdir() first, and only if that doesn't work, it attempts mkdirs(). Since mkdir() doesn't require read permission, it will work without read permission if the parent directory already exists, and it will still create the directory if the parent directory doesn't exist and the required read permission has been granted.
By the way, that was the same approach we ended up with in
My patch broke NetworkServerControlApiTest, GetCurrentPropertiesTest and SecureServerTest, so that's apparently *not* the way to fix it...
I think it might be part of the solution though. I think the problem is that mkdir() will return false if the directory already exists, so we proceed to the mkdirs() call.
This morning it occurred to me that maybe we can use File.canWrite() to determine if the trace directory exists and is writable without needing read permission and then only if that returns false try the mkdir/mkdirs(). I'll try that. Kathey Nope using traceDirectory.canWrite() won't work either because it still requires a new permission if the directory already existed. I think the only way to do this without creating a need for a new permission is to try to make the trace file and then if that fails, try to make the directory. I will try that next.
ok. Here I think is a trunk patch which has the minimal required permissions (derby-4128_kmarsden_diff.txt) If the trace directory already exists, it still only requires write permission on the contents of the directory. The patch attempts to make the trace file and if that fails on the first try with a FileNotFoundException, it will attempt to make the directory and try again.
SystemPropertiesTest and NetworkServerControlApiTest pass with the 10.4.2.0 derbyTesting.jar. Running trunk tests now. We need to document that tracing will now attempt to create the directory and the permissions needed to do that. I will open an issue for that, but I don't see that it will cause any regression or require a release note, because turning on tracing where the directory did not exist would always fail before 10.5. Regression tests passed. Please review. I tried to do a soft upgrade run on trunk, but that doesn't seem to be possible without the upgrade logic in yet. I did run the 10.4.2.0 tests against 10.5 and saw no permissions related issues and 10.4.2.0 ServerPropertiesTest and NetworkServerControlApiTest now pass when run against 10.5.
I noticed an issue with the patch I checked in yesterday for this issue. With the patch, we would not break out of the loop if we successfully created the trace file on the first try, so would create it a second time. I will commit the attached patch (derby-4128_part2_diff.txt) after running tests to correct that problem.
Checked in fix to trunk and 10.5 branch. Thank you Suran for finding this issue.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Resolved
Major
Failure in ServerPropertiesTest due to java.security.AccessControlException on the server side, in 10.4 to 10.5.1. soft upgrade mode
