On Wed 29 May 2013 01:18:50 PM CEST, shanliang wrote:
> Jaroslav,
>
> Introspector.java
> ---------------------
> Line 496 - 515
> It is good to do check:
>     (Modifier.isPublic(c.getModifiers()) ||
> MBeanAnalyzer.ALLOW_NONPUBLIC_MBEAN)
> but it is not necessary if an interface is not equal to clMBeanName.

This is being checked within the same condition.

>
> is it possible to simplify the method as:
>      private static <T> Class<? super T> implementsMBean(Class<T> c,
> String clName) {
>         Class<T> ret = null;
>
>         try {
>             Class clMBeanClass = Class.forName(clName + "MBean");
>             List list = Arrays.asList(c.getInterfaces());
>
>             if (list.contains(clMBeanClass)
>                     && (Modifier.isPublic(clMBeanClass.getModifiers())
> || MBeanAnalyzer.ALLOW_NONPUBLIC_MBEAN)) {
>                 ret = clMBeanClass;
>             }
>         } catch (ClassNotFoundException cne) {
>             // clMBeanClass does not exist?
>         }
>
>         return ret;
>     }
>

This would not be the best for the current situation - when I took a 
closer look at the code it seems to contain a bug that makes searching 
for an associate MBean interface much more complex than necessary 
(getStandardMBeanInterface() walks up the class hierarchy and calls 
findMBeanInterface() which does its own hierarchy crawl for the same 
classes). I will try to fix and optimize the code - the Class.forName() 
seems to be a rather good approach.

> Is there any special reason to not have a unit test?

Nope. They were not finished - they will be the part of the next update 
to the webrev.

-JB-

>
> Shanliang
>
>
> Jaroslav Bachorik wrote:
>> And the webrev would come handy, of course.
>>
>> http://cr.openjdk.java.net/~jbachorik/8010285/webrev.00/
>>
>> -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-
>>>
>>>
>>
>>
>


Reply via email to