[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-core-libs-dev
Subject:    Re: RFR (XS) 8230415 : Avoid redundant permission checking in FilePermissionCollection and SocketPer
From:       Ivan Gerasimov <ivan.gerasimov () oracle ! com>
Date:       2019-09-30 18:42:03
Message-ID: 10175bdf-027b-54ec-fb30-370843aed6f0 () oracle ! com
[Download RAW message or body]

Thank you Sean for reviewing!

With kind regards,

Ivan


On 9/27/19 7:20 AM, Sean Mullan wrote:
> Hi Ivan,
>
> The fix looks good. Good catch.
>
> --Sean
>
> On 8/30/19 7:32 PM, Ivan Gerasimov wrote:
>> Hello!
>>
>> In the two implementations of 
>> PermissionCollection.implies(Permission), all the permissions are 
>> traversed, and their corresponding bit mask are checked.
>>
>> For example, here's a snippet from FilePermission.java:
>>
>>          int desired = fperm.getMask();
>>          int effective = 0;
>>          int needed = desired;
>>
>>          for (Permission perm : perms.values()) {
>>              FilePermission fp = (FilePermission)perm;
>>              if (((needed & fp.getMask()) != 0) && 
>> fp.impliesIgnoreMask(fperm)) {
>>                  effective |= fp.getMask();
>>                  if ((effective & desired) == desired) {
>>                      return true;
>>                  }
>>                  needed = (desired ^ effective);// <<< should be 
>> (desired & ~effective)
>>              }
>>          }
>>
>> Here, if a permission's mask `fp.getMask()` intersects with `needed`, 
>> but does not fully cover all the needed bits, the variable `needed` 
>> is updated as XOR of desired and effective. This can raise a 
>> not-really-needed bits in the `needed` mask, so that for all 
>> subsequent permissions from the collection with that unneeded bits in 
>> the mask, an expensive fp.impliesIgnoreMask(fperm) will be executed.
>>
>> The fix does not change the behavior, but helps avoid unnecessary 
>> calls to impliesIgnoreMask().
>>
>> Would you please help review a trivial fix?
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8230415
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8230415/00/webrev/
>>
>> Thanks in advance!
>>
>
-- 
With kind regards,
Ivan Gerasimov


[prev in list] [next in list] [prev in thread] [next in thread]