code review request: 6856069 PrincipalName.clone() does not invoke super.clone()
Chris Hegarty
chris.hegarty at oracle.com
Wed Apr 21 12:11:37 UTC 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