jmx-dev RFR: 8010285 Enforce the requirement of Management Interfaces being public

[Jaroslav Bachorik]

On 05/30/2013 09:32 AM, Jaroslav Bachorik wrote:
> On Wed 29 May 2013 07:44:34 PM CEST, Daniel Fuchs wrote:
>> On 5/29/13 7:17 PM, Jaroslav Bachorik wrote:
>>> On Wed 29 May 2013 05:33:21 PM CEST, Eamonn McManus wrote:
>>>> I would recommend against changing the code to do additional calls to
>>>> Class.forName during MBean introspection. As I recall we made the
>>>> opposite change some years ago, both because Class.forName can be slow
>>>> (it may call out to a user ClassLoader) and because it is a potential
>>>> source of security problems.
>>>
>>> Thanks. I was trying to dig some history from mercurial but couldn't.
>>> Walking through all the related interfaces is equally acceptable - I've
>>> tried both of the solutions and they test well with the regtests.
>>>
>>> I am still puzzled by the current implementation which will fail to
>>> locate the correct MBean interface in eg.
>>>
>>> <<CInterface>> extends <<BInterface>> extends <<ServiceMBean>>
>>>
>>> ClassA extends Service implements <<CInterface>>
>>>
>>> as the process would stop on <<BInterface>> (checks the superclass of
>>> the ClassA, checks all the interfaces implemented by the Service class,
>>> checks all the interfaces extended by <<CInterface>>) which plainly
>>> does not conform to the MBean interface naming convention and would
>>> miss the <<ServiceMBean>> interface.
>>
>> Hi Jaroslav,
>>
>> <<Service>> would have to implement <<ServiceMBean>> either
>> directly or indirectly.
>>
>> So the current implementation is correct.
>>
>> If <<ServiceMBean>> is not assignable from <<Service>> then
>> <<ServiceMBean>> is not an MBean interface for ClassA.
> 
> Actually, when you do
> ClassA extends Service implements <<BInterface>>
> 
> the Introspector will return <<ServiceMBean>> as the standard mbean 
> interface for ClassA. I've just tried it on a simple project to make 
> sure I understand the code correctly. The puzzle is which behaviour is 
> correct? Either all the levels of the interface hierarchy should be 
> checked for the [className]MBean interfaces or none, I guess. However, 
> I can not find anything in the spec related to this case.

I've updated the webrev not to use Class.forName() when inferring the
MBean interface to use.

However, I've changed the way the interface hierarchy is treated -
originally, all the interfaces of a certain class and their direct super
interfaces were checked for the MBean interface candidates (with
[class.Name]MBean class name) - but only one level of the hierarchy was
checked. The new implementation will check only the direct interfaces.

I am still not sure whether the interface hierarchy should be searched
for the appropriately named interfaces or not. The spec does not say
anything about this and all the jtreg and jck tests are indifferent to
my changes. But I am sure that if it is supposed to search the interface
hierarchy it should not stop after the first level has been checked, as
is done by the current implementation.

Webrev: http://cr.openjdk.java.net/~jbachorik/8010285/webrev.02

-JB-

> 
> -JB-
> 
>>
>> You can work around that by wrapping an instance of ClassA
>> in an instance of  javax.management.StandardMBean, and by
>> specifying <<ServiceMBean>>.class as the MBean interface
>> in the constructor.
>>
>> Hope this helps,
>>
>> -- daniel
>>
>>>
>>> -JB-
>>>
>>>>
>>>> Éamonn
>>>>
>>>>
>>>> 2013/5/29 Jaroslav Bachorik <jaroslav.bachorik at oracle.com
>>>> <mailto:jaroslav.bachorik at oracle.com>>
>>>>
>>>>      Updated webrev -
>>>>      http://cr.openjdk.java.net/~jbachorik/8010285/webrev.01
>>>>
>>>>      It adds regtests and takes care of the comments from David and
>>>>      Shanliang.
>>>>
>>>>      -JB-
>>>>
>>>>      On 05/28/2013 04:22 PM, Jaroslav Bachorik wrote:
>>>>      > The fix enforces the management interfaces (read MBean and
>>>> MXBean
>>>>      > interfaces) being public. While this is defined in the
>>>>      specification it
>>>>      > was not enforced in any way and it was allowed to create MBeans
>>>>      for eg.
>>>>      > private MBean interfaces.
>>>>      >
>>>>      > The fix adds checks when creating and registering MBeans and
>>>> throws
>>>>      > javax.management.NotCompliantMBeanException when a user tries to
>>>>      create
>>>>      > an MBean with non-public management interface.
>>>>      >
>>>>      > Since this change can cause problems for users having non-public
>>>>      > management interfaces a system property is introduced that will
>>>>      revert
>>>>      > to the old behaviour when set
>>>> (com.sun.jmx.mbeans.allowNonPublic).
>>>>      >
>>>>      > Thanks,
>>>>      >
>>>>      > -JB-
>>>>      >
>>>>
>>>>
>>>
>>>
>>
> 
>