RFR: 8210838: Override javax.crypto.Cipher.toString()

Sean Mullan sean.mullan at oracle.com
Fri Nov 16 17:33:34 UTC 2018


Looks ok. If there are no disadvantages to using a StringBuilder, I 
would probably do that, since you are creating 4-5 separate Strings in 
the toString method.

--Sean

On 11/16/18 11:35 AM, Seán Coffey wrote:
> 
> On 16/11/18 16:16, Sean Mullan wrote:
>> On 11/16/18 9:04 AM, Seán Coffey wrote:
>>> That's a good example and point Max. How does this revision look ?
>>>
>>> http://cr.openjdk.java.net/~coffeys/webrev.8210838.v2/webrev/
>>
>> 2832      * This implementation returns a String containing the 
>> transformation
>> 2833      * used by this Cipher, the Cipher mode and the Cipher Provider.
>>
>> I would suggest rewording this as: "This implementation returns a 
>> String containing the transformation, mode, and provider of this Cipher."
> Good suggestion. Changed.
>>
>> 2840         String m = "not initialized";
>> 2841         if (initialized)
>> 2842             m = getOpmodeString(opmode);
>>
>> Or:
>>
>> String m = initialized ? getOpmodeString(opmode) : "not initialized";
> I've moved the switch expression into the toString() call now as per 
> suggestion from Max. I've just
> seen that we should never have an unexpected opmode given the 
> checkOpmode(int) method check.
> I've let an unexpected opmode register with "error" in the switch 
> statement. // should never happen.
> 
> webrev updated in place:
>   http://cr.openjdk.java.net/~coffeys/webrev.8210838.v3/webrev/
> 
> regards,
> Sean.
> 
>>
>> Also, it might be worthwhile to use a StringBuilder if you think this 
>> method may be called often.
>>
>> --Sean
>>
>>>
>>> Regards,
>>> Sean.
>>>
>>> On 16/11/18 03:35, Weijun Wang wrote:
>>>> Signature's toString looks like
>>>>
>>>> public String toString() {
>>>>      String initState = "";
>>>>      switch (state) {
>>>>      case UNINITIALIZED:
>>>>          initState = "<not initialized>";
>>>>          break;
>>>>      case VERIFY:
>>>>          initState = "<initialized for verifying>";
>>>>          break;
>>>>      case SIGN:
>>>>          initState = "<initialized for signing>";
>>>>          break;
>>>>      }
>>>>      return "Signature object: " + getAlgorithm() + initState;
>>>> }
>>>>
>>>> Maybe you can add some similar info.
>>>>
>>>> In fact, it looks like you can extract the debug output at the end 
>>>> of each init() methods into a new toString() method.
>>>>
>>>> Thanks
>>>> Max
>>>>
>>>>> On Nov 16, 2018, at 12:35 AM, Seán Coffey <sean.coffey at oracle.com> 
>>>>> wrote:
>>>>>
>>>>> A simple enhancement to override toString() for javax.crypto.Cipher 
>>>>> class
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8210838
>>>>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8210838/webrev/
>>>>>
>>>>> regards,
>>>>> Sean.
>>>>>
>>>
> 



More information about the security-dev mailing list