Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v3]
On Mon, 21 Mar 2022 20:19:03 GMT, Kevin Walls wrote: >> Removing permission checks which, in the presence of a Security Manager, >> would check for a RuntimePermission "className.subclass". This was to >> prevent subclassing these classes, but is no longer necessary with strong >> encapsulation from modules. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > remove redundant constructors Thanks all for the comments and reviews! - PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v3]
On Mon, 21 Mar 2022 20:19:03 GMT, Kevin Walls wrote: >> Removing permission checks which, in the presence of a Security Manager, >> would check for a RuntimePermission "className.subclass". This was to >> prevent subclassing these classes, but is no longer necessary with strong >> encapsulation from modules. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > remove redundant constructors Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v2]
On Mon, 21 Mar 2022 18:40:53 GMT, Mandy Chung wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Test update
>
> src/java.management/share/classes/sun/management/spi/PlatformMBeanProvider.java
> line 210:
>
>> 208: }
>> 209:
>> 210: private PlatformMBeanProvider(Void unused) {
>
> This `PlatformMBeanProvider(Void)` constructor is no longer needed. Same
> for `AgentProvider(Void)` constructor.
Thanks, yes they can go... Rebuilt and tested ok after the new commit.
-
PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v3]
> Removing permission checks which, in the presence of a Security Manager, > would check for a RuntimePermission "className.subclass". This was to > prevent subclassing these classes, but is no longer necessary with strong > encapsulation from modules. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: remove redundant constructors - Changes: - all: https://git.openjdk.java.net/jdk/pull/7827/files - new: https://git.openjdk.java.net/jdk/pull/7827/files/98f1577d..913958e9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7827=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7827=01-02 Stats: 6 lines in 2 files changed: 0 ins; 6 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7827.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7827/head:pull/7827 PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v2]
On Fri, 18 Mar 2022 17:49:05 GMT, Kevin Walls wrote:
>> Removing permission checks which, in the presence of a Security Manager,
>> would check for a RuntimePermission "className.subclass". This was to
>> prevent subclassing these classes, but is no longer necessary with strong
>> encapsulation from modules.
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Test update
src/java.management/share/classes/sun/management/spi/PlatformMBeanProvider.java
line 210:
> 208: }
> 209:
> 210: private PlatformMBeanProvider(Void unused) {
This `PlatformMBeanProvider(Void)` constructor is no longer needed. Same for
`AgentProvider(Void)` constructor.
-
PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v2]
> Removing permission checks which, in the presence of a Security Manager, > would check for a RuntimePermission "className.subclass". This was to > prevent subclassing these classes, but is no longer necessary with strong > encapsulation from modules. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Test update - Changes: - all: https://git.openjdk.java.net/jdk/pull/7827/files - new: https://git.openjdk.java.net/jdk/pull/7827/files/a6d05f73..98f1577d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7827=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7827=00-01 Stats: 71 lines in 1 file changed: 28 ins; 11 del; 32 mod Patch: https://git.openjdk.java.net/jdk/pull/7827.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7827/head:pull/7827 PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation
On Tue, 15 Mar 2022 20:22:16 GMT, Kevin Walls wrote: > Removing permission checks which, in the presence of a Security Manager, > would check for a RuntimePermission "className.subclass". This was to > prevent subclassing these classes, but is no longer necessary with strong > encapsulation from modules. Added test update. Checks that with permissive module options we can extend sun.management.spi.PlatformMBeanProvider, and that without them we cannot. Output of the new test is like this: --System.out:(11/1279)-- ---PlatformMBeanProviderConstructorCheck: ---PlatformMBeanProviderConstructorCheck: invoke MyProvider with expectedFail = false ---PlatformMBeanProviderConstructorCheck PASSED (1) (expectedFail = false) ---PlatformMBeanProviderConstructorCheck: re-invoke without --add-modules or --add-exports Command line: [/tank/kwalls/repos/personal/jdk/open/test/../../build/linux-x86_64-server-release/images/jdk/bin/java -cp /tank/kwalls/repos/personal/jdk/open/test/JTwork/classes/sun/management/PlatformMBeanProviderConstructorCheck.d:/tank/kwalls/repos/personal/jdk/open/test/jdk/sun/management:/tank/kwalls/repos/personal/jdk/open/test/JTwork/classes/test/lib:/tank/kwalls/repos/personal/jdk/open/test/lib:/opt/jtreg6.1/lib/javatest.jar:/opt/jtreg6.1/lib/jtreg.jar PlatformMBeanProviderConstructorCheck --nomoduleargs ] [2022-03-18T17:19:34.798024163Z] Gathering output for process 10627 [2022-03-18T17:19:34.902532753Z] Waiting for completion for process 10627 [2022-03-18T17:19:34.902807078Z] Waiting for completion finished for process 10627 Output and diagnostic info for process 10627 was saved into 'pid-10627-output.log' [2022-03-18T17:19:34.908368606Z] Waiting for completion for process 10627 [2022-03-18T17:19:34.908503964Z] Waiting for completion finished for process 10627 --System.err:(9/647)-- stdout: [---PlatformMBeanProviderConstructorCheck: ---PlatformMBeanProviderConstructorCheck: invoke MyProvider with expectedFail = true ---PlatformMBeanProviderConstructorCheck got exception: java.lang.IllegalAccessError: superclass access check failed: class PlatformMBeanProviderConstructorCheck$MyProvider (in unnamed module @0x4795bbf5) cannot access class sun.management.spi.PlatformMBeanProvider (in module java.management) because module java.management does not export sun.management.spi to unnamed module @0x4795bbf5 ---PlatformMBeanProviderConstructorCheck PASSED (2) (expectedFail = true) ]; stderr: [] exitValue = 0 STATUS:Passed. - PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation
On Thu, 17 Mar 2022 13:55:22 GMT, Sean Mullan wrote: > test/jdk/sun/management/PlatformMBeanProviderConstructorCheck.java Thank for noticing that Sean - had run various tests but missed this. I have an update, will add it here soon. - PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation
On Tue, 15 Mar 2022 20:22:16 GMT, Kevin Walls wrote: > Removing permission checks which, in the presence of a Security Manager, > would check for a RuntimePermission "className.subclass". This was to > prevent subclassing these classes, but is no longer necessary with strong > encapsulation from modules. I think you also need to update this test such that it checks that the module access restrictions prevent subclassing instead of the permission check: test/jdk/sun/management/PlatformMBeanProviderConstructorCheck.java Also, it looks like this permission was never publicly documented and was only used internally, so it probably doesn't need a CSR, but I don't know for sure. - PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation
On Wed, 16 Mar 2022 15:14:53 GMT, Daniel Fuchs wrote: >> Removing permission checks which, in the presence of a Security Manager, >> would check for a RuntimePermission "className.subclass". This was to >> prevent subclassing these classes, but is no longer necessary with strong >> encapsulation from modules. > > src/java.management/share/classes/sun/management/spi/PlatformMBeanProvider.java > line 233: > >> 231: } >> 232: return null; >> 233: } > > I have verified in module-info.java that sun.management.spi is only > conditionally exported so I agree that the explicit permission check can now > be removed. thanks > src/jdk.management.agent/share/classes/jdk/internal/agent/spi/AgentProvider.java > line 35: > >> 33: >> 34: /** >> 35: * Instantiates a new AgentProvider. > > This class should probably be removed altogether. I see that you logged > [JDK-8283254](https://bugs.openjdk.java.net/browse/JDK-8283254) so this is > fine. Thanks Daniel - yes will get rid of this class soon, I'll keep these two actions in separate changes. 8-) - PR: https://git.openjdk.java.net/jdk/pull/7827
Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation
On Tue, 15 Mar 2022 20:22:16 GMT, Kevin Walls wrote: > Removing permission checks which, in the presence of a Security Manager, > would check for a RuntimePermission "className.subclass". This was to > prevent subclassing these classes, but is no longer necessary with strong > encapsulation from modules. Marked as reviewed by dfuchs (Reviewer). src/java.management/share/classes/sun/management/spi/PlatformMBeanProvider.java line 233: > 231: } > 232: return null; > 233: } I have verified in module-info.java that sun.management.spi is only conditionally exported so I agree that the explicit permission check can now be removed. src/jdk.management.agent/share/classes/jdk/internal/agent/spi/AgentProvider.java line 35: > 33: > 34: /** > 35: * Instantiates a new AgentProvider. This class should probably be removed altogether. I see that you logged [JDK-8283254](https://bugs.openjdk.java.net/browse/JDK-8283254) so this is fine. - PR: https://git.openjdk.java.net/jdk/pull/7827
