code review request: 6856069 PrincipalName.clone() does not invoke super.clone()

Chris Hegarty chris.hegarty at oracle.com
Wed Apr 21 05:11:37 PDT 2010


On 21/04/2010 12:24, Weijun Wang wrote:
>
> On Apr 21, 2010, at 6:53 PM, Chris Hegarty wrote:
>
>> Max,
>>
>> Good catch to find this bug!
>>
>> Some comments:
>> 1) I don't get why salt now becomes transient. I don't see that it has
>>    any effect on how the object is cloned and class is not Serializable.
>
> Reversed. I just had a habit to mark all non-core data as transient, be it in serialization or clone. Not necessary anyway.
>
>>
>> 2) You should be able to remove L128 in the new file. The cloned object
>>    will have same value for nameType, and since it is a primitive there
>>    shouldn't be an issue.
>>
>> 3) You should be able to replace the arraycopy with nameStrings.clone().
>>    The array elements are immutable Strings, right?
>
> Correct!
>
> Webrev updated at --
>
>      http://cr.openjdk.java.net/~weijun/6856069/webrev.01/

Thanks Max, this looks much cleaner now.

-Chris.

>
> Thanks
> Max
>
> P.S. Oh I hate the latest Google Chrome browser removes http:// in the address bar: have to manually add here.
>
>>
>> -Chris.
>>
>> On 21/04/2010 04:56, Weijun Wang wrote:
>>> Hi
>>>
>>> Anyone can review this code change?
>>>
>>>      http://cr.openjdk.java.net/~weijun/6856069/webrev.00/
>>>
>>> Thanks
>>> Max
>>>
>>> Begin forwarded message:
>>>
>>>> *Change Request ID*: 6856069
>>>> *Synopsis*: PrincipalName.clone() does not invoke super.clone()
>>>>
>>>> === *Description* ============================================================
>>>> PrincipalName's clone() method does not invoke super.clone(), and it has a child class ServiceName. This means the clone of a ServiceName object is not of type ServiceName.
>>>>
>>>> See "Effective Java" Item 10.
>>>>
>>>> *** (#1 of 1): 2009-06-30 07:34:10 GMT+00:00 weijun.wang at sun.com
>>>
>



More information about the security-dev mailing list