On 26/11/2016 08:54, Wang Weijun wrote:
Please take a review at
http://cr.openjdk.java.net/~weijun/8170364/webrev.00/
The compatibility layer introduced in the new FilePermission implementation requires one
FilePermission to imply another with either a relative path or an absolute path. This is
solved with a private field npath2 inside. When this field is set, the permission's name
is modified a little (JDK-8168127) so that when adding to a FilePermissionCollection, it
is not confused with one without the field. Unfortunately, this modified name is reused
in the merge to create a new "merged" FilePermission which contains a malformed
path now.
This fix creates a new non-public method to create this "merged" FilePermission.
I checked other places and seems the name is not used to create a new
FilePermission.
Ideally FilePermission (and all permissions) would have final fields, I
hope that can be fixed some day. In the mean-time then having
withNewActions create a new FilePermission and then mutate it looks a
bit ugly, can't you just introduce a new non-public constructor to
create it with the effective mask?
In passing then maybe the comment at L1063-1065 can be re-examined and
it looks to be stale (the specific bootstrapping issue mentioned was
fixed in 2015).
The test is one specific scenario and I wonder if you've considered
beefing this up to other scenarios and other targets so that it's more
broadly testing the merging scenarios.
-Alan
