Re: [8] RFR (S): 8033278: Missed access checks for Lookup.unreflect* after 8032585
On Jan 31, 2014, at 1:58 AM, Vladimir Ivanov wrote: > http://cr.openjdk.java.net/~vlivanov/8033278/webrev.00/ > https://bugs.openjdk.java.net/browse/JDK-8033278 > > The fix for 8032585 [1] introduced a regression: in some cases access check > on a reference class is omitted. > > The fix is to ensure that access check on a reference class is always > performed. > 104 case PROTECTED: 105 if ((allowedModes & PROTECTED_OR_PACKAGE_ALLOWED) != 0 && 106 isSamePackage(defc, lookupClass)) 107 return true; 108 if ((allowedModes & PROTECTED) == 0) 109 return false; 110 if ((mods & STATIC) != 0 && 111 !isRelatedClass(refc, lookupClass)) 112 return false; 113 if ((allowedModes & PROTECTED) != 0 && 114 isSuperClass(defc, lookupClass)) 115 return true; 116 return false; Can lines 113 to 116 be reduced to: return isSuperClass(defc, lookupClass)); ? The shuffling of the code looks correct (and simpler), but i am fuzzy on the nuances of access control. Paul. > Testing: regression test, jdk/test/java/lang/invoke/, > jdk/test/java/util/stream, vm.defmeth.testlist, vm.mlvm.testlist, nashorn > (unit tests, octane), groovy > > Thanks! > > Best regards, > Vladimir Ivanov > > [1] http://cr.openjdk.java.net/~vlivanov/8032585/webrev.00/ signature.asc Description: Message signed with OpenPGP using GPGMail ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: [8] RFR (S): 8033278: Missed access checks for Lookup.unreflect* after 8032585
On Jan 31, 2014, at 3:21 PM, Vladimir Ivanov wrote: > Paul, > > The transformation you suggest is equivalent, but I reluctant to apply it. > IMO, it doesn't add much value and current version is easier to read. OK, i guess we will just have to agree to disagree on that small point as i think the opposite :-) but I don't wanna block the review. Paul. > Considering the current level of complexity in VA.isMemberAccessible I don't > want to increase it even further :-) > > Best regards, > Vladimir Ivanov > > PS: thanks for looking into the fix. signature.asc Description: Message signed with OpenPGP using GPGMail ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: [8] RFR (S): 8033278: Missed access checks for Lookup.unreflect* after 8032585
Chris, thank you. Best regards, Vladimir Ivanov On 1/31/14 11:22 PM, Christian Thalinger wrote: > Looks good. > > On Jan 30, 2014, at 4:58 PM, Vladimir Ivanov > wrote: > >> http://cr.openjdk.java.net/~vlivanov/8033278/webrev.00/ >> https://bugs.openjdk.java.net/browse/JDK-8033278 >> >> The fix for 8032585 [1] introduced a regression: in some cases access >> check on a reference class is omitted. >> >> The fix is to ensure that access check on a reference class is always >> performed. >> >> Testing: regression test, jdk/test/java/lang/invoke/, >> jdk/test/java/util/stream, vm.defmeth.testlist, vm.mlvm.testlist, >> nashorn (unit tests, octane), groovy >> >> Thanks! >> >> Best regards, >> Vladimir Ivanov >> >> [1] http://cr.openjdk.java.net/~vlivanov/8032585/webrev.00/ >> ___ >> mlvm-dev mailing list >> mlvm-dev@openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > > ___ > mlvm-dev mailing list > mlvm-dev@openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: [8] RFR (S): 8033278: Missed access checks for Lookup.unreflect* after 8032585
Looks good. On Jan 30, 2014, at 4:58 PM, Vladimir Ivanov wrote: > http://cr.openjdk.java.net/~vlivanov/8033278/webrev.00/ > https://bugs.openjdk.java.net/browse/JDK-8033278 > > The fix for 8032585 [1] introduced a regression: in some cases access > check on a reference class is omitted. > > The fix is to ensure that access check on a reference class is always > performed. > > Testing: regression test, jdk/test/java/lang/invoke/, > jdk/test/java/util/stream, vm.defmeth.testlist, vm.mlvm.testlist, > nashorn (unit tests, octane), groovy > > Thanks! > > Best regards, > Vladimir Ivanov > > [1] http://cr.openjdk.java.net/~vlivanov/8032585/webrev.00/ > ___ > mlvm-dev mailing list > mlvm-dev@openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: [8] RFR (S): 8033278: Missed access checks for Lookup.unreflect* after 8032585
Paul, The transformation you suggest is equivalent, but I reluctant to apply it. IMO, it doesn't add much value and current version is easier to read. Considering the current level of complexity in VA.isMemberAccessible I don't want to increase it even further :-) Best regards, Vladimir Ivanov PS: thanks for looking into the fix. On 1/31/14 1:31 PM, Paul Sandoz wrote: > > On Jan 31, 2014, at 1:58 AM, Vladimir Ivanov > wrote: > >> http://cr.openjdk.java.net/~vlivanov/8033278/webrev.00/ >> https://bugs.openjdk.java.net/browse/JDK-8033278 >> >> The fix for 8032585 [1] introduced a regression: in some cases access check >> on a reference class is omitted. >> >> The fix is to ensure that access check on a reference class is always >> performed. >> > > 104 case PROTECTED: > 105 if ((allowedModes & PROTECTED_OR_PACKAGE_ALLOWED) != 0 && > 106 isSamePackage(defc, lookupClass)) > 107 return true; > 108 if ((allowedModes & PROTECTED) == 0) > 109 return false; > 110 if ((mods & STATIC) != 0 && > 111 !isRelatedClass(refc, lookupClass)) > 112 return false; > 113 if ((allowedModes & PROTECTED) != 0 && > 114 isSuperClass(defc, lookupClass)) > 115 return true; > 116 return false; > > Can lines 113 to 116 be reduced to: > >return isSuperClass(defc, lookupClass)); > > ? > > The shuffling of the code looks correct (and simpler), but i am fuzzy on the > nuances of access control. > > Paul. > >> Testing: regression test, jdk/test/java/lang/invoke/, >> jdk/test/java/util/stream, vm.defmeth.testlist, vm.mlvm.testlist, nashorn >> (unit tests, octane), groovy >> >> Thanks! >> >> Best regards, >> Vladimir Ivanov >> >> [1] http://cr.openjdk.java.net/~vlivanov/8032585/webrev.00/ > ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
[8] RFR (S): 8033278: Missed access checks for Lookup.unreflect* after 8032585
http://cr.openjdk.java.net/~vlivanov/8033278/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8033278 The fix for 8032585 [1] introduced a regression: in some cases access check on a reference class is omitted. The fix is to ensure that access check on a reference class is always performed. Testing: regression test, jdk/test/java/lang/invoke/, jdk/test/java/util/stream, vm.defmeth.testlist, vm.mlvm.testlist, nashorn (unit tests, octane), groovy Thanks! Best regards, Vladimir Ivanov [1] http://cr.openjdk.java.net/~vlivanov/8032585/webrev.00/ ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
