RFR: 8210838: Override javax.crypto.Cipher.toString()
Valerie Peng
valerie.peng at oracle.com
Tue Nov 20 22:57:20 UTC 2018
I saw you've integrated this.
Just want to add that, instead of mode (which is used also for part of
the transformation, e.g. CBC, GCM), maybe we can use "operation" or
"operating mode". Something to keep in mind for future changes, I guess.
Regards,
Valerie
On 11/17/2018 11:36 PM, Weijun Wang wrote:
> The version looks fine. Thanks.
>
>> On Nov 17, 2018, at 1:56 AM, Seán Coffey <sean.coffey at oracle.com> wrote:
>>
>> Thanks Sean. StringBuilder use added :
>>
>> http://cr.openjdk.java.net/~coffeys/webrev.8210838.v4/webrev/
>>
>> Regards,
>> Sean.
>>
>> On 16/11/18 17:33, Sean Mullan wrote:
>>> 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