jmx-dev RFR: 8002307 javax.management.modelmbean.ModelMBeanInfoSupport may expose internal representation by storing an externally mutable object

[David Holmes]

Based on discussions around 8010815 I withdraw my comments on this change.

I'll leave it to others to review this.

David

On 4/06/2013 7:51 PM, Jaroslav Bachorik wrote:
> On 05/29/2013 04:08 AM, David Holmes wrote:
>> On 28/05/2013 11:13 PM, Jaroslav Bachorik wrote:
>>> Please, review the fix for JDK-8002307.
>>>
>>> The fix assures the immutability by cloning the provided arrays in the
>>> constructor and then cloning them again in the getters.
>>
>> This fix has the same problems/issues as 8010815:
>>
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/016930.html
>>
>> It is not obvious that the specification of these classes allows for
>> cloning, nor that people using this won't expect external changes to the
>> arrays to be passed through.
>>
>> The use of "immutability" in the spec for MBeanInfo is ambiguous. It
>> might just mean that the references to the arrays never change. It need
>> not imply/require "deep" immutability.
>
> Hm, I see. While the implicit mutability is an obvious error as it
> allows external parties to modify the MBeanInfo contents without proper
> checks and without sending the proper notifications there is no word
> about the provided info arrays being immutable either in javadoc or the
> JMX spec.
>
>>
>> If we want to do this aggressive defensive copying to placate FindBugs
>> (which is simply flagging things to pay attention to, not that are
>> necessarily incorrect) then I think spec updates may also be required.
>
> We wouldn't be doing this only to placate FindBugs - it is rather easy
> to break JMX by eg. nullifying a slot of MBeanAttributeInfo[] array that
> was previously used to build a ModelMBeanInfoSupport.
>
> If the spec is to be updated we should also rev the JSR 3. I'm just
> wandering whether it is really worth to address this issue :/
>
> -JB-
>
>>
>> David
>> -----
>>
>>> The constructors are fixed in the javax/management/MBeanInfo.java and
>>> the arrays used in getters are cloned using an already existing
>>> functionality in the same class.
>>>
>>> http://cr.openjdk.java.net/~jbachorik/8002307/webrev.01
>>>
>>> Thanks,
>>>
>>> -JB-
>>>
>